From 56e295280f6bd408618c0b65feab9180232daeda Mon Sep 17 00:00:00 2001 From: Fire162 Date: Sat, 17 Oct 2026 14:36:01 +0530 Subject: [PATCH] compare identity, computed defaults when compare_server_default is false In 1.18.0, comparators were refactored to use PriorityDispatcher. _user_compare_server_default was registered at DispatchPriority.FIRST under the "server_default" subgroup. When compare_server_default is omitted or False (the default), _user_compare_server_default returns STOP, which aborted all subsequent comparators registered in the "server_default" subgroup before _compare_computed_default and _compare_identity_default could run. In addition, _setup_autoincrement was erroneously registered under the "server_default" subgroup, causing autoincrement settings to be skipped when server default comparison stopped. Fix this by: 1. Registering _compare_computed_default and _compare_identity_default at DispatchPriority.FIRST in the "server_default" subgroup, so they run before _user_compare_server_default (at DispatchPriority.MEDIUM). 2. Ensuring _compare_identity_default always returns STOP when an identity default is present, and populates existing_server_default. 3. Setting existing_server_default in _compare_computed_default. 4. Registering _setup_autoincrement under the "autoincrement" element. Fixes: #1877 --- .../autogenerate/compare/server_defaults.py | 20 ++- docs/build/unreleased/1877.rst | 7 + tests/test_autogen_diffs.py | 122 ++++++++++++++++++ 3 files changed, 142 insertions(+), 7 deletions(-) create mode 100644 docs/build/unreleased/1877.rst diff --git a/alembic/autogenerate/compare/server_defaults.py b/alembic/autogenerate/compare/server_defaults.py index f5d6d0d9..b3029a30 100644 --- a/alembic/autogenerate/compare/server_defaults.py +++ b/alembic/autogenerate/compare/server_defaults.py @@ -74,12 +74,15 @@ def _compare_computed_default( if sqla_compat._server_default_is_computed( conn_col_default ) and not sqla_compat._server_default_is_computed(metadata_default): + alter_column_op.existing_server_default = conn_col_default _warn_computed_not_supported(tname, cname) return PriorityDispatchResult.STOP if not sqla_compat._server_default_is_computed(metadata_default): return PriorityDispatchResult.CONTINUE + alter_column_op.existing_server_default = conn_col_default + rendered_metadata_default = str( cast(sa_schema.Computed, metadata_col.server_default).sqltext.compile( dialect=autogen_context.dialect, @@ -145,6 +148,8 @@ def _compare_identity_default( ): return PriorityDispatchResult.CONTINUE + alter_column_op.existing_server_default = conn_col_default + assert isinstance( metadata_col.server_default, (sa_schema.Identity, sa_schema.Sequence, NoneType), @@ -170,9 +175,7 @@ def _compare_identity_default( sorted(diff), ) - return PriorityDispatchResult.STOP - - return PriorityDispatchResult.CONTINUE + return PriorityDispatchResult.STOP def _user_compare_server_default( @@ -312,27 +315,30 @@ def _setup_autoincrement( def setup(plugin: Plugin) -> None: plugin.add_autogenerate_comparator( - _user_compare_server_default, + _compare_computed_default, "column", "server_default", priority=DispatchPriority.FIRST, ) + plugin.add_autogenerate_comparator( - _compare_computed_default, + _compare_identity_default, "column", "server_default", + priority=DispatchPriority.FIRST, ) plugin.add_autogenerate_comparator( - _compare_identity_default, + _user_compare_server_default, "column", "server_default", + priority=DispatchPriority.MEDIUM, ) plugin.add_autogenerate_comparator( _setup_autoincrement, "column", - "server_default", + "autoincrement", ) plugin.add_autogenerate_comparator( _dialect_impl_compare_server_default, diff --git a/docs/build/unreleased/1877.rst b/docs/build/unreleased/1877.rst new file mode 100644 index 00000000..1abe5097 --- /dev/null +++ b/docs/build/unreleased/1877.rst @@ -0,0 +1,7 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 1877 + + Fixed regression introduced in 1.18.0 where identity and computed column + comparisons, as well as autoincrement metadata setting, were skipped during + autogenerate when ``compare_server_default`` was not enabled. diff --git a/tests/test_autogen_diffs.py b/tests/test_autogen_diffs.py index 2d28b02d..8c9136db 100644 --- a/tests/test_autogen_diffs.py +++ b/tests/test_autogen_diffs.py @@ -4,6 +4,7 @@ from sqlalchemy import CHAR from sqlalchemy import CheckConstraint from sqlalchemy import Column +from sqlalchemy import Computed from sqlalchemy import DATE from sqlalchemy import DateTime from sqlalchemy import DECIMAL @@ -12,6 +13,7 @@ from sqlalchemy import ForeignKey from sqlalchemy import ForeignKeyConstraint from sqlalchemy import func +from sqlalchemy import Identity from sqlalchemy import Index from sqlalchemy import inspect from sqlalchemy import INTEGER @@ -943,6 +945,126 @@ def my_compare_server_default( else: assert not diff + def test_computed_warning_when_compare_server_default_false( + self, connection, metadata + ): + t1 = Table( + "t1", + metadata, + Column("id", Integer, primary_key=True), + Column("x", Integer), + Column("y", Integer, Computed("x + 1")), + ) + t1.create(connection) + + new_metadata = MetaData() + Table( + "t1", + new_metadata, + Column("id", Integer, primary_key=True), + Column("x", Integer), + Column("y", Integer, Computed("x + 2")), + ) + + mc = MigrationContext.configure( + connection, opts={"compare_server_default": False} + ) + + with mock.patch("alembic.util.warn") as mock_warn: + diff = api.compare_metadata(mc, new_metadata) + + eq_( + mock_warn.mock_calls, + [mock.call("Computed default on t1.y cannot be modified")], + ) + eq_(diff, []) + + def test_identity_compared_when_compare_server_default_false( + self, connection, metadata + ): + t1 = Table( + "t1", + metadata, + Column("id", Integer, primary_key=True), + ) + t1.create(connection) + + new_metadata = MetaData() + Table( + "t1", + new_metadata, + Column("id", Integer, Identity(start=10), primary_key=True), + ) + + orig_get_columns = connection.dialect.get_columns + + def mock_get_columns(conn, table_name, schema=None, **kw): + cols = orig_get_columns(conn, table_name, schema=schema, **kw) + if table_name == "t1": + for c in cols: + if c["name"] == "id": + c["identity"] = {"start": 1, "increment": 1} + return cols + + mc = MigrationContext.configure( + connection, opts={"compare_server_default": False} + ) + + with mock.patch.object( + connection.dialect, "get_columns", side_effect=mock_get_columns + ): + diff = api.compare_metadata(mc, new_metadata) + + eq_(len(diff), 1) + eq_(diff[0][0][0], "modify_default") + eq_(diff[0][0][2], "t1") + eq_(diff[0][0][3], "id") + old = diff[0][0][5] + new = diff[0][0][6] + is_(isinstance(old, Identity), True) + eq_(old.start, 1) + eq_(old.increment, 1) + is_(isinstance(new, Identity), True) + eq_(new.start, 10) + + def test_autoincrement_preserved_when_compare_server_default_false( + self, connection, metadata + ): + t1 = Table( + "t1", + metadata, + Column("id", Integer, primary_key=True), + Column("x", Integer, server_default="0"), + ) + t1.create(connection) + + new_metadata = MetaData() + Table( + "t1", + new_metadata, + Column("id", Integer, primary_key=True), + Column("x", BigInteger, server_default="0", autoincrement=True), + ) + + mc = MigrationContext.configure( + connection, + opts={"compare_type": True, "compare_server_default": False}, + ) + + script = api.produce_migrations(mc, new_metadata) + uo = script.upgrade_ops_list[-1] + modify_table_ops = [ + op for op in uo.ops if isinstance(op, ops.ModifyTableOps) + ] + eq_(len(modify_table_ops), 1) + alter_col_ops = [ + op + for op in modify_table_ops[0].ops + if isinstance(op, ops.AlterColumnOp) + ] + eq_(len(alter_col_ops), 1) + eq_(alter_col_ops[0].kw.get("autoincrement"), True) + class CompareMetadataToInspectorTest(TestBase): __backend__ = True