[Django] #23379: sql_create generates incorrect SQL with synced database

6 views
Skip to first unread message

Django

unread,
Aug 28, 2014, 12:30:26 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
----------------------------------------------+----------------------
Reporter: flakfizer | Owner: nobody
Type: Bug | Status: new
Component: Database layer (models, ORM) | Version: 1.7-rc-3
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Easy pickings: 0 | UI/UX: 0
----------------------------------------------+----------------------
The change between 1.6.6 to 1.7c3 in sql_create's first parameter from
"app" to "app_config" is causing a bug, namely because app.get_models()
returned a list, whereas app_config.get_models() returns a generator.
This causes the "known_models" set to be larger than it should be.

This bug will cause invalid sql - mysql, in my case - to be generated if
tables already exist, as shown in the attached reproduction app.

1) python manage.py repro # generates correct sql

CREATE TABLE `repro_app_a` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`b_id` integer NOT NULL
)
;
CREATE TABLE `repro_app_b` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`a_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_b` ADD CONSTRAINT `a_id_refs_id_4838e71a` FOREIGN
KEY (`a_id`) REFERENCES `repro_app_a` (`id`);
ALTER TABLE `repro_app_a` ADD CONSTRAINT `b_id_refs_id_b49a3179` FOREIGN
KEY (`b_id`) REFERENCES `repro_app_b` (`id`);

2) python manage.py migrate
3) python manage.py repro # generates incorrect sql; the first constraint
fails because table repro_app_b hasn't been created yet.

CREATE TABLE `repro_app_a` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`b_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_a` ADD CONSTRAINT `b_id_refs_id_b49a3179` FOREIGN
KEY (`b_id`) REFERENCES `repro_app_b` (`id`);
CREATE TABLE `repro_app_b` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`a_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_b` ADD CONSTRAINT `a_id_refs_id_4838e71a` FOREIGN
KEY (`a_id`) REFERENCES `repro_app_a` (`id`);


Fortunately, the (or, "a") fix is an easy one-liner:

diff --git a/django/core/management/sql.py b/django/core/management/sql.py
index 323cb54..048b051 100644
--- a/django/core/management/sql.py
+++ b/django/core/management/sql.py
@@ -36,7 +36,7 @@ def sql_create(app_config, style, connection):
# We trim models from the current app so that the sqlreset command
does not
# generate invalid SQL (leaving models out of known_models is
harmless, so
# we can be conservative).
- app_models = app_config.get_models(include_auto_created=True)
+ app_models = set(app_config.get_models(include_auto_created=True))
final_output = []
tables = connection.introspection.table_names()
known_models = set(model for model in
connection.introspection.installed_models(tables) if model not in
app_models)

--
Ticket URL: <https://code.djangoproject.com/ticket/23379>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Aug 28, 2014, 1:28:48 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------

Reporter: flakfizer | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by charettes):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0


Old description:

New description:

The change between 1.6.6 to 1.7c3 in sql_create's first parameter from
"app" to "app_config" is causing a bug, namely because app.get_models()
returned a list, whereas app_config.get_models() returns a generator.
This causes the "known_models" set to be larger than it should be.

This bug will cause invalid sql - mysql, in my case - to be generated if
tables already exist, as shown in the attached reproduction app.

1) python manage.py repro # generates correct sql

{{{#!sql


CREATE TABLE `repro_app_a` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`b_id` integer NOT NULL
)
;
CREATE TABLE `repro_app_b` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`a_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_b` ADD CONSTRAINT `a_id_refs_id_4838e71a` FOREIGN
KEY (`a_id`) REFERENCES `repro_app_a` (`id`);
ALTER TABLE `repro_app_a` ADD CONSTRAINT `b_id_refs_id_b49a3179` FOREIGN
KEY (`b_id`) REFERENCES `repro_app_b` (`id`);
}}}
2) python manage.py migrate
3) python manage.py repro # generates incorrect sql; the first constraint
fails because table repro_app_b hasn't been created yet.

{{{#!sql


CREATE TABLE `repro_app_a` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`b_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_a` ADD CONSTRAINT `b_id_refs_id_b49a3179` FOREIGN
KEY (`b_id`) REFERENCES `repro_app_b` (`id`);
CREATE TABLE `repro_app_b` (
`id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
`a_id` integer NOT NULL
)
;
ALTER TABLE `repro_app_b` ADD CONSTRAINT `a_id_refs_id_4838e71a` FOREIGN
KEY (`a_id`) REFERENCES `repro_app_a` (`id`);
}}}

Fortunately, the (or, "a") fix is an easy one-liner:

{{{
#!diff


--- a/django/core/management/sql.py
+++ b/django/core/management/sql.py
@@ -36,7 +36,7 @@ def sql_create(app_config, style, connection):
# We trim models from the current app so that the sqlreset command
does not
# generate invalid SQL (leaving models out of known_models is
harmless, so
# we can be conservative).
- app_models = app_config.get_models(include_auto_created=True)
+ app_models = set(app_config.get_models(include_auto_created=True))
final_output = []
tables = connection.introspection.table_names()
known_models = set(model for model in
connection.introspection.installed_models(tables) if model not in
app_models)
}}}

--

--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:1>

Django

unread,
Aug 28, 2014, 1:40:16 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------
Reporter: flakfizer | Owner: charettes
Type: Bug | Status: assigned

Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by charettes):

* owner: nobody => charettes
* status: new => assigned
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted


--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:2>

Django

unread,
Aug 28, 2014, 2:21:16 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------
Reporter: flakfizer | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------

Comment (by charettes):

Here's [https://github.com/django/django/pull/3133 a working patch] with
tests.

--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:3>

Django

unread,
Aug 28, 2014, 3:10:47 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------
Reporter: flakfizer | Owner: charettes
Type: Bug | Status: assigned
Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution:
Severity: Release blocker | Triage Stage: Ready for
Keywords: | checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):

* stage: Accepted => Ready for checkin


--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:4>

Django

unread,
Aug 28, 2014, 3:18:12 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------
Reporter: flakfizer | Owner: charettes
Type: Bug | Status: closed

Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution: fixed

Severity: Release blocker | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Simon Charette <charette.s@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"1e404180c1bd3535987aba96d42030daab1e9c9f"]:
{{{
#!CommitTicketReference repository=""
revision="1e404180c1bd3535987aba96d42030daab1e9c9f"
Fixed #23379 -- Corrected a referencing issue in sql_create.

Thanks to Trac alias flakfizer for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:5>

Django

unread,
Aug 28, 2014, 3:24:11 PM8/28/14
to django-...@googlegroups.com
#23379: sql_create generates incorrect SQL with synced database
-------------------------------------+-------------------------------------
Reporter: flakfizer | Owner: charettes
Type: Bug | Status: closed
Component: Database layer | Version: 1.7-rc-3
(models, ORM) | Resolution: fixed
Severity: Release blocker | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Simon Charette <charette.s@…>):

In [changeset:"c32c220881ebca2be7b7bb5bec6ca4b25d1d6454"]:
{{{
#!CommitTicketReference repository=""
revision="c32c220881ebca2be7b7bb5bec6ca4b25d1d6454"
[1.7.x] Fixed #23379 -- Corrected a referencing issue in sql_create.

Thanks to Trac alias flakfizer for the report.

Backport of 1e404180c1 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/23379#comment:6>

Reply all
Reply to author
Forward
0 new messages