Opened 8 months ago

Closed 8 months ago

Last modified 8 months ago

#23379 closed Bug (fixed)

sql_create generates incorrect SQL with synced database

Reported by: flakfizer Owned by: charettes
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 (last modified by charettes)

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:

  • django/core/management/sql.py

    a b def sql_create(app_config, style, connection): 
    3636    # We trim models from the current app so that the sqlreset command does not
    3737    # generate invalid SQL (leaving models out of known_models is harmless, so
    3838    # we can be conservative).
    39     app_models = app_config.get_models(include_auto_created=True)
     39    app_models = set(app_config.get_models(include_auto_created=True))
    4040    final_output = []
    4141    tables = connection.introspection.table_names()
    4242    known_models = set(model for model in connection.introspection.installed_models(tables) if model not in app_models)

Attachments (1)

repro.tgz (2.1 KB) - added by flakfizer 8 months ago.
Reproduction project

Download all attachments as: .zip

Change History (7)

Changed 8 months ago by flakfizer

Reproduction project

comment:1 Changed 8 months ago by charettes

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 8 months ago by charettes

  • Owner changed from nobody to charettes
  • Severity changed from Normal to Release blocker
  • Status changed from new to assigned
  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 8 months ago by charettes

Here's a working patch with tests.

comment:4 Changed 8 months ago by aaugustin

  • Triage Stage changed from Accepted to Ready for checkin

comment:5 Changed 8 months ago by Simon Charette <charette.s@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 1e404180c1bd3535987aba96d42030daab1e9c9f:

Fixed #23379 -- Corrected a referencing issue in sql_create.

Thanks to Trac alias flakfizer for the report.

comment:6 Changed 8 months ago by Simon Charette <charette.s@…>

In 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

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