From: Mike Bayer Date: Thu, 8 Jan 2026 21:37:38 +0000 (-0500) Subject: mark has()/any() as ORM; adjust criteria in WHERE without explicit FROM X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5491f251b4db55b8d60455ed56b36482e452490e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git mark has()/any() as ORM; adjust criteria in WHERE without explicit FROM 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`. Fixes: #13070 Change-Id: I9afb48e1c95b9e61f4e1b6be8e4a634ed5b1df43 --- diff --git a/doc/build/changelog/unreleased_21/13070.rst b/doc/build/changelog/unreleased_21/13070.rst new file mode 100644 index 0000000000..4444261630 --- /dev/null +++ b/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`. diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py index 03366bc9eb..a25bcb1f76 100644 --- a/lib/sqlalchemy/orm/context.py +++ b/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 diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index e385d08ea0..c4a9256dd8 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/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) diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py index 9736b1ce70..59de4a9176 100644 --- a/lib/sqlalchemy/sql/util.py +++ b/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: diff --git a/test/orm/inheritance/test_relationship.py b/test/orm/inheritance/test_relationship.py index f97e9248d4..83f74bd60d 100644 --- a/test/orm/inheritance/test_relationship.py +++ b/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"), ) diff --git a/test/orm/inheritance/test_single.py b/test/orm/inheritance/test_single.py index a26aed1cf1..dc35db7987 100644 --- a/test/orm/inheritance/test_single.py +++ b/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""" + + 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. + + """ - go() + 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): diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py index e22a4c8ca2..cd499a37a3 100644 --- a/test/orm/test_relationship_criteria.py +++ b/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 diff --git a/test/orm/test_relationships.py b/test/orm/test_relationships.py index f610860d5d..9bd97dd16f 100644 --- a/test/orm/test_relationships.py +++ b/test/orm/test_relationships.py @@ -6756,9 +6756,12 @@ class AnnotationsMaintainedTest(AssertsCompiledSQL, fixtures.TestBase): ) else: # will not render "type IN " + # 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) )