accommodate False conditions for unique / index merge

Fixed issue in ORM annotated declarative where using
:func:`_orm.mapped_column()` with an :paramref:`_orm.mapped_column.index`
or :paramref:`_orm.mapped_column.unique` setting of False would be
overridden by an incoming ``Annotated`` element that featured that
parameter set to ``True``, even though the immediate
:func:`_orm.mapped_column()` element is more specific and should take
precedence.  The logic to reconcile the booleans has been enhanced to
accommodate a local value of ``False`` as still taking precedence over an
incoming ``True`` value from the annotated element.

Fixes: #11091
Change-Id: I15cda4a0a07a289015c0a09bbe3ca2849956604e
(cherry picked from commit e4c4bd03abae2d3948f894d38992d51c9be2a8c0)

diff --git a/doc/build/changelog/unreleased_20/11091.rst b/doc/build/changelog/unreleased_20/11091.rst
new file mode 100644 (file)
index 0000000..30f2fbc
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/11091.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 11091
+
+    Fixed issue in ORM annotated declarative where using
+    :func:`_orm.mapped_column()` with an :paramref:`_orm.mapped_column.index`
+    or :paramref:`_orm.mapped_column.unique` setting of False would be
+    overridden by an incoming ``Annotated`` element that featured that
+    parameter set to ``True``, even though the immediate
+    :func:`_orm.mapped_column()` element is more specific and should take
+    precedence.  The logic to reconcile the booleans has been enhanced to
+    accommodate a local value of ``False`` as still taking precedence over an
+    incoming ``True`` value from the annotated element.

diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 2932fffad471f47a78ff96c6c3ac1d5586428978..6d5f941786aeb22afea954391e02f080bbbc7eab 100644 (file)
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -2569,8 +2569,11 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause[_T]):
             new_onupdate = self.onupdate._copy()
             new_onupdate._set_parent(other)
 
-        if self.index and not other.index:
-            other.index = True
+        if self.index in (True, False) and other.index is None:
+            other.index = self.index
+
+        if self.unique in (True, False) and other.unique is None:
+            other.unique = self.unique
 
         if self.doc and other.doc is None:
             other.doc = self.doc
@@ -2578,9 +2581,6 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause[_T]):
         if self.comment and other.comment is None:
             other.comment = self.comment
 
-        if self.unique and not other.unique:
-            other.unique = True
-
         for const in self.constraints:
             if not const._type_bound:
                 new_const = const._copy()

diff --git a/test/orm/declarative/test_tm_future_annotations_sync.py b/test/orm/declarative/test_tm_future_annotations_sync.py
index 9b55827d49931d4e06e5280d667808d897c636e5..6b118139178354ba7816997e6f044a6406281021 100644 (file)
--- a/test/orm/declarative/test_tm_future_annotations_sync.py
+++ b/test/orm/declarative/test_tm_future_annotations_sync.py
@@ -904,7 +904,9 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
         ),
         ("index", True, lambda column: column.index is True),
         ("index", _NoArg.NO_ARG, lambda column: column.index is None),
+        ("index", False, lambda column: column.index is False),
         ("unique", True, lambda column: column.unique is True),
+        ("unique", False, lambda column: column.unique is False),
         ("autoincrement", True, lambda column: column.autoincrement is True),
         ("system", True, lambda column: column.system is True),
         ("primary_key", True, lambda column: column.primary_key is True),
@@ -1062,6 +1064,32 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
                 argument,
             )
 
+    @testing.combinations(("index",), ("unique",), argnames="paramname")
+    @testing.combinations((True,), (False,), (None,), argnames="orig")
+    @testing.combinations((True,), (False,), (None,), argnames="merging")
+    def test_index_unique_combinations(
+        self, paramname, orig, merging, decl_base
+    ):
+        """test #11091"""
+
+        global myint
+
+        amc = mapped_column(**{paramname: merging})
+        myint = Annotated[int, amc]
+
+        mc = mapped_column(**{paramname: orig})
+
+        class User(decl_base):
+            __tablename__ = "user"
+            id: Mapped[int] = mapped_column(primary_key=True)
+            myname: Mapped[myint] = mc
+
+        result = getattr(User.__table__.c.myname, paramname)
+        if orig is None:
+            is_(result, merging)
+        else:
+            is_(result, orig)
+
     def test_pep484_newtypes_as_typemap_keys(
         self, decl_base: Type[DeclarativeBase]
     ):

diff --git a/test/orm/declarative/test_typed_mapping.py b/test/orm/declarative/test_typed_mapping.py
index ba8ab455ca113962d26ad14d8d4e278a610b0259..0b9f4c1acbdb085615487e73771c9a6d18ea24c5 100644 (file)
--- a/test/orm/declarative/test_typed_mapping.py
+++ b/test/orm/declarative/test_typed_mapping.py
@@ -895,7 +895,9 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
         ),
         ("index", True, lambda column: column.index is True),
         ("index", _NoArg.NO_ARG, lambda column: column.index is None),
+        ("index", False, lambda column: column.index is False),
         ("unique", True, lambda column: column.unique is True),
+        ("unique", False, lambda column: column.unique is False),
         ("autoincrement", True, lambda column: column.autoincrement is True),
         ("system", True, lambda column: column.system is True),
         ("primary_key", True, lambda column: column.primary_key is True),
@@ -1053,6 +1055,32 @@ class MappedColumnTest(fixtures.TestBase, testing.AssertsCompiledSQL):
                 argument,
             )
 
+    @testing.combinations(("index",), ("unique",), argnames="paramname")
+    @testing.combinations((True,), (False,), (None,), argnames="orig")
+    @testing.combinations((True,), (False,), (None,), argnames="merging")
+    def test_index_unique_combinations(
+        self, paramname, orig, merging, decl_base
+    ):
+        """test #11091"""
+
+        # anno only: global myint
+
+        amc = mapped_column(**{paramname: merging})
+        myint = Annotated[int, amc]
+
+        mc = mapped_column(**{paramname: orig})
+
+        class User(decl_base):
+            __tablename__ = "user"
+            id: Mapped[int] = mapped_column(primary_key=True)
+            myname: Mapped[myint] = mc
+
+        result = getattr(User.__table__.c.myname, paramname)
+        if orig is None:
+            is_(result, merging)
+        else:
+            is_(result, orig)
+
     def test_pep484_newtypes_as_typemap_keys(
         self, decl_base: Type[DeclarativeBase]
     ):

diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 8b43b0f98acddb4c7fa67616c060643d8f4dc42a..a54a5fcc8d59da42d81ae1c27ebc97cba47d2941 100644 (file)
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -4376,6 +4376,28 @@ class ColumnDefinitionTest(AssertsCompiledSQL, fixtures.TestBase):
 
         deregister(schema.CreateColumn)
 
+    @testing.combinations(("index",), ("unique",), argnames="paramname")
+    @testing.combinations((True,), (False,), (None,), argnames="orig")
+    @testing.combinations((True,), (False,), (None,), argnames="merging")
+    def test_merge_index_unique(self, paramname, orig, merging):
+        """test #11091"""
+        source = Column(**{paramname: merging})
+
+        target = Column(**{paramname: orig})
+
+        source._merge(target)
+
+        target_copy = target._copy()
+        for col in (
+            target,
+            target_copy,
+        ):
+            result = getattr(col, paramname)
+            if orig is None:
+                is_(result, merging)
+            else:
+                is_(result, orig)
+
     @testing.combinations(
         ("default", lambda ctx: 10),
         ("default", func.foo()),