From: Mike Bayer Date: Tue, 27 Apr 2021 15:14:26 +0000 (-0400) Subject: Sort per-column comparisons in metadata order X-Git-Tag: rel_1_6_0~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=08dc1112a296500c40c3904d321de72ed3b8beb3;p=thirdparty%2Fsqlalchemy%2Falembic.git Sort per-column comparisons in metadata order Improved the rendering of ``op.add_column()`` operations when adding multiple columns to an existing table, so that the order of these statements matches the order in which the columns were declared in the application's table metadata. Previously the added columns were being sorted alphabetically. Change-Id: Ia2a603c934ab101d102d63e715e92f9be9b747a7 Fixes: #827 --- diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index 7ae7b619..dd71515e 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -295,17 +295,20 @@ def _compare_columns( inspector, ): name = "%s.%s" % (schema, tname) if schema else tname - metadata_cols_by_name = dict( - (c.name, c) for c in metadata_table.c if not c.system + metadata_col_names = OrderedSet( + c.name for c in metadata_table.c if not c.system ) - conn_col_names = dict( - (c.name, c) + metadata_cols_by_name = { + c.name: c for c in metadata_table.c if not c.system + } + + conn_col_names = { + c.name: c for c in conn_table.c if autogen_context.run_name_filters( c.name, "column", {"table_name": tname, "schema_name": schema} ) - ) - metadata_col_names = OrderedSet(sorted(metadata_cols_by_name)) + } for cname in metadata_col_names.difference(conn_col_names): if autogen_context.run_object_filters( diff --git a/docs/build/unreleased/827.rst b/docs/build/unreleased/827.rst new file mode 100644 index 00000000..d92fff8f --- /dev/null +++ b/docs/build/unreleased/827.rst @@ -0,0 +1,10 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 827 + + Improved the rendering of ``op.add_column()`` operations when adding + multiple columns to an existing table, so that the order of these + statements matches the order in which the columns were declared in the + application's table metadata. Previously the added columns were being + sorted alphabetically. + diff --git a/tests/test_autogen_composition.py b/tests/test_autogen_composition.py index c37ecbaa..d7b001e8 100644 --- a/tests/test_autogen_composition.py +++ b/tests/test_autogen_composition.py @@ -1,5 +1,12 @@ import re +from sqlalchemy import Column +from sqlalchemy import Integer +from sqlalchemy import MetaData +from sqlalchemy import String +from sqlalchemy import Table +from sqlalchemy.sql.sqltypes import DateTime + from alembic import autogenerate from alembic.migration import MigrationContext from alembic.testing import eq_ @@ -97,13 +104,13 @@ nullable=True)) nullable=True, existing_server_default=sa.text('0')) op.create_foreign_key(None, 'order', 'user', ['user_id'], ['id']) + op.alter_column('user', 'name', + existing_type=sa.VARCHAR(length=50), + nullable=False) op.alter_column('user', 'a1', existing_type=sa.TEXT(), server_default='x', existing_nullable=True) - op.alter_column('user', 'name', - existing_type=sa.VARCHAR(length=50), - nullable=False) op.drop_index('pw_idx', table_name='user') op.drop_column('user', 'pw') # ### end Alembic commands ###""", @@ -115,13 +122,13 @@ nullable=True)) op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \ nullable=True)) op.create_index('pw_idx', 'user', ['pw'], unique=False) - op.alter_column('user', 'name', - existing_type=sa.VARCHAR(length=50), - nullable=True) op.alter_column('user', 'a1', existing_type=sa.TEXT(), server_default=None, existing_nullable=True) + op.alter_column('user', 'name', + existing_type=sa.VARCHAR(length=50), + nullable=True) op.drop_constraint(None, 'order', type_='foreignkey') op.alter_column('order', 'amount', existing_type=sa.Numeric(precision=10, scale=2), @@ -173,13 +180,13 @@ nullable=True)) batch_op.create_foreign_key(None, 'user', ['user_id'], ['id']) with op.batch_alter_table('user', schema=None) as batch_op: + batch_op.alter_column('name', + existing_type=sa.VARCHAR(length=50), + nullable=False) batch_op.alter_column('a1', existing_type=sa.TEXT(), server_default='x', existing_nullable=True) - batch_op.alter_column('name', - existing_type=sa.VARCHAR(length=50), - nullable=False) batch_op.drop_index('pw_idx') batch_op.drop_column('pw') @@ -192,13 +199,13 @@ nullable=True)) with op.batch_alter_table('user', schema=None) as batch_op: batch_op.add_column(sa.Column('pw', sa.VARCHAR(length=50), nullable=True)) batch_op.create_index('pw_idx', ['pw'], unique=False) - batch_op.alter_column('name', - existing_type=sa.VARCHAR(length=50), - nullable=True) batch_op.alter_column('a1', existing_type=sa.TEXT(), server_default=None, existing_nullable=True) + batch_op.alter_column('name', + existing_type=sa.VARCHAR(length=50), + nullable=True) with op.batch_alter_table('order', schema=None) as batch_op: batch_op.drop_constraint(None, type_='foreignkey') @@ -245,6 +252,60 @@ nullable=True)) ) +class AddColumnOrderTest(AutogenTest, TestBase): + @classmethod + def _get_db_schema(cls): + m = MetaData() + + Table( + "user", + m, + Column("id", Integer, primary_key=True), + Column("name", String(50)), + ) + + return m + + @classmethod + def _get_model_schema(cls): + m = MetaData() + + Table( + "user", + m, + Column("id", Integer, primary_key=True), + Column("name", String(50)), + Column("username", String(50)), + Column("password_hash", String(32)), + Column("timestamp", DateTime), + ) + + return m + + def test_render_add_columns(self): + """test #827""" + + template_args = {} + autogenerate._render_migration_diffs(self.context, template_args) + eq_( + re.sub(r"u'", "'", template_args["upgrades"]), + """# ### commands auto generated by Alembic - please adjust! ### + op.add_column('user', sa.Column('username', sa.String(length=50), nullable=True)) + op.add_column('user', sa.Column('password_hash', sa.String(length=32), nullable=True)) + op.add_column('user', sa.Column('timestamp', sa.DateTime(), nullable=True)) + # ### end Alembic commands ###""", # noqa E501 + ) + + eq_( + re.sub(r"u'", "'", template_args["downgrades"]), + """# ### commands auto generated by Alembic - please adjust! ### + op.drop_column('user', 'timestamp') + op.drop_column('user', 'password_hash') + op.drop_column('user', 'username') + # ### end Alembic commands ###""", + ) + + class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestBase): __only_on__ = "postgresql" schema = "test_schema" @@ -318,15 +379,15 @@ schema='%(schema)s') schema='%(schema)s') op.create_foreign_key(None, 'order', 'user', ['user_id'], ['id'], \ source_schema='%(schema)s', referent_schema='%(schema)s') + op.alter_column('user', 'name', + existing_type=sa.VARCHAR(length=50), + nullable=False, + schema='%(schema)s') op.alter_column('user', 'a1', existing_type=sa.TEXT(), server_default='x', existing_nullable=True, schema='%(schema)s') - op.alter_column('user', 'name', - existing_type=sa.VARCHAR(length=50), - nullable=False, - schema='%(schema)s') op.drop_index('pw_idx', table_name='user', schema='test_schema') op.drop_column('user', 'pw', schema='%(schema)s') # ### end Alembic commands ###""" @@ -339,15 +400,15 @@ source_schema='%(schema)s', referent_schema='%(schema)s') op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \ autoincrement=False, nullable=True), schema='%(schema)s') op.create_index('pw_idx', 'user', ['pw'], unique=False, schema='%(schema)s') - op.alter_column('user', 'name', - existing_type=sa.VARCHAR(length=50), - nullable=True, - schema='%(schema)s') op.alter_column('user', 'a1', existing_type=sa.TEXT(), server_default=None, existing_nullable=True, schema='%(schema)s') + op.alter_column('user', 'name', + existing_type=sa.VARCHAR(length=50), + nullable=True, + schema='%(schema)s') op.drop_constraint(None, 'order', schema='%(schema)s', type_='foreignkey') op.alter_column('order', 'amount', existing_type=sa.Numeric(precision=10, scale=2), diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index 94a91673..2a1d762b 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -462,15 +462,15 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase): diffs[6], "add_fk", "order", ["user_id"], "user", ["id"] ) - eq_(diffs[7][0][0], "modify_default") - eq_(diffs[7][0][1], None) - eq_(diffs[7][0][2], "user") - eq_(diffs[7][0][3], "a1") - eq_(diffs[7][0][6].arg, "x") + eq_(diffs[7][0][0], "modify_nullable") + eq_(diffs[7][0][5], True) + eq_(diffs[7][0][6], False) - eq_(diffs[8][0][0], "modify_nullable") - eq_(diffs[8][0][5], True) - eq_(diffs[8][0][6], False) + eq_(diffs[8][0][0], "modify_default") + eq_(diffs[8][0][1], None) + eq_(diffs[8][0][2], "user") + eq_(diffs[8][0][3], "a1") + eq_(diffs[8][0][6].arg, "x") eq_(diffs[9][0], "remove_index") eq_(diffs[9][1].name, "pw_idx") @@ -777,15 +777,15 @@ class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestBase): source_schema=config.test_schema, ) - eq_(diffs[7][0][0], "modify_default") - eq_(diffs[7][0][1], self.schema) - eq_(diffs[7][0][2], "user") - eq_(diffs[7][0][3], "a1") - eq_(diffs[7][0][6].arg, "x") + eq_(diffs[7][0][0], "modify_nullable") + eq_(diffs[7][0][5], True) + eq_(diffs[7][0][6], False) - eq_(diffs[8][0][0], "modify_nullable") - eq_(diffs[8][0][5], True) - eq_(diffs[8][0][6], False) + eq_(diffs[8][0][0], "modify_default") + eq_(diffs[8][0][1], self.schema) + eq_(diffs[8][0][2], "user") + eq_(diffs[8][0][3], "a1") + eq_(diffs[8][0][6].arg, "x") eq_(diffs[9][0], "remove_index") eq_(diffs[9][1].name, "pw_idx") @@ -1558,15 +1558,15 @@ class CompareMetadataTest(ModelOne, AutogenTest, TestBase): diffs[6], "add_fk", "order", ["user_id"], "user", ["id"] ) - eq_(diffs[7][0][0], "modify_default") - eq_(diffs[7][0][1], None) - eq_(diffs[7][0][2], "user") - eq_(diffs[7][0][3], "a1") - eq_(diffs[7][0][6].arg, "x") + eq_(diffs[7][0][0], "modify_nullable") + eq_(diffs[7][0][5], True) + eq_(diffs[7][0][6], False) - eq_(diffs[8][0][0], "modify_nullable") - eq_(diffs[8][0][5], True) - eq_(diffs[8][0][6], False) + eq_(diffs[8][0][0], "modify_default") + eq_(diffs[8][0][1], None) + eq_(diffs[8][0][2], "user") + eq_(diffs[8][0][3], "a1") + eq_(diffs[8][0][6].arg, "x") eq_(diffs[9][0], "remove_index") eq_(diffs[9][1].name, "pw_idx")