Merge "ensure whereclause, returning copied as tuples" into main

4 files changed, 78 insertions, 16 deletions

diff --git a/doc/build/changelog/unreleased_14/9033.rst b/doc/build/changelog/unreleased_14/9033.rst
new file mode 100644
index 000000000..d0b0d2f3f
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/9033.rst
@@ -0,0 +1,8 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 9033
+
+    Fixed issue in the internal SQL traversal for DML statements like
+    :class:`_dml.Update` and :class:`_dml.Delete` which would cause among other
+    potential issues, a specific issue using lambda statements with the ORM
+    update/delete feature.
diff --git a/lib/sqlalchemy/sql/dml.py b/lib/sqlalchemy/sql/dml.py
index c279e344b..01776e8fc 100644
--- a/lib/sqlalchemy/sql/dml.py
+++ b/lib/sqlalchemy/sql/dml.py
@@ -1164,12 +1164,12 @@ class Insert(ValuesBase):
             ("_multi_values", InternalTraversal.dp_dml_multi_values),
             ("select", InternalTraversal.dp_clauseelement),
             ("_post_values_clause", InternalTraversal.dp_clauseelement),
-            ("_returning", InternalTraversal.dp_clauseelement_list),
+            ("_returning", InternalTraversal.dp_clauseelement_tuple),
             ("_hints", InternalTraversal.dp_table_hint_list),
             ("_return_defaults", InternalTraversal.dp_boolean),
             (
                 "_return_defaults_columns",
-                InternalTraversal.dp_clauseelement_list,
+                InternalTraversal.dp_clauseelement_tuple,
             ),
         ]
         + HasPrefixes._has_prefixes_traverse_internals
@@ -1479,16 +1479,16 @@ class Update(DMLWhereBase, ValuesBase):
     _traverse_internals = (
         [
             ("table", InternalTraversal.dp_clauseelement),
-            ("_where_criteria", InternalTraversal.dp_clauseelement_list),
+            ("_where_criteria", InternalTraversal.dp_clauseelement_tuple),
             ("_inline", InternalTraversal.dp_boolean),
             ("_ordered_values", InternalTraversal.dp_dml_ordered_values),
             ("_values", InternalTraversal.dp_dml_values),
-            ("_returning", InternalTraversal.dp_clauseelement_list),
+            ("_returning", InternalTraversal.dp_clauseelement_tuple),
             ("_hints", InternalTraversal.dp_table_hint_list),
             ("_return_defaults", InternalTraversal.dp_boolean),
             (
                 "_return_defaults_columns",
-                InternalTraversal.dp_clauseelement_list,
+                InternalTraversal.dp_clauseelement_tuple,
             ),
         ]
         + HasPrefixes._has_prefixes_traverse_internals
@@ -1684,8 +1684,8 @@ class Delete(DMLWhereBase, UpdateBase):
     _traverse_internals = (
         [
             ("table", InternalTraversal.dp_clauseelement),
-            ("_where_criteria", InternalTraversal.dp_clauseelement_list),
-            ("_returning", InternalTraversal.dp_clauseelement_list),
+            ("_where_criteria", InternalTraversal.dp_clauseelement_tuple),
+            ("_returning", InternalTraversal.dp_clauseelement_tuple),
             ("_hints", InternalTraversal.dp_table_hint_list),
         ]
         + HasPrefixes._has_prefixes_traverse_internals
diff --git a/test/orm/dml/test_update_delete_where.py b/test/orm/dml/test_update_delete_where.py
index cf9a30c19..19e557fd9 100644
--- a/test/orm/dml/test_update_delete_where.py
+++ b/test/orm/dml/test_update_delete_where.py
@@ -934,7 +934,8 @@ class UpdateDeleteTest(fixtures.MappedTest):
             list(zip([15, 27, 19, 27])),
         )
 
-    def test_update_future_lambda(self):
+    @testing.variation("values_first", [True, False])
+    def test_update_future_lambda(self, values_first):
         User, users = self.classes.User, self.tables.users
 
         sess = Session(testing.db, future=True)
@@ -943,14 +944,22 @@ class UpdateDeleteTest(fixtures.MappedTest):
             sess.execute(select(User).order_by(User.id)).scalars().all()
         )
 
-        sess.execute(
-            lambda_stmt(
+        new_value = 10
+
+        if values_first:
+            stmt = lambda_stmt(lambda: update(User))
+            stmt += lambda s: s.values({"age": User.age - new_value})
+            stmt += lambda s: s.where(User.age > 29).execution_options(
+                synchronize_session="evaluate"
+            )
+        else:
+            stmt = lambda_stmt(
                 lambda: update(User)
                 .where(User.age > 29)
-                .values({"age": User.age - 10})
+                .values({"age": User.age - new_value})
                 .execution_options(synchronize_session="evaluate")
-            ),
-        )
+            )
+        sess.execute(stmt)
 
         eq_([john.age, jack.age, jill.age, jane.age], [25, 37, 29, 27])
         eq_(
@@ -958,14 +967,21 @@ class UpdateDeleteTest(fixtures.MappedTest):
             list(zip([25, 37, 29, 27])),
         )
 
-        sess.execute(
-            lambda_stmt(
+        if values_first:
+            stmt = lambda_stmt(lambda: update(User))
+            stmt += lambda s: s.values({"age": User.age - new_value})
+            stmt += lambda s: s.where(User.age > 29).execution_options(
+                synchronize_session="evaluate"
+            )
+        else:
+            stmt = lambda_stmt(
                 lambda: update(User)
                 .where(User.age > 29)
                 .values({User.age: User.age - 10})
                 .execution_options(synchronize_session="evaluate")
             )
-        )
+
+        sess.execute(stmt)
         eq_([john.age, jack.age, jill.age, jane.age], [25, 27, 29, 27])
         eq_(
             sess.query(User.age).order_by(User.id).all(),
diff --git a/test/sql/test_external_traversal.py b/test/sql/test_external_traversal.py
index 8940276e3..8ccbd8d20 100644
--- a/test/sql/test_external_traversal.py
+++ b/test/sql/test_external_traversal.py
@@ -2964,3 +2964,41 @@ class ValuesBaseTest(fixtures.TestBase, AssertsCompiledSQL):
             "UPDATE construct does not support multiple parameter sets.",
             stmt.compile,
         )
+
+    @testing.variation("stmt_type", ["update", "delete"])
+    def test_whereclause_returning_adapted(self, stmt_type):
+        """test #9033"""
+
+        if stmt_type.update:
+            stmt = (
+                t1.update()
+                .where(t1.c.col1 == 10)
+                .values(col1=15)
+                .returning(t1.c.col1)
+            )
+        elif stmt_type.delete:
+            stmt = t1.delete().where(t1.c.col1 == 10).returning(t1.c.col1)
+        else:
+            stmt_type.fail()
+
+        stmt = visitors.replacement_traverse(stmt, {}, lambda elem: None)
+
+        assert isinstance(stmt._where_criteria, tuple)
+        assert isinstance(stmt._returning, tuple)
+
+        stmt = stmt.where(t1.c.col2 == 5).returning(t1.c.col2)
+
+        if stmt_type.update:
+            self.assert_compile(
+                stmt,
+                "UPDATE table1 SET col1=:col1 WHERE table1.col1 = :col1_1 "
+                "AND table1.col2 = :col2_1 RETURNING table1.col1, table1.col2",
+            )
+        elif stmt_type.delete:
+            self.assert_compile(
+                stmt,
+                "DELETE FROM table1 WHERE table1.col1 = :col1_1 "
+                "AND table1.col2 = :col2_1 RETURNING table1.col1, table1.col2",
+            )
+        else:
+            stmt_type.fail()