Include GROUP BY in _should_nest_selectable criteria

Fixed bug where usage of joined eager loading would not properly wrap the
query inside of a subquery when :meth:`.Query.group_by` were used against
the query.   When any kind of result-limiting approach is used, such as
DISTINCT, LIMIT, OFFSET, joined eager loading embeds the row-limited query
inside of a subquery so that the collection results are not impacted.   For
some reason, the presence of GROUP BY was never included in this criterion,
even though it has a similar effect as using DISTINCT.   Additionally, the
bug would prevent using GROUP BY at all for a joined eager load query for
most database platforms which forbid non-aggregated, non-grouped columns
from being in the query, as the additional columns for the joined eager
load would not be accepted by the database.

Fixes: #5065
Change-Id: I9a2ed8196f83297ec38012138d1a5acdf9e88155
(cherry picked from commit 2d5fa22c7d53ff8109d47ba5ae4fe3b9849ddd09)

diff --git a/doc/build/changelog/unreleased_13/5065.rst b/doc/build/changelog/unreleased_13/5065.rst
new file mode 100644 (file)
index 0000000..256f641
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5065.rst
@@ -0,0 +1,17 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 5065
+
+    Fixed bug where usage of joined eager loading would not properly wrap the
+    query inside of a subquery when :meth:`.Query.group_by` were used against
+    the query.   When any kind of result-limiting approach is used, such as
+    DISTINCT, LIMIT, OFFSET, joined eager loading embeds the row-limited query
+    inside of a subquery so that the collection results are not impacted.   For
+    some reason, the presence of GROUP BY was never included in this criterion,
+    even though it has a similar effect as using DISTINCT.   Additionally, the
+    bug would prevent using GROUP BY at all for a joined eager load query for
+    most database platforms which forbid non-aggregated, non-grouped columns
+    from being in the query, as the additional columns for the joined eager
+    load would not be accepted by the database.
+
+

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 35b854e79a56400bf4ef1864bc22fc5c6c152661..d99efe48481cf32879033f37cd777526eab18fdb 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -3529,6 +3529,7 @@ class Query(object):
             kwargs.get("limit") is not None
             or kwargs.get("offset") is not None
             or kwargs.get("distinct", False)
+            or kwargs.get("group_by", False)
         )
 
     def exists(self):

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 8f9b662f02f6f2243287552a7bb4bae72a49514c..56beb8c1c4d4327869cbedf97e29fd04d644ff6f 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -1109,6 +1109,46 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
 
         self.assert_sql_count(testing.db, go, 1)
 
+    def test_group_by_only(self):
+        # like distinct(), a group_by() has a similar effect so the
+        # joined eager load needs to subquery for this as well
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        mapper(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    mapper(Address, addresses),
+                    lazy="joined",
+                    order_by=addresses.c.email_address,
+                )
+            },
+        )
+
+        q = create_session().query(User)
+        eq_(
+            [
+                User(id=7, addresses=[Address(id=1)]),
+                User(
+                    id=8,
+                    addresses=[
+                        Address(id=3, email_address="ed@bettyboop.com"),
+                        Address(id=4, email_address="ed@lala.com"),
+                        Address(id=2, email_address="ed@wood.com"),
+                    ],
+                ),
+                User(id=9, addresses=[Address(id=5)]),
+                User(id=10, addresses=[]),
+            ],
+            q.order_by(User.id).group_by(User).all(),  # group by all columns
+        )
+
     def test_limit_2(self):
         keywords, items, item_keywords, Keyword, Item = (
             self.tables.keywords,

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index 8faefd3c9594b25ec6d1a4111e7e89f36b503212..1c2f720467e0775d3adcc6c7c0c6fc3c8cb06f35 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -1113,6 +1113,45 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
         result = q.order_by(sa.desc(User.id)).limit(2).offset(2).all()
         eq_(list(reversed(self.static.user_all_result[0:2])), result)
 
+    def test_group_by_only(self):
+        # test group_by() not impacting results, similarly to joinedload
+        users, Address, addresses, User = (
+            self.tables.users,
+            self.classes.Address,
+            self.tables.addresses,
+            self.classes.User,
+        )
+
+        mapper(
+            User,
+            users,
+            properties={
+                "addresses": relationship(
+                    mapper(Address, addresses),
+                    lazy="subquery",
+                    order_by=addresses.c.email_address,
+                )
+            },
+        )
+
+        q = create_session().query(User)
+        eq_(
+            [
+                User(id=7, addresses=[Address(id=1)]),
+                User(
+                    id=8,
+                    addresses=[
+                        Address(id=3, email_address="ed@bettyboop.com"),
+                        Address(id=4, email_address="ed@lala.com"),
+                        Address(id=2, email_address="ed@wood.com"),
+                    ],
+                ),
+                User(id=9, addresses=[Address(id=5)]),
+                User(id=10, addresses=[]),
+            ],
+            q.order_by(User.id).group_by(User).all(),  # group by all columns
+        )
+
     def test_one_to_many_scalar(self):
         Address, addresses, users, User = (
             self.classes.Address,