From ba8efc59445b41dc6a28ffb1f8b5dace57be8c33 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Sun, 21 Sep 2025 10:59:51 -0400 Subject: [PATCH] Apply nested for joined eager m2o w/ GROUP BY, DISTINCT Fixed issue where joined eager loading would fail to use the "nested" form of the query when GROUP BY or DISTINCT were present if the eager joins being added were many-to-ones, leading to additional columns in the columns clause which would then cause errors. The check for "nested" is tuned to be enabled for these queries even for many-to-one joined eager loaders, and the "only do nested if it's one to many" aspect is now localized to when the query only has LIMIT or OFFSET added. Fixes: #11226 Change-Id: I0b4b71cacfe1c6a25d0924e0954ceced1e399ca1 --- doc/build/changelog/unreleased_21/11226.rst | 11 +++ lib/sqlalchemy/orm/context.py | 20 +++-- lib/sqlalchemy/orm/strategies.py | 5 +- test/orm/test_eager_relations.py | 83 +++++++++++++++++++++ 4 files changed, 108 insertions(+), 11 deletions(-) create mode 100644 doc/build/changelog/unreleased_21/11226.rst diff --git a/doc/build/changelog/unreleased_21/11226.rst b/doc/build/changelog/unreleased_21/11226.rst new file mode 100644 index 0000000000..11e871ed31 --- /dev/null +++ b/doc/build/changelog/unreleased_21/11226.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, orm + :tickets: 11226 + + Fixed issue where joined eager loading would fail to use the "nested" form + of the query when GROUP BY or DISTINCT were present if the eager joins + being added were many-to-ones, leading to additional columns in the columns + clause which would then cause errors. The check for "nested" is tuned to + be enabled for these queries even for many-to-one joined eager loaders, and + the "only do nested if it's one to many" aspect is now localized to when + the query only has LIMIT or OFFSET added. diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py index 15a9d2a786..e906fcbc0f 100644 --- a/lib/sqlalchemy/orm/context.py +++ b/lib/sqlalchemy/orm/context.py @@ -1405,11 +1405,7 @@ class _ORMSelectCompileState(_ORMCompileState, SelectState): if self.order_by is False: self.order_by = None - if ( - self.multi_row_eager_loaders - and self.eager_adding_joins - and self._should_nest_selectable - ): + if self._should_nest_selectable: self.statement = self._compound_eager_statement() else: self.statement = self._simple_statement() @@ -2431,9 +2427,19 @@ class _ORMSelectCompileState(_ORMCompileState, SelectState): @property def _should_nest_selectable(self): kwargs = self._select_args + + if not self.eager_adding_joins: + return False + return ( - kwargs.get("limit_clause") is not None - or kwargs.get("offset_clause") is not None + ( + kwargs.get("limit_clause") is not None + and self.multi_row_eager_loaders + ) + or ( + kwargs.get("offset_clause") is not None + and self.multi_row_eager_loaders + ) or kwargs.get("distinct", False) or kwargs.get("distinct_on", ()) or kwargs.get("group_by", False) diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 8af9c02038..cd0d97598f 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -2448,10 +2448,7 @@ class _JoinedLoader(_AbstractRelationshipLoader): # whether or not the Query will wrap the selectable in a subquery, # and then attach eager load joins to that (i.e., in the case of # LIMIT/OFFSET etc.) - should_nest_selectable = ( - compile_state.multi_row_eager_loaders - and compile_state._should_nest_selectable - ) + should_nest_selectable = compile_state._should_nest_selectable query_entity_key = None diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py index 7e0eca62c6..80178582c6 100644 --- a/test/orm/test_eager_relations.py +++ b/test/orm/test_eager_relations.py @@ -3133,6 +3133,89 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): ], ) + @testing.fixture + def issue_11226_fixture(self): + users, items, order_items, Order, Item, User, orders = ( + self.tables.users, + self.tables.items, + self.tables.order_items, + self.classes.Order, + self.classes.Item, + self.classes.User, + self.tables.orders, + ) + + self.mapper_registry.map_imperatively( + User, + users, + ) + self.mapper_registry.map_imperatively( + Order, + orders, + properties=dict( + items=relationship( + Item, secondary=order_items, order_by=items.c.id + ), + user=relationship(User), + ), + ) + self.mapper_registry.map_imperatively(Item, items) + + def test_nested_for_group_by(self, issue_11226_fixture): + """test issue #11226""" + + Order, Item = self.classes("Order", "Item") + + stmt = ( + select(Order, func.count(Item.id)) + .join(Order.items) + .group_by(Order.id) + .options(joinedload(Order.user)) + ) + + # the query has a many-to-one joinedload, but also a GROUP BY. + # eager loading needs to use nested form so that the eager joins + # can be added to the outside of the GROUP BY query. + # change #11226 liberalizes the conditions where we do nested form + # to include non-multi-row eager loads, when the columns list is + # otherwise sensitive to more columns being added. + self.assert_compile( + stmt, + "SELECT anon_1.id, anon_1.user_id, anon_1.address_id, " + "anon_1.description, anon_1.isopen, anon_1.count_1, " + "users_1.id AS id_1, users_1.name " + "FROM (SELECT orders.id AS id, orders.user_id AS user_id, " + "orders.address_id AS address_id, " + "orders.description AS description, orders.isopen AS isopen, " + "count(items.id) AS count_1 " + "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 " + "GROUP BY orders.id) " + "AS anon_1 " + "LEFT OUTER JOIN users AS users_1 ON users_1.id = anon_1.user_id", + ) + + def test_nested_for_distinct(self, issue_11226_fixture): + """test issue #11226""" + + Order, Item = self.classes("Order", "Item") + + stmt = select(Order).distinct().options(joinedload(Order.user)) + + self.assert_compile( + stmt, + "SELECT anon_1.id, anon_1.user_id, anon_1.address_id, " + "anon_1.description, anon_1.isopen, " + "users_1.id AS id_1, users_1.name " + "FROM (SELECT DISTINCT orders.id AS id, " + "orders.user_id AS user_id, orders.address_id AS address_id, " + "orders.description AS description, orders.isopen AS isopen " + "FROM orders) AS anon_1 " + "LEFT OUTER JOIN users AS users_1 ON users_1.id = anon_1.user_id", + ) + class SelectUniqueTest(_fixtures.FixtureTest): run_inserts = "once" -- 2.47.3