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

diff --git a/doc/build/changelog/unreleased_14/6549.rst b/doc/build/changelog/unreleased_14/6549.rst
new file mode 100644 (file)
index 0000000..fbee0db
--- /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 ac7eba03b8bfde0f9575730cf3b3d8617e58d8af..5cc00dbffd0d6374bb01442e50182cf15a1dc0f3 100644 (file)
--- 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 00a14b04ca73c381d950a7fd8afccf6f187c04fb..efdd7edf101616afe41b6588904fdb294f7c4294 100644 (file)
--- 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 e87b5a363611d2c82ef31346234f9851d53094ce..68547b1539d3abadc6a3794b18ddf03b41edc132 100644 (file)
--- 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(