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 c441a200243c0b855b0a46887debb9620abd4d0c..a24a75d1c92c14ce801717e2c687cb5b32a16239 100644 (file)
--- 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 a6102d1e3b2787c315cedf152b34ba6bf21d52b8..21371f1a58187d26d340f5f49e2f18e12b4d0aa9 100644 (file)
--- 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 e77fd1b5bb70c6309d29c3e52a7380b1883035d5..dbd8de6c000280aae410e24e0240ba0faf31c98c 100644 (file)
--- 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 af0d156ea5632625b98495ff1a878dda4b6a269e..b63938ac4617161a8c75a1912b64df88d476331b 100644 (file)
--- 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 9b114e8507240c251a24873cb6bbd7da66ac897c..f4ae2227bbd2f092fe318e825de5a36d06154332 100644 (file)
--- 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 (file)
index 0000000..f2f6e04
--- /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 99e5486ffdda1b604aed2b508cdf4163f912658b..aac0c34619ec76eb7a7a7d5d76c5e60c9ac71ca4 100644 (file)
--- 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 3a5e98380e9db77e3095d20422626b3aeb1a83eb..a91d7f9429b94b2240063aba9f18a9616fe3fb9c 100644 (file)
--- 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))