Merge "mark has()/any() as ORM; adjust criteria in WHERE without explicit FROM" into main

doc/build/changelog/unreleased_21/13070.rst

@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 13070
+
+    A significant change to the ORM mechanics involved with both
+    :func:`.orm.with_loader_criteria` as well as single table inheritance, to
+    more aggressively locate WHERE criteria which should be augmented by either
+    the custom criteria or single-table inheritance criteria; SELECT statements
+    that do not include the entity within the columns clause or as an explicit
+    FROM, but still reference the entity within the WHERE clause, are now
+    covered, in particular this will allow subqueries using ``EXISTS (SELECT
+    1)`` such as those rendered by :meth:`.RelationshipProperty.Comparator.any`
+    and :meth:`.RelationshipProperty.Comparator.has`.

lib/sqlalchemy/orm/context.py

@@ -2528,9 +2528,16 @@ class _ORMSelectCompileState(_ORMCompileState, SelectState):
         associated with the global context.
 
         """
+        ext_infos = [
+            fromclause._annotations.get("parententity", None)
+            for fromclause in self.from_clauses
+        ] + [
+            elem._annotations.get("parententity", None)
+            for where_crit in self.select_statement._where_criteria
+            for elem in sql_util.surface_expressions(where_crit)
+        ]
 
-        for fromclause in self.from_clauses:
-            ext_info = fromclause._annotations.get("parententity", None)
+        for ext_info in ext_infos:
 
             if (
                 ext_info

lib/sqlalchemy/orm/relationships.py

@@ -846,9 +846,11 @@ class RelationshipProperty(
                         where_criteria = single_crit & where_criteria
                     else:
                         where_criteria = single_crit
+                dest_entity = info
             else:
                 is_aliased_class = False
                 to_selectable = None
+                dest_entity = self.mapper
 
             if self.adapter:
                 source_selectable = self._source_selectable()
@@ -903,6 +905,18 @@ class RelationshipProperty(
 
             crit = j & sql.True_._ifnone(where_criteria)
 
+            # ensure the exists query gets picked up by the ORM
+            # compiler and that it has what we expect as parententity so that
+            # _adjust_for_extra_criteria() gets set up
+            dest = dest._annotate(
+                {
+                    "parentmapper": dest_entity.mapper,
+                    "entity_namespace": dest_entity,
+                    "parententity": dest_entity,
+                }
+            )._set_propagate_attrs(
+                {"compile_state_plugin": "orm", "plugin_subject": dest_entity}
+            )
             if secondary is not None:
                 ex = (
                     sql.exists(1)

lib/sqlalchemy/sql/util.py

@@ -468,6 +468,15 @@ def tables_from_leftmost(clause: FromClause) -> Iterator[FromClause]:
         yield clause
 
 
+def surface_expressions(clause):
+    stack = [clause]
+    while stack:
+        elem = stack.pop()
+        yield elem
+        if isinstance(elem, ColumnElement):
+            stack.extend(elem.get_children())
+
+
 def surface_selectables(clause):
     stack = [clause]
     while stack:

test/orm/inheritance/test_relationship.py

@@ -22,6 +22,7 @@ from sqlalchemy.orm import relationship
 from sqlalchemy.orm import Session
 from sqlalchemy.orm import sessionmaker
 from sqlalchemy.orm import subqueryload
+from sqlalchemy.orm import with_loader_criteria
 from sqlalchemy.orm import with_polymorphic
 from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL
 from sqlalchemy.testing import AssertsCompiledSQL
@@ -476,8 +477,10 @@ class SelfReferentialJ2JSelfTest(fixtures.MappedTest):
     def _two_obj_fixture(self):
         e1 = Engineer(name="wally")
         e2 = Engineer(name="dilbert", reports_to=e1)
+        e3 = Engineer(name="not wally")
+        e4 = Engineer(name="not dilbert", reports_to=e3)
         sess = fixture_session()
-        sess.add_all([e1, e2])
+        sess.add_all([e1, e2, e3, e4])
         sess.commit()
         return sess
 
@@ -492,10 +495,45 @@ class SelfReferentialJ2JSelfTest(fixtures.MappedTest):
 
     def test_has(self):
         sess = self._two_obj_fixture()
+
         eq_(
             sess.query(Engineer)
             .filter(Engineer.reports_to.has(Engineer.name == "wally"))
-            .first(),
+            .one(),
+            Engineer(name="dilbert"),
+        )
+
+    def test_has_w_aliased(self):
+        sess = self._two_obj_fixture()
+
+        managing_engineer = aliased(Engineer)
+
+        eq_(
+            sess.query(Engineer)
+            .filter(
+                Engineer.reports_to.of_type(managing_engineer).has(
+                    managing_engineer.name == "wally"
+                )
+            )
+            .one(),
+            Engineer(name="dilbert"),
+        )
+
+    def test_has_w_aliased_w_loader_criteria(self):
+        """test for #13070"""
+        sess = self._two_obj_fixture()
+
+        managing_engineer = aliased(Engineer)
+
+        eq_(
+            sess.query(Engineer)
+            .filter(Engineer.reports_to.of_type(managing_engineer).has())
+            .options(
+                with_loader_criteria(
+                    managing_engineer, managing_engineer.name == "wally"
+                )
+            )
+            .one(),
             Engineer(name="dilbert"),
         )
 

test/orm/inheritance/test_single.py

@@ -26,6 +26,7 @@ from sqlalchemy.orm import relationship
 from sqlalchemy.orm import selectinload
 from sqlalchemy.orm import Session
 from sqlalchemy.orm import subqueryload
+from sqlalchemy.orm import with_loader_criteria
 from sqlalchemy.orm import with_polymorphic
 from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL
 from sqlalchemy.testing import AssertsCompiledSQL
@@ -1734,7 +1735,8 @@ class RelationshipToSingleTest(
                 "WHERE employees.type IN (__[POSTCOMPILE_type_2])",
             )
 
-    def test_relationship_to_subclass(self):
+    @testing.fixture
+    def rel_to_subclass_fixture(self):
         (
             JuniorEngineer,
             Company,
@@ -1791,10 +1793,14 @@ class RelationshipToSingleTest(
         sess.add_all([c1, c2, m1, m2, e1, e2])
         sess.commit()
 
-        eq_(c1.engineers, [e2])
-        eq_(c2.engineers, [e1])
+    def test_relationship_to_subclass_one(self, rel_to_subclass_fixture):
+        Company = self.classes.Company
+        JuniorEngineer = self.classes.JuniorEngineer
+        Engineer = self.classes.Engineer
+        Employee = self.classes.Employee
+
+        sess = fixture_session()
 
-        sess.expunge_all()
         eq_(
             sess.query(Company).order_by(Company.name).all(),
             [
@@ -1854,26 +1860,70 @@ class RelationshipToSingleTest(
             [Company(name="c2")],
         )
 
-        # this however fails as it does not limit the subtypes to just
-        # "Engineer". with joins constructed by filter(), we seem to be
-        # following a policy where we don't try to make decisions on how to
-        # join to the target class, whereas when using join() we seem to have
-        # a lot more capabilities. we might want to document
-        # "advantages of join() vs. straight filtering", or add a large
-        # section to "inheritance" laying out all the various behaviors Query
-        # has.
-        @testing.fails_on_everything_except()
-        def go():
-            sess.expunge_all()
-            eq_(
-                sess.query(Company)
-                .filter(Company.company_id == Engineer.company_id)
-                .filter(Engineer.name.in_(["Tom", "Kurt"]))
-                .all(),
-                [Company(name="c2")],
-            )
+    def test_relationship_to_subclass_implicit_from(
+        self, rel_to_subclass_fixture
+    ):
+        """additional tests related to #13070"""
 
-        go()
+        Company = self.classes.Company
+        Engineer = self.classes.Engineer
+
+        sess = fixture_session()
+        eq_(
+            sess.query(Company)
+            .filter(Company.company_id == Engineer.company_id)
+            .filter(Engineer.name.in_(["Tom", "Kurt"]))
+            .all(),
+            [Company(name="c2")],
+        )
+
+    def test_relationship_to_subclass_any(self, rel_to_subclass_fixture):
+        """additional tests related to #13070.
+
+        this test is using any() with single-inh criteria, however this
+        doesnt seem to actually need anything from the #13070 change
+        as the `Engineer.name` criteria itself already includes the
+        single-inh criteria with no need for _adjust_for_extra_criteria.
+        apparently.
+
+        """
+
+        Company = self.classes.Company
+        Engineer = self.classes.Engineer
+
+        sess = fixture_session()
+        eq_(
+            sess.query(Company)
+            .filter(Company.engineers.any(Engineer.name.in_(["Tom", "Kurt"])))
+            .all(),
+            [Company(name="c2")],
+        )
+
+    def test_relationship_to_subclass_any_loader_criteria(
+        self, rel_to_subclass_fixture
+    ):
+        """additional tests related to #13070.
+
+        in this version, we put the criteria in with_loader_criteria, now
+        we need the #13070 fix for this to work.
+
+        """
+
+        Company = self.classes.Company
+        Engineer = self.classes.Engineer
+
+        sess = fixture_session()
+        eq_(
+            sess.query(Company)
+            .filter(Company.engineers.any())
+            .options(
+                with_loader_criteria(
+                    Engineer, Engineer.name.in_(["Tom", "Kurt"])
+                )
+            )
+            .all(),
+            [Company(name="c2")],
+        )
 
 
 class ManyToManyToSingleTest(fixtures.MappedTest, AssertsCompiledSQL):

test/orm/test_relationship_criteria.py

@@ -10,6 +10,7 @@ from sqlalchemy import DateTime
 from sqlalchemy import delete
 from sqlalchemy import event
 from sqlalchemy import exc as sa_exc
+from sqlalchemy import exists
 from sqlalchemy import ForeignKey
 from sqlalchemy import func
 from sqlalchemy import insert
@@ -263,6 +264,158 @@ class LoaderCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
 
     __dialect__ = "default"
 
+    @testing.combinations(
+        (
+            "one",
+            lambda Address: select(Address.user_id)
+            .where(Address.user_id == 5)
+            .options(
+                with_loader_criteria(Address, Address.email_address != "foo")
+            ),
+            (
+                "SELECT addresses.user_id FROM addresses WHERE"
+                " addresses.user_id = :user_id_1 AND addresses.email_address"
+                " != :email_address_1"
+            ),
+        ),
+        (
+            "two",
+            lambda aliased_address: select(aliased_address.user_id)
+            .where(aliased_address.user_id == 5)
+            .options(
+                with_loader_criteria(
+                    aliased_address, aliased_address.email_address != "foo"
+                )
+            ),
+            (
+                "SELECT addresses_1.user_id FROM addresses AS addresses_1"
+                " WHERE addresses_1.user_id = :user_id_1 AND"
+                " addresses_1.email_address != :email_address_1"
+            ),
+        ),
+        (
+            "three",
+            lambda Address: select(1)
+            .where(Address.user_id == 5)
+            .options(
+                with_loader_criteria(Address, Address.email_address != "foo")
+            ),
+            (
+                "SELECT 1 FROM addresses WHERE addresses.user_id = :user_id_1"
+                " AND addresses.email_address != :email_address_1"
+            ),
+        ),
+        (
+            "four",
+            lambda aliased_address: select(1)
+            .where(aliased_address.user_id == 5)
+            .options(
+                with_loader_criteria(
+                    aliased_address, aliased_address.email_address != "foo"
+                )
+            ),
+            (
+                "SELECT 1 FROM addresses AS addresses_1 WHERE"
+                " addresses_1.user_id = :user_id_1 AND"
+                " addresses_1.email_address != :email_address_1"
+            ),
+        ),
+        (
+            "five",
+            lambda User, Address: select(User)
+            .where(User.addresses.any())
+            .options(
+                with_loader_criteria(Address, Address.email_address != "foo")
+            ),
+            (
+                "SELECT users.id, users.name FROM users WHERE EXISTS (SELECT 1"
+                " FROM addresses WHERE users.id = addresses.user_id AND"
+                " addresses.email_address != :email_address_1)"
+            ),
+        ),
+        (
+            "six",
+            lambda User, aliased_address: select(User)
+            .where(User.addresses.of_type(aliased_address).any())
+            .options(
+                with_loader_criteria(
+                    aliased_address, aliased_address.email_address != "foo"
+                )
+            ),
+            (
+                "SELECT users.id, users.name FROM users WHERE EXISTS (SELECT 1"
+                " FROM addresses AS addresses_1 WHERE users.id ="
+                " addresses_1.user_id AND addresses_1.email_address !="
+                " :email_address_1)"
+            ),
+        ),
+        (
+            "seven",
+            # note plugin_subject on this one is User, not Address.
+            lambda User, Address: select(User)
+            .where(exists(1).where(User.id == Address.user_id))
+            .options(
+                with_loader_criteria(Address, Address.email_address != "foo")
+            ),
+            (
+                "SELECT users.id, users.name FROM users WHERE EXISTS (SELECT 1"
+                " FROM addresses WHERE users.id = addresses.user_id AND"
+                " addresses.email_address != :email_address_1)"
+            ),
+        ),
+        (
+            "eight",
+            lambda User, aliased_address: select(User)
+            .where(exists(1).where(User.id == aliased_address.user_id))
+            .options(
+                with_loader_criteria(
+                    aliased_address, aliased_address.email_address != "foo"
+                )
+            ),
+            (
+                "SELECT users.id, users.name FROM users WHERE EXISTS (SELECT 1"
+                " FROM addresses AS addresses_1 WHERE users.id ="
+                " addresses_1.user_id AND addresses_1.email_address !="
+                " :email_address_1)"
+            ),
+        ),
+        (
+            "nine",
+            lambda User, Address: select(User)
+            .where(exists(Address.user_id).where(User.id == Address.user_id))
+            .options(
+                with_loader_criteria(Address, Address.email_address != "foo")
+            ),
+            (
+                "SELECT users.id, users.name FROM users WHERE EXISTS (SELECT"
+                " addresses.user_id FROM addresses WHERE users.id ="
+                " addresses.user_id AND addresses.email_address !="
+                " :email_address_1)"
+            ),
+        ),
+        argnames="statement, expected",
+        id_="iaa",
+    )
+    def test_search_harder_for_criteria(
+        self, user_address_fixture, statement, expected
+    ):
+        """test #13070
+
+        loader_criteria taking effect for SELECT(1) statements with only
+        WHERE criteria, has()/any() calls, exists(1) calls
+
+        """
+        User, Address = user_address_fixture
+
+        stmt = testing.resolve_lambda(
+            statement,
+            User=User,
+            Address=Address,
+            aliased_address=aliased(Address),
+        )
+
+        self.assert_compile(stmt, expected)
+
     def test_select_mapper_mapper_criteria(self, user_address_fixture):
         User, Address = user_address_fixture
 

test/orm/test_relationships.py

@@ -6756,9 +6756,12 @@ class AnnotationsMaintainedTest(AssertsCompiledSQL, fixtures.TestBase):
             )
         else:
             # will not render "type IN <types>"
+            # note due to #13070 we had to also change the reference
+            # to Engineer.company_id which also would pull in ORM
+            # handling for the entity
             subq = (
                 select(Engineer.__table__)
-                .where(foreign(Engineer.company_id) == Company.id)
+                .where(foreign(Engineer.__table__.c.company_id) == Company.id)
                 .correlate(Company)
             )