From: Mike Bayer Date: Wed, 14 Feb 2024 14:29:19 +0000 (-0500) Subject: ensure secondary cols not excluded from adaption X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8844cb0b4148ff52c0377edf01d6e88f3bbe1ab0;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git ensure secondary cols not excluded from adaption Fixed regression caused by :ticket:`9779` where using the "secondary" table in a relationship ``and_()`` expression would fail to be aliased to match how the "secondary" table normally renders within a :meth:`_sql.Select.join` expression, leading to an invalid query. Fixes: #11010 Change-Id: I535ce8b14f6a779c26b6b50b796ce64e57d7ee3d --- diff --git a/doc/build/changelog/unreleased_20/11010.rst b/doc/build/changelog/unreleased_20/11010.rst new file mode 100644 index 0000000000..bd24772dd6 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11010.rst @@ -0,0 +1,8 @@ +.. change:: + :tags: bug, orm, regression + :tickets: 11010 + + Fixed regression caused by :ticket:`9779` where using the "secondary" table + in a relationship ``and_()`` expression would fail to be aliased to match + how the "secondary" table normally renders within a + :meth:`_sql.Select.join` expression, leading to an invalid query. diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 5de886f79b..383bf24d45 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -19,6 +19,7 @@ import collections from collections import abc import dataclasses import inspect as _py_inspect +import itertools import re import typing from typing import Any @@ -26,6 +27,7 @@ from typing import Callable from typing import cast from typing import Collection from typing import Dict +from typing import FrozenSet from typing import Generic from typing import Iterable from typing import Iterator @@ -3288,6 +3290,15 @@ class JoinCondition: if annotation_set.issubset(col._annotations) } + @util.memoized_property + def _secondary_lineage_set(self) -> FrozenSet[ColumnElement[Any]]: + if self.secondary is not None: + return frozenset( + itertools.chain(*[c.proxy_set for c in self.secondary.c]) + ) + else: + return util.EMPTY_SET + def join_targets( self, source_selectable: Optional[FromClause], @@ -3338,23 +3349,25 @@ class JoinCondition: if extra_criteria: - def mark_unrelated_columns_as_ok_to_adapt( + def mark_exclude_cols( elem: SupportsAnnotations, annotations: _AnnotationDict ) -> SupportsAnnotations: - """note unrelated columns in the "extra criteria" as OK - to adapt, even though they are not part of our "local" - or "remote" side. + """note unrelated columns in the "extra criteria" as either + should be adapted or not adapted, even though they are not + part of our "local" or "remote" side. - see #9779 for this case + see #9779 for this case, as well as #11010 for a follow up """ parentmapper_for_element = elem._annotations.get( "parentmapper", None ) + if ( parentmapper_for_element is not self.prop.parent and parentmapper_for_element is not self.prop.mapper + and elem not in self._secondary_lineage_set ): return _safe_annotate(elem, annotations) else: @@ -3363,8 +3376,8 @@ class JoinCondition: extra_criteria = tuple( _deep_annotate( elem, - {"ok_to_adapt_in_join_condition": True}, - annotate_callable=mark_unrelated_columns_as_ok_to_adapt, + {"should_not_adapt": True}, + annotate_callable=mark_exclude_cols, ) for elem in extra_criteria ) @@ -3378,14 +3391,16 @@ class JoinCondition: if secondary is not None: secondary = secondary._anonymous_fromclause(flat=True) primary_aliasizer = ClauseAdapter( - secondary, exclude_fn=_ColInAnnotations("local") + secondary, + exclude_fn=_local_col_exclude, ) secondary_aliasizer = ClauseAdapter( dest_selectable, equivalents=self.child_equivalents ).chain(primary_aliasizer) if source_selectable is not None: primary_aliasizer = ClauseAdapter( - secondary, exclude_fn=_ColInAnnotations("local") + secondary, + exclude_fn=_local_col_exclude, ).chain( ClauseAdapter( source_selectable, @@ -3397,14 +3412,14 @@ class JoinCondition: else: primary_aliasizer = ClauseAdapter( dest_selectable, - exclude_fn=_ColInAnnotations("local"), + exclude_fn=_local_col_exclude, equivalents=self.child_equivalents, ) if source_selectable is not None: primary_aliasizer.chain( ClauseAdapter( source_selectable, - exclude_fn=_ColInAnnotations("remote"), + exclude_fn=_remote_col_exclude, equivalents=self.parent_equivalents, ) ) @@ -3483,18 +3498,24 @@ class JoinCondition: class _ColInAnnotations: - """Serializable object that tests for a name in c._annotations.""" + """Serializable object that tests for names in c._annotations. - __slots__ = ("name",) + TODO: does this need to be serializable anymore? can we find what the + use case was for that? - def __init__(self, name: str): - self.name = name + """ + + __slots__ = ("names",) + + def __init__(self, *names: str): + self.names = frozenset(names) def __call__(self, c: ClauseElement) -> bool: - return ( - self.name in c._annotations - or "ok_to_adapt_in_join_condition" in c._annotations - ) + return bool(self.names.intersection(c._annotations)) + + +_local_col_exclude = _ColInAnnotations("local", "should_not_adapt") +_remote_col_exclude = _ColInAnnotations("remote", "should_not_adapt") class Relationship( # type: ignore diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py index 69279f6004..4add92c1e7 100644 --- a/test/orm/test_relationship_criteria.py +++ b/test/orm/test_relationship_criteria.py @@ -2409,6 +2409,28 @@ class RelationshipCriteriaTest(_Fixtures, testing.AssertsCompiledSQL): "AND items_1.description != :description_1", ) + def test_use_secondary_table_in_criteria(self, order_item_fixture): + """test #11010 , regression caused by #9779""" + + Order, Item = order_item_fixture + order_items = self.tables.order_items + + stmt = select(Order).join( + Order.items.and_( + order_items.c.item_id > 1, Item.description != "description" + ) + ) + + self.assert_compile( + stmt, + "SELECT orders.id, orders.user_id, orders.address_id, " + "orders.description, orders.isopen FROM orders JOIN order_items " + "AS order_items_1 ON orders.id = order_items_1.order_id " + "JOIN items ON items.id = order_items_1.item_id " + "AND order_items_1.item_id > :item_id_1 " + "AND items.description != :description_1", + ) + class SubqueryCriteriaTest(fixtures.DeclarativeMappedTest): """test #10223"""