From c5cc332bf2a0b3ebfa3ad5d26c5b308de5e505be Mon Sep 17 00:00:00 2001 From: Markus Holtermann Date: Sun, 29 Mar 2015 16:59:35 +0200 Subject: [PATCH] Fixed #24550 -- Added migration operation description to sqlmigrate output Thanks Tim Graham for the review. --- django/db/migrations/migration.py | 23 ++++++++----- docs/intro/tutorial01.txt | 11 +++++- docs/ref/contrib/gis/tutorial.txt | 3 ++ docs/ref/django-admin.txt | 6 ++++ docs/releases/1.9.txt | 4 +++ tests/migrations/test_commands.py | 57 +++++++++++++++++++++++++------ 6 files changed, 83 insertions(+), 21 deletions(-) diff --git a/django/db/migrations/migration.py b/django/db/migrations/migration.py index 1c3f560d6e7..59f89afd086 100644 --- a/django/db/migrations/migration.py +++ b/django/db/migrations/migration.py @@ -91,13 +91,16 @@ class Migration(object): for operation in self.operations: # If this operation cannot be represented as SQL, place a comment # there instead - if collect_sql and not operation.reduces_to_sql: + if collect_sql: schema_editor.collected_sql.append("--") - schema_editor.collected_sql.append("-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE " - "WRITTEN AS SQL:") + if not operation.reduces_to_sql: + schema_editor.collected_sql.append( + "-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE WRITTEN AS SQL:" + ) schema_editor.collected_sql.append("-- %s" % operation.describe()) schema_editor.collected_sql.append("--") - continue + if not operation.reduces_to_sql: + continue # Save the state before the operation has run old_state = project_state.clone() operation.state_forwards(self.app_label, project_state) @@ -142,12 +145,14 @@ class Migration(object): # Phase 2 for operation, to_state, from_state in to_run: if collect_sql: + schema_editor.collected_sql.append("--") + if not operation.reduces_to_sql: + schema_editor.collected_sql.append( + "-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE WRITTEN AS SQL:" + ) + schema_editor.collected_sql.append("-- %s" % operation.describe()) + schema_editor.collected_sql.append("--") if not operation.reduces_to_sql: - schema_editor.collected_sql.append("--") - schema_editor.collected_sql.append("-- MIGRATION NOW PERFORMS OPERATION THAT CANNOT BE " - "WRITTEN AS SQL:") - schema_editor.collected_sql.append("-- %s" % operation.describe()) - schema_editor.collected_sql.append("--") continue if not schema_editor.connection.features.can_rollback_ddl and operation.atomic: # We're forcing a transaction on a non-transactional-DDL backend diff --git a/docs/intro/tutorial01.txt b/docs/intro/tutorial01.txt index 8fd59408eb2..9f728207ca9 100644 --- a/docs/intro/tutorial01.txt +++ b/docs/intro/tutorial01.txt @@ -446,8 +446,8 @@ You should see something similar to the following: Migrations for 'polls': 0001_initial.py: - - Create model Question - Create model Choice + - Create model Question - Add field question to choice By running ``makemigrations``, you're telling Django that you've made @@ -476,16 +476,25 @@ readability): .. code-block:: sql BEGIN; + -- + -- Create model Choice + -- CREATE TABLE "polls_choice" ( "id" serial NOT NULL PRIMARY KEY, "choice_text" varchar(200) NOT NULL, "votes" integer NOT NULL ); + -- + -- Create model Question + -- CREATE TABLE "polls_question" ( "id" serial NOT NULL PRIMARY KEY, "question_text" varchar(200) NOT NULL, "pub_date" timestamp with time zone NOT NULL ); + -- + -- Add field question to choice + -- ALTER TABLE "polls_choice" ADD COLUMN "question_id" integer NOT NULL; ALTER TABLE "polls_choice" ALTER COLUMN "question_id" DROP DEFAULT; CREATE INDEX "polls_choice_7aa0f6ee" ON "polls_choice" ("question_id"); diff --git a/docs/ref/contrib/gis/tutorial.txt b/docs/ref/contrib/gis/tutorial.txt index c93e55ec691..ea78274781e 100644 --- a/docs/ref/contrib/gis/tutorial.txt +++ b/docs/ref/contrib/gis/tutorial.txt @@ -286,6 +286,9 @@ This command should produce the following output: .. code-block:: sql BEGIN; + -- + -- Create model WorldBorder + -- CREATE TABLE "world_worldborder" ( "id" serial NOT NULL PRIMARY KEY, "name" varchar(50) NOT NULL, diff --git a/docs/ref/django-admin.txt b/docs/ref/django-admin.txt index e9dbe4b746a..3515c615e7f 100644 --- a/docs/ref/django-admin.txt +++ b/docs/ref/django-admin.txt @@ -984,6 +984,12 @@ By default, the SQL created is for running the migration in the forwards direction. Pass ``--backwards`` to generate the SQL for unapplying the migration instead. +.. versionchanged:: 1.9 + + To increase the readability of the overall SQL output the SQL code + generated for each migration operation is preceded by the operation's + description. + sqlsequencereset ------------------------------------------ diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 16a9e9d5f89..aebdc7ba858 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -152,6 +152,10 @@ Management Commands * The new :djadmin:`sendtestemail` command lets you send a test email to easily confirm that email sending through Django is working. +* To increase the readability of the SQL code generated by + :djadmin:`sqlmigrate`, the SQL code generated for each migration operation is + preceded by the operation's description. + Models ^^^^^^ diff --git a/tests/migrations/test_commands.py b/tests/migrations/test_commands.py index bc289d2f910..7e258b451d8 100644 --- a/tests/migrations/test_commands.py +++ b/tests/migrations/test_commands.py @@ -289,29 +289,64 @@ class MigrateTests(MigrationTestBase): ) @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"}) - def test_sqlmigrate(self): + def test_sqlmigrate_forwards(self): """ Makes sure that sqlmigrate does something. """ - # Make sure the output is wrapped in a transaction out = six.StringIO() call_command("sqlmigrate", "migrations", "0001", stdout=out) - output = out.getvalue() - self.assertIn(connection.ops.start_transaction_sql(), output) - self.assertIn(connection.ops.end_transaction_sql(), output) + output = out.getvalue().lower() - # Test forwards. All the databases agree on CREATE TABLE, at least. - out = six.StringIO() - call_command("sqlmigrate", "migrations", "0001", stdout=out) - self.assertIn("create table", out.getvalue().lower()) + index_tx_start = output.find(connection.ops.start_transaction_sql().lower()) + index_op_desc_author = output.find('-- create model author') + index_create_table = output.find('create table') + index_op_desc_tribble = output.find('-- create model tribble') + index_op_desc_unique_together = output.find('-- alter unique_together') + index_tx_end = output.find(connection.ops.end_transaction_sql().lower()) + self.assertGreater(index_tx_start, -1, "Transaction start not found") + self.assertGreater(index_op_desc_author, index_tx_start, + "Operation description (author) not found or found before transaction start") + self.assertGreater(index_create_table, index_op_desc_author, + "CREATE TABLE not found or found before operation description (author)") + self.assertGreater(index_op_desc_tribble, index_create_table, + "Operation description (tribble) not found or found before CREATE TABLE (author)") + self.assertGreater(index_op_desc_unique_together, index_op_desc_tribble, + "Operation description (unique_together) not found or found before operation description (tribble)") + self.assertGreater(index_tx_end, index_op_desc_unique_together, + "Transaction end not found or found before operation description (unique_together)") + + @override_settings(MIGRATION_MODULES={"migrations": "migrations.test_migrations"}) + def test_sqlmigrate_backwards(self): + """ + Makes sure that sqlmigrate does something. + """ # Cannot generate the reverse SQL unless we've applied the migration. call_command("migrate", "migrations", verbosity=0) - # And backwards is a DROP TABLE out = six.StringIO() call_command("sqlmigrate", "migrations", "0001", stdout=out, backwards=True) - self.assertIn("drop table", out.getvalue().lower()) + output = out.getvalue().lower() + + index_tx_start = output.find(connection.ops.start_transaction_sql().lower()) + index_op_desc_unique_together = output.find('-- alter unique_together') + index_op_desc_tribble = output.find('-- create model tribble') + index_op_desc_author = output.find('-- create model author') + index_drop_table = output.rfind('drop table') + index_tx_end = output.find(connection.ops.end_transaction_sql().lower()) + + self.assertGreater(index_tx_start, -1, "Transaction start not found") + self.assertGreater(index_op_desc_unique_together, index_tx_start, + "Operation description (unique_together) not found or found before transaction start") + self.assertGreater(index_op_desc_tribble, index_op_desc_unique_together, + "Operation description (tribble) not found or found before operation description (unique_together)") + self.assertGreater(index_op_desc_author, index_op_desc_tribble, + "Operation description (author) not found or found before operation description (tribble)") + + self.assertGreater(index_drop_table, index_op_desc_author, + "DROP TABLE not found or found before operation description (author)") + self.assertGreater(index_tx_end, index_op_desc_unique_together, + "Transaction end not found or found before DROP TABLE") # Cleanup by unmigrating everything call_command("migrate", "migrations", "zero", verbosity=0)