From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Mon, 10 Feb 2025 20:26:24 +0000 (-0500)
Subject: reconcile #12326 and #12328
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1cdaae5e5706749614f04cef79e85a50143f9ec7;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

reconcile #12326 and #12328

These two issues both involve ORM DML RETURNING.   The looser
column inclusion rules given in #12328 then included a correlated
subquery column_property given in #12326, which does not work
in RETURNING.  so re-tighten UPDATE/DELETE with a more specific
rule to cut out local mapped props that are not persisted columns,
but still allow other mapped props through without blocking them.

Fixes: #12326
Change-Id: I8fe7b8ab9b85907e562648433fdb3c7ba160c0d0
---

diff --git a/lib/sqlalchemy/orm/context.py b/lib/sqlalchemy/orm/context.py
index fa57bcfae8..a67331fe80 100644
--- a/lib/sqlalchemy/orm/context.py
+++ b/lib/sqlalchemy/orm/context.py
@@ -667,14 +667,8 @@ class _ORMCompileState(_AbstractORMCompileState):
         )
 
 
-class _DMLBulkInsertReturningColFilter:
-    """an adapter used for the DML RETURNING case specifically
-    for ORM bulk insert (or any hypothetical DML that is splitting out a class
-    hierarchy among multiple DML statements....ORM bulk insert is the only
-    example right now)
-
-    its main job is to limit the columns in a RETURNING to only a specific
-    mapped table in a hierarchy.
+class _DMLReturningColFilter:
+    """a base for an adapter used for the DML RETURNING cases
 
     Has a subset of the interface used by
     :class:`.ORMAdapter` and is used for :class:`._QueryEntity`
@@ -708,6 +702,21 @@ class _DMLBulkInsertReturningColFilter:
         else:
             return None
 
+    def adapt_check_present(self, col):
+        raise NotImplementedError()
+
+
+class _DMLBulkInsertReturningColFilter(_DMLReturningColFilter):
+    """an adapter used for the DML RETURNING case specifically
+    for ORM bulk insert (or any hypothetical DML that is splitting out a class
+    hierarchy among multiple DML statements....ORM bulk insert is the only
+    example right now)
+
+    its main job is to limit the columns in a RETURNING to only a specific
+    mapped table in a hierarchy.
+
+    """
+
     def adapt_check_present(self, col):
         mapper = self.mapper
         prop = mapper._columntoproperty.get(col, None)
@@ -716,6 +725,30 @@ class _DMLBulkInsertReturningColFilter:
         return mapper.local_table.c.corresponding_column(col)
 
 
+class _DMLUpdateDeleteReturningColFilter(_DMLReturningColFilter):
+    """an adapter used for the DML RETURNING case specifically
+    for ORM enabled UPDATE/DELETE
+
+    its main job is to limit the columns in a RETURNING to include
+    only direct persisted columns from the immediate selectable, not
+    expressions like column_property(), or to also allow columns from other
+    mappers for the UPDATE..FROM use case.
+
+    """
+
+    def adapt_check_present(self, col):
+        mapper = self.mapper
+        prop = mapper._columntoproperty.get(col, None)
+        if prop is not None:
+            # if the col is from the immediate mapper, only return a persisted
+            # column, not any kind of column_property expression
+            return mapper.persist_selectable.c.corresponding_column(col)
+
+        # if the col is from some other mapper, just return it, assume the
+        # user knows what they are doing
+        return col
+
+
 @sql.base.CompileState.plugin_for("orm", "orm_from_statement")
 class _ORMFromStatementCompileState(_ORMCompileState):
     _from_obj_alias = None
@@ -882,6 +915,10 @@ class _ORMFromStatementCompileState(_ORMCompileState):
             adapter = _DMLBulkInsertReturningColFilter(
                 target_mapper, dml_mapper
             )
+        elif self.statement.is_update or self.statement.is_delete:
+            adapter = _DMLUpdateDeleteReturningColFilter(
+                target_mapper, dml_mapper
+            )
         else:
             adapter = None
 
@@ -2560,7 +2597,7 @@ class _QueryEntity:
     def setup_dml_returning_compile_state(
         self,
         compile_state: _ORMCompileState,
-        adapter: Optional[_DMLBulkInsertReturningColFilter],
+        adapter: Optional[_DMLReturningColFilter],
     ) -> None:
         raise NotImplementedError()
 
@@ -2762,7 +2799,7 @@ class _MapperEntity(_QueryEntity):
     def setup_dml_returning_compile_state(
         self,
         compile_state: _ORMCompileState,
-        adapter: Optional[_DMLBulkInsertReturningColFilter],
+        adapter: Optional[_DMLReturningColFilter],
     ) -> None:
         loading._setup_entity_query(
             compile_state,
@@ -2921,7 +2958,7 @@ class _BundleEntity(_QueryEntity):
     def setup_dml_returning_compile_state(
         self,
         compile_state: _ORMCompileState,
-        adapter: Optional[_DMLBulkInsertReturningColFilter],
+        adapter: Optional[_DMLReturningColFilter],
     ) -> None:
         return self.setup_compile_state(compile_state)
 
@@ -3111,7 +3148,7 @@ class _RawColumnEntity(_ColumnEntity):
     def setup_dml_returning_compile_state(
         self,
         compile_state: _ORMCompileState,
-        adapter: Optional[_DMLBulkInsertReturningColFilter],
+        adapter: Optional[_DMLReturningColFilter],
     ) -> None:
         return self.setup_compile_state(compile_state)
 
@@ -3228,7 +3265,7 @@ class _ORMColumnEntity(_ColumnEntity):
     def setup_dml_returning_compile_state(
         self,
         compile_state: _ORMCompileState,
-        adapter: Optional[_DMLBulkInsertReturningColFilter],
+        adapter: Optional[_DMLReturningColFilter],
     ) -> None:
 
         self._fetch_column = column = self.column