From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 19 Sep 2023 12:58:52 +0000 (-0400)
Subject: Ensure loader criteria used for ORM join with expression condition
X-Git-Tag: rel_1_4_52~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f3e8d08d6124440bc37f9799adb8caad71e63f1;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Ensure loader criteria used for ORM join with expression condition

Fixed bug where ORM :func:`_orm.with_loader_criteria` would not apply
itself to a :meth:`_sql.Select.join` where the ON clause were given as a
plain SQL comparison, rather than as a relationship target or similar.

Fixes: #10365
Change-Id: Ie6d08fb01a3079b7c3ccd3a8241031d46a56e19d
(cherry picked from commit f8086a809bba358790cff032d745814b186ab8cb)
---

diff --git a/doc/build/changelog/unreleased_14/10365.rst b/doc/build/changelog/unreleased_14/10365.rst
new file mode 100644
index 0000000000..5eb4f44065
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/10365.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 10365
+
+    Fixed bug where ORM :func:`_orm.with_loader_criteria` would not apply
+    itself to a :meth:`_sql.Select.join` where the ON clause were given as a
+    plain SQL comparison, rather than as a relationship target or similar.
+
+    This is a backport of the same issue fixed in version 2.0 for 2.0.22.
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index a296fc0c17..28bf5b76c8 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -1737,9 +1737,8 @@ class _ORMJoin(expression.Join):
         else:
             prop = None
 
+        left_selectable = left_info.selectable
         if prop:
-            left_selectable = left_info.selectable
-
             if sql_util.clause_is_present(on_selectable, left_selectable):
                 adapt_from = on_selectable
             else:
@@ -1774,25 +1773,25 @@ class _ORMJoin(expression.Join):
 
             self._target_adapter = target_adapter
 
-            # we don't use the normal coercions logic for _ORMJoin
-            # (probably should), so do some gymnastics to get the entity.
-            # logic here is for #8721, which was a major bug in 1.4
-            # for almost two years, not reported/fixed until 1.4.43 (!)
-            if left_info.is_selectable:
-                parententity = left_selectable._annotations.get(
-                    "parententity", None
-                )
-            elif left_info.is_mapper or left_info.is_aliased_class:
-                parententity = left_info
-            else:
-                parententity = None
+        # we don't use the normal coercions logic for _ORMJoin
+        # (probably should), so do some gymnastics to get the entity.
+        # logic here is for #8721, which was a major bug in 1.4
+        # for almost two years, not reported/fixed until 1.4.43 (!)
+        if left_info.is_selectable:
+            parententity = left_selectable._annotations.get(
+                "parententity", None
+            )
+        elif left_info.is_mapper or left_info.is_aliased_class:
+            parententity = left_info
+        else:
+            parententity = None
 
-            if parententity is not None:
-                self._annotations = self._annotations.union(
-                    {"parententity": parententity}
-                )
+        if parententity is not None:
+            self._annotations = self._annotations.union(
+                {"parententity": parententity}
+            )
 
-        augment_onclause = onclause is None and _extra_criteria
+        augment_onclause = bool(_extra_criteria) and not prop
         expression.Join.__init__(self, left, right, onclause, isouter, full)
 
         if augment_onclause:
diff --git a/setup.cfg b/setup.cfg
index 852089a99b..c11b40cbbf 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -114,6 +114,8 @@ ignore =
     N801,N802,N806,
     RST304,RST303,RST299,RST399,
     W503,W504
+    U100
+    IS001
 exclude = .venv,.git,.tox,dist,doc,*egg,build
 import-order-style = google
 application-import-names = sqlalchemy,test
diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py
index d93f1fc8f3..82ad752c44 100644
--- a/test/orm/test_relationship_criteria.py
+++ b/test/orm/test_relationship_criteria.py
@@ -306,6 +306,13 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
             .join(User.addresses)
             .options(with_loader_criteria(User, User.name != "name")),
         ),
+        (
+            # issue #10365
+            lambda User, Address: select(Address)
+            .select_from(User)
+            .join(Address, User.id == Address.user_id)
+            .options(with_loader_criteria(User, User.name != "name")),
+        ),
         (
             lambda User, Address: select(Address)
             .select_from(orm_join(User, Address, User.addresses))
@@ -354,6 +361,13 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
             .join(User.addresses)
             .options(with_loader_criteria(User, User.name != "name")),
         ),
+        (
+            # issue #10365 - this seems to have already worked
+            lambda User, Address: select(Address.id, User.id)
+            .select_from(User)
+            .join(Address, User.id == Address.user_id)
+            .options(with_loader_criteria(User, User.name != "name")),
+        ),
         (
             lambda User, Address: select(Address.id, User.id)
             .select_from(orm_join(User, Address, User.addresses))
@@ -403,6 +417,15 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
                 with_loader_criteria(Address, Address.email_address != "email")
             ),
         ),
+        (
+            # issue #10365
+            lambda User, Address: select(Address)
+            .select_from(User)
+            .join(Address, User.id == Address.user_id)
+            .options(
+                with_loader_criteria(Address, Address.email_address != "email")
+            ),
+        ),
         (
             # for orm_join(), this is set up before we have the context
             # available that allows with_loader_criteria to be set up