Opened 10 years ago

Last modified 10 years ago

#23379 closed Bug

sql_create generates incorrect SQL with synced database — at Initial Version

Reported by: flakfizer Owned by: nobody
Component: Database layer (models, ORM) Version: 1.7-rc-3
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

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

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)

Change History (1)

by flakfizer, 10 years ago

Attachment: repro.tgz added

Reproduction project

Note: See TracTickets for help on using tickets.
Back to Top