From aaba0650d7410f579b2c14f8f1b0680a1d1852c4 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Thu, 27 May 2021 21:15:01 -0400 Subject: [PATCH] ensure relationship.order_by stored as a tuple; check in dynamic also Fixed regression in dynamic loader strategy and :func:`_orm.relationship` overall where the :paramref:`_orm.relationship.order_by` parameter were stored as a mutable list, which could then be mutated when combined with additional "order_by" methods used against the dynamic query object, causing the ORDER BY criteria to continue to grow repetitively. Fixes: #6549 Change-Id: I9f4c9a723aa0923f115cbe39bfaaa9cac62153b1 --- doc/build/changelog/unreleased_14/6549.rst | 9 ++++++ lib/sqlalchemy/orm/dynamic.py | 4 ++- lib/sqlalchemy/orm/relationships.py | 4 +-- test/orm/test_dynamic.py | 34 ++++++++++++++++++++++ 4 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 doc/build/changelog/unreleased_14/6549.rst diff --git a/doc/build/changelog/unreleased_14/6549.rst b/doc/build/changelog/unreleased_14/6549.rst new file mode 100644 index 0000000000..fbee0db3f8 --- /dev/null +++ b/doc/build/changelog/unreleased_14/6549.rst @@ -0,0 +1,9 @@ +.. change:: + :tags: bug, sql, regression + :tickets: 6549 + + Fixed regression in dynamic loader strategy and :func:`_orm.relationship` + overall where the :paramref:`_orm.relationship.order_by` parameter were + stored as a mutable list, which could then be mutated when combined with + additional "order_by" methods used against the dynamic query object, + causing the ORDER BY criteria to continue to grow repetitively. diff --git a/lib/sqlalchemy/orm/dynamic.py b/lib/sqlalchemy/orm/dynamic.py index ac7eba03b8..5cc00dbffd 100644 --- a/lib/sqlalchemy/orm/dynamic.py +++ b/lib/sqlalchemy/orm/dynamic.py @@ -66,6 +66,7 @@ class DynamicAttributeImpl(attributes.AttributeImpl): supports_population = False collection = False dynamic = True + order_by = () def __init__( self, @@ -82,7 +83,8 @@ class DynamicAttributeImpl(attributes.AttributeImpl): class_, key, typecallable, dispatch, **kw ) self.target_mapper = target_mapper - self.order_by = order_by + if order_by: + self.order_by = tuple(order_by) if not query_class: self.query_class = AppenderQuery elif AppenderMixin in query_class.mro(): diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index 00a14b04ca..efdd7edf10 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -2208,12 +2208,12 @@ class RelationshipProperty(StrategizedProperty): # ensure expressions in self.order_by, foreign_keys, # remote_side are all columns, not strings. if self.order_by is not False and self.order_by is not None: - self.order_by = [ + self.order_by = tuple( coercions.expect( roles.ColumnArgumentRole, x, argname="order_by" ) for x in util.to_list(self.order_by) - ] + ) self._user_defined_foreign_keys = util.column_set( coercions.expect( diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py index e87b5a3636..68547b1539 100644 --- a/test/orm/test_dynamic.py +++ b/test/orm/test_dynamic.py @@ -343,6 +343,40 @@ class DynamicTest(_DynamicFixture, _fixtures.FixtureTest, AssertsCompiledSQL): ], ) + def test_order_by_composition_uses_immutable_tuple(self): + addresses = self.tables.addresses + User, Address = self._user_address_fixture( + addresses_args={"order_by": addresses.c.email_address.desc()} + ) + + sess = fixture_session() + u = sess.query(User).get(8) + + with self.sql_execution_asserter() as asserter: + for i in range(3): + eq_( + list(u.addresses.order_by(desc(Address.email_address))), + [ + Address(email_address="ed@wood.com"), + Address(email_address="ed@lala.com"), + Address(email_address="ed@bettyboop.com"), + ], + ) + asserter.assert_( + *[ + CompiledSQL( + "SELECT addresses.id AS addresses_id, addresses.user_id " + "AS addresses_user_id, addresses.email_address " + "AS addresses_email_address FROM addresses " + "WHERE :param_1 = addresses.user_id " + "ORDER BY addresses.email_address DESC, " + "addresses.email_address DESC", + [{"param_1": 8}], + ) + for i in range(3) + ] + ) + def test_configured_order_by(self): addresses = self.tables.addresses User, Address = self._user_address_fixture( -- 2.47.3