From 7ba024cabaccc071c8a515253114d08070c626bd Mon Sep 17 00:00:00 2001 From: Lumir Balhar Date: Mon, 4 Dec 2023 14:00:03 +0100 Subject: [PATCH] Fix GROUP BY for joined relationships Resolves: RHEL-17988 --- python-sqlalchemy.spec | 12 +- ...hemy-1.4.0-group_by_joined_relations.patch | 143 ++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 sqlalchemy-1.4.0-group_by_joined_relations.patch diff --git a/python-sqlalchemy.spec b/python-sqlalchemy.spec index 941064c..cd9ffba 100644 --- a/python-sqlalchemy.spec +++ b/python-sqlalchemy.spec @@ -13,7 +13,7 @@ Name: python-sqlalchemy Version: 1.2.7 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Modular and flexible ORM library for python Group: Development/Libraries @@ -25,6 +25,11 @@ Source0: https://files.pythonhosted.org/packages/source/S/%{srcname}/%{sr # https://github.com/zzzeek/sqlalchemy/pull/452 Patch1: python-sqlalchemy-1.2.8-sqlite-3.24.patch +# Include GROUP BY in _should_nest_selectable criteria +# Upstream issue: https://github.com/sqlalchemy/sqlalchemy/issues/5065 +# Backported from upstream change: https://github.com/sqlalchemy/sqlalchemy/commit/2d5fa22c7d53ff8109d47ba5ae4fe3b9849ddd09 +Patch2: sqlalchemy-1.4.0-group_by_joined_relations.patch + BuildRequires: gcc %if %{with python2} @@ -99,6 +104,7 @@ This package includes the python 3 version of the module. %setup -q -n %{srcname}-%{version} %patch1 -p1 -b .sqlite-3.24 +%patch2 -p1 -b .nest_selectable_crit %build %if %{with python2} @@ -149,6 +155,10 @@ PYTHONPATH=. %{__python3} -m pytest test %endif # with_python3 %changelog +* Mon Dec 04 2023 Lumír Balhar - 1.2.7-4 +- Fix GROUP BY for joined relationships +Resolves: RHEL-17988 + * Wed Aug 01 2018 Petr Viktorin - 1.2.7-3 - Fix tests with SQLite 3.24+ diff --git a/sqlalchemy-1.4.0-group_by_joined_relations.patch b/sqlalchemy-1.4.0-group_by_joined_relations.patch new file mode 100644 index 0000000..d2dca5c --- /dev/null +++ b/sqlalchemy-1.4.0-group_by_joined_relations.patch @@ -0,0 +1,143 @@ +From 2c71a8d17df6671fa0cde33d643c993a4317536d Mon Sep 17 00:00:00 2001 +From: Lumir Balhar +Date: Mon, 4 Dec 2023 13:33:29 +0100 +Subject: [PATCH] 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 +--- + lib/sqlalchemy/orm/query.py | 3 ++- + test/orm/test_eager_relations.py | 40 +++++++++++++++++++++++++++++ + test/orm/test_subquery_relations.py | 39 ++++++++++++++++++++++++++++ + 3 files changed, 81 insertions(+), 1 deletion(-) + +diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py +index a17f590..bd0081f 100644 +--- a/lib/sqlalchemy/orm/query.py ++++ b/lib/sqlalchemy/orm/query.py +@@ -3076,7 +3076,8 @@ class Query(object): + kwargs = self._select_args + return (kwargs.get('limit') is not None or + kwargs.get('offset') is not None or +- kwargs.get('distinct', False)) ++ kwargs.get('distinct', False) or ++ kwargs.get("group_by", False)) + + def exists(self): + """A convenience method that turns a query into an EXISTS subquery +diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py +index 3c669d9..9aeeca5 100644 +--- a/test/orm/test_eager_relations.py ++++ b/test/orm/test_eager_relations.py +@@ -747,6 +747,46 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): + eq_(self.static.user_address_result, result) + 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 606beb5..aef9c34 100644 +--- a/test/orm/test_subquery_relations.py ++++ b/test/orm/test_subquery_relations.py +@@ -754,6 +754,45 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL): + result = q.limit(2).all() + eq_(result, list(reversed(self.static.user_address_result[2:4]))) + ++ 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, + self.tables.addresses, +-- +2.43.0 +