Skip to content

Commit

Permalink
accommodate False conditions for unique / index merge
Browse files Browse the repository at this point in the history
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: sqlalchemy#11091
Change-Id: I15cda4a0a07a289015c0a09bbe3ca2849956604e
(cherry picked from commit e4c4bd0)
  • Loading branch information
zzzeek committed Mar 4, 2024
1 parent a4e85e7 commit ef9520e
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 5 deletions.
13 changes: 13 additions & 0 deletions doc/build/changelog/unreleased_20/11091.rst
Original file line number Diff line number Diff line change
@@ -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.
10 changes: 5 additions & 5 deletions lib/sqlalchemy/sql/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -2569,18 +2569,18 @@ def _merge(self, other: Column[Any]) -> None:
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

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()
Expand Down
28 changes: 28 additions & 0 deletions test/orm/declarative/test_tm_future_annotations_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,9 @@ def test_we_got_all_attrs_test_annotated(self):
),
("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),
Expand Down Expand Up @@ -1062,6 +1064,32 @@ class User(Base):
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]
):
Expand Down
28 changes: 28 additions & 0 deletions test/orm/declarative/test_typed_mapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,9 @@ def test_we_got_all_attrs_test_annotated(self):
),
("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),
Expand Down Expand Up @@ -1053,6 +1055,32 @@ class User(Base):
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]
):
Expand Down
22 changes: 22 additions & 0 deletions test/sql/test_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -4376,6 +4376,28 @@ def compile_(element, compiler, **kw):

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()),
Expand Down

0 comments on commit ef9520e

Please sign in to comment.