From: Federico Caselli Date: Tue, 8 Aug 2023 20:32:59 +0000 (+0200) Subject: Fix autogenerate issue in nulls not distinct X-Git-Tag: rel_1_11_3~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1cb5f6bc7c2e387f3b02d1f5cb8c00b0434b366;p=thirdparty%2Fsqlalchemy%2Falembic.git Fix autogenerate issue in nulls not distinct Fixed issue with ``NULLS NOT DISTINCT`` detection in postgresql that would keep detecting changes in the index or unique constraint. Fixes: #1291 Change-Id: Ibb3872e8d01020fa31e85fb8aed50b84541466bf --- diff --git a/alembic/autogenerate/compare.py b/alembic/autogenerate/compare.py index c441a200..a24a75d1 100644 --- a/alembic/autogenerate/compare.py +++ b/alembic/autogenerate/compare.py @@ -287,7 +287,9 @@ _IndexColumnSortingOps: Mapping[str, Any] = util.immutabledict( ) -def _make_index(params: Dict[str, Any], conn_table: Table) -> Optional[Index]: +def _make_index( + impl: DefaultImpl, params: Dict[str, Any], conn_table: Table +) -> Optional[Index]: exprs: list[Union[Column[Any], TextClause]] = [] sorting = params.get("column_sorting") @@ -306,7 +308,11 @@ def _make_index(params: Dict[str, Any], conn_table: Table) -> Optional[Index]: item = _IndexColumnSortingOps[operator](item) exprs.append(item) ix = sa_schema.Index( - params["name"], *exprs, unique=params["unique"], _table=conn_table + params["name"], + *exprs, + unique=params["unique"], + _table=conn_table, + **impl.adjust_reflected_dialect_options(params, "index"), ) if "duplicates_constraint" in params: ix.info["duplicates_constraint"] = params["duplicates_constraint"] @@ -314,11 +320,12 @@ def _make_index(params: Dict[str, Any], conn_table: Table) -> Optional[Index]: def _make_unique_constraint( - params: Dict[str, Any], conn_table: Table + impl: DefaultImpl, params: Dict[str, Any], conn_table: Table ) -> UniqueConstraint: uq = sa_schema.UniqueConstraint( *[conn_table.c[cname] for cname in params["column_names"]], name=params["name"], + **impl.adjust_reflected_dialect_options(params, "unique_constraint"), ) if "duplicates_index" in params: uq.info["duplicates_index"] = params["duplicates_index"] @@ -532,6 +539,7 @@ def _compare_indexes_and_uniques( inspector = autogen_context.inspector is_create_table = conn_table is None is_drop_table = metadata_table is None + impl = autogen_context.migration_context.impl # 1a. get raw indexes and unique constraints from metadata ... if metadata_table is not None: @@ -603,20 +611,21 @@ def _compare_indexes_and_uniques( conn_uniques = set() # type:ignore[assignment] else: conn_uniques = { # type:ignore[assignment] - _make_unique_constraint(uq_def, conn_table) + _make_unique_constraint(impl, uq_def, conn_table) for uq_def in conn_uniques } conn_indexes = { # type:ignore[assignment] index - for index in (_make_index(ix, conn_table) for ix in conn_indexes) + for index in ( + _make_index(impl, ix, conn_table) for ix in conn_indexes + ) if index is not None } # 2a. if the dialect dupes unique indexes as unique constraints # (mysql and oracle), correct for that - impl = autogen_context.migration_context.impl if unique_constraints_duplicate_unique_indexes: _correct_for_uq_duplicates_uix( conn_uniques, diff --git a/alembic/ddl/impl.py b/alembic/ddl/impl.py index a6102d1e..21371f1a 100644 --- a/alembic/ddl/impl.py +++ b/alembic/ddl/impl.py @@ -691,6 +691,11 @@ class DefaultImpl(metaclass=ImplMeta): ) metadata_indexes.discard(idx) + def adjust_reflected_dialect_options( + self, reflected_object: Dict[str, Any], kind: str + ) -> Dict[str, Any]: + return reflected_object.get("dialect_options", {}) + def _compare_identity_options( attributes, metadata_io, inspector_io, default_io diff --git a/alembic/ddl/mssql.py b/alembic/ddl/mssql.py index e77fd1b5..dbd8de6c 100644 --- a/alembic/ddl/mssql.py +++ b/alembic/ddl/mssql.py @@ -2,6 +2,7 @@ from __future__ import annotations import re from typing import Any +from typing import Dict from typing import List from typing import Optional from typing import TYPE_CHECKING @@ -263,6 +264,17 @@ class MSSQLImpl(DefaultImpl): return diff, ignored, is_alter + def adjust_reflected_dialect_options( + self, reflected_object: Dict[str, Any], kind: str + ) -> Dict[str, Any]: + options: Dict[str, Any] + options = reflected_object.get("dialect_options", {}).copy() + if not options.get("mssql_include"): + options.pop("mssql_include", None) + if not options.get("mssql_clustered"): + options.pop("mssql_clustered", None) + return options + class _ExecDropConstraint(Executable, ClauseElement): inherit_cache = False diff --git a/alembic/ddl/postgresql.py b/alembic/ddl/postgresql.py index af0d156e..b63938ac 100644 --- a/alembic/ddl/postgresql.py +++ b/alembic/ddl/postgresql.py @@ -4,6 +4,7 @@ import logging import re from typing import Any from typing import cast +from typing import Dict from typing import List from typing import Optional from typing import Sequence @@ -310,16 +311,10 @@ class PostgresqlImpl(DefaultImpl): def _dialect_sig( self, item: Union[Index, UniqueConstraint] ) -> Tuple[Any, ...]: - if ( - item.dialect_kwargs.get("postgresql_nulls_not_distinct") - is not None - ): - return ( - ( - "nulls_not_distinct", - item.dialect_kwargs["postgresql_nulls_not_distinct"], - ), - ) + # only the positive case is returned by sqlalchemy reflection so + # None and False are threated the same + if item.dialect_kwargs.get("postgresql_nulls_not_distinct"): + return ("nulls_not_distinct",) return () def create_index_sig(self, index: Index) -> Tuple[Any, ...]: @@ -342,6 +337,15 @@ class PostgresqlImpl(DefaultImpl): sorted([col.name for col in const.columns]) ) + self._dialect_sig(const) + def adjust_reflected_dialect_options( + self, reflected_options: Dict[str, Any], kind: str + ) -> Dict[str, Any]: + options: Dict[str, Any] + options = reflected_options.get("dialect_options", {}).copy() + if not options.get("postgresql_include"): + options.pop("postgresql_include", None) + return options + def _compile_element(self, element: ClauseElement) -> str: return element.compile( dialect=self.dialect, diff --git a/docs/build/changelog.rst b/docs/build/changelog.rst index 9b114e85..f4ae2227 100644 --- a/docs/build/changelog.rst +++ b/docs/build/changelog.rst @@ -21,7 +21,7 @@ Changelog :tags: usecase, autogenerate :tickets: 1248 - Added support in autogenerate for NULLS NOT DISTINCT in + Added support in autogenerate for ``NULLS NOT DISTINCT`` in the PostgreSQL dialect. .. change:: diff --git a/docs/build/unreleased/1291.rst b/docs/build/unreleased/1291.rst new file mode 100644 index 00000000..f2f6e04b --- /dev/null +++ b/docs/build/unreleased/1291.rst @@ -0,0 +1,6 @@ +.. change:: + :tags: bug, autogenerate + :tickets: 1291 + + Fixed issue with ``NULLS NOT DISTINCT`` detection in postgresql that + would keep detecting changes in the index or unique constraint. diff --git a/tests/test_autogen_composition.py b/tests/test_autogen_composition.py index 99e5486f..aac0c346 100644 --- a/tests/test_autogen_composition.py +++ b/tests/test_autogen_composition.py @@ -1,5 +1,3 @@ -import re - from sqlalchemy import Column from sqlalchemy import Integer from sqlalchemy import MetaData @@ -34,13 +32,13 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase): autogenerate._render_migration_diffs(context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", @@ -65,13 +63,13 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase): autogenerate._render_migration_diffs(context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", @@ -83,7 +81,7 @@ class AutogenerateDiffTest(ModelOne, AutogenTest, TestBase): template_args = {} autogenerate._render_migration_diffs(self.context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.create_table('item', sa.Column('id', sa.Integer(), nullable=False), @@ -117,7 +115,7 @@ nullable=True)) ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \ nullable=True)) @@ -155,7 +153,7 @@ nullable=True)) autogenerate._render_migration_diffs(self.context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.create_table('item', sa.Column('id', sa.Integer(), nullable=False), @@ -194,7 +192,7 @@ nullable=True)) ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### with op.batch_alter_table('user', schema=None) as batch_op: batch_op.add_column(sa.Column('pw', sa.VARCHAR(length=50), nullable=True)) @@ -286,7 +284,7 @@ class AddColumnOrderTest(AutogenTest, TestBase): template_args = {} autogenerate._render_migration_diffs(self.context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + 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)) @@ -295,7 +293,7 @@ class AddColumnOrderTest(AutogenTest, TestBase): ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.drop_column('user', 'timestamp') op.drop_column('user', 'password_hash') @@ -326,13 +324,13 @@ class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestBase): autogenerate._render_migration_diffs(context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### pass # ### end Alembic commands ###""", @@ -351,7 +349,7 @@ class AutogenerateDiffTestWSchema(ModelOne, AutogenTest, TestBase): autogenerate._render_migration_diffs(self.context, template_args) eq_( - re.sub(r"u'", "'", template_args["upgrades"]), + template_args["upgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.create_table('item', sa.Column('id', sa.Integer(), nullable=False), @@ -393,7 +391,7 @@ source_schema='%(schema)s', referent_schema='%(schema)s') ) eq_( - re.sub(r"u'", "'", template_args["downgrades"]), + template_args["downgrades"], """# ### commands auto generated by Alembic - please adjust! ### op.add_column('user', sa.Column('pw', sa.VARCHAR(length=50), \ autoincrement=False, nullable=True), schema='%(schema)s') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 3a5e9838..a91d7f94 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -1447,7 +1447,13 @@ class PGUniqueIndexAutogenerateTest(AutogenFixtureTest, TestBase): eq_(len(diffs), 1) -case = combinations(False, True, None, argnames="case", id_="s") +case = combinations( + ("nulls_not_distinct=False", False), + ("nulls_not_distinct=True", True), + ("nulls_not_distinct=None", None), + argnames="case", + id_="ia", +) name_type = combinations( ( "index", @@ -1545,3 +1551,25 @@ class PGNullsNotDistinctAutogenerateTest(AutogenFixtureTest, TestBase): eq_(diffs[1][0], f"add_{name}") eq_(diffs[1][1].name, "nnd_obj") eq_(diffs[1][1].dialect_kwargs["postgresql_nulls_not_distinct"], to) + + @case + @name_type + def test_no_change(self, case, name, type_): + m1 = MetaData() + m2 = MetaData() + Table( + "tbl", + m1, + Column("id", Integer, primary_key=True), + Column("name", String), + type_(case), + ) + Table( + "tbl", + m2, + Column("id", Integer, primary_key=True), + Column("name", String), + type_(case), + ) + diffs = self._fixture(m1, m2) + eq_(len(diffs), 0, str(diffs))