Fix the order of SQL statements when creating batches of new models.

Review Request #11167 — Created Sept. 10, 2020 and updated

chipx86
Django Evolution
master
django-evolution

When creating models, EvolveAppTask was generating and storing the SQL
for an entire model creation, which included not just the CREATE TABLE
but any deferred constraints. Each batch of SQL statements would later
be executed in order. If any of these SQL statements contained
constraints that referenced a table introduced by a subsequent task, the
SQL would fail to execute, because that table wouldn't have existed yet.

What's supposed to happen is that the deferred statements are executed
after the tables are created. Hence the "deferred" part. The way the
logic worked, this wasn't possible. SchemaEditor knows how to do this
correctly, as does our legacy logic for Django 1.6, but they require
having the full list of new models (not just a subset) passed to them.

This change redoes this logic to gather all models up-front and to
generate the SQL in one go, ensuring that the ordering of statements is
correct. The only real behavioral change introduced from this is that we
have to emit all creating_models signals up-front, and then emit all
created_models after all models are created, rather than emitting
these in pairs. This means that callers can't expect to receive them in
that order.

It also means that the evolve command will be listing all
"Creating new database models for ..." up-front, rather than when each
model will be created. It's an annoyance, but hopefully a mostly
harmless one, and is necessary without a much larger reworking of how
this SQL is generated.

Unit tests pass for all supported databases.

Verified this fixed an issue with setting up a Review Board database.

Summary
Fix the order of SQL statements when creating batches of new models.
Description From Last Updated

E303 too many blank lines (2)

reviewbotreviewbot
david
  1. Ship It!

  2. 
      
chipx86
Review request changed

Change Summary:

This is largely rewritten and requires re-review.

  • Changed back to generating SQL per-app, but also across all apps. This is to cover the "create these immediate models right now for this app" use case (used when creating initial Django Evolution tables) vs. "create for multiple apps" (standard use case).
  • Added unit tests
  • Added more compatible index name generation for unit tests for Django 1.7 through 1.10.

Commits:

Summary
-
Fix the order of SQL statements when creating batches of new models.
+
Fix the order of SQL statements when creating batches of new models.

Diff:

Revision 2 (+742 -92)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. Ship It!
  2. 
      
Loading...