accommodate for "clone" of ColumnClause

for use with the ClauseElement.params() method,
altered ColumnClause._clone() so that while the element
stays immutable, if the column is associated with a subquery,
it returns a new version of itself as corresponding to a
clone of the subquery.  this allows processing functions
to access the parameters in the subquery and produce a
copy of it.  The use case here is the expanded use of
.params() within loader strategies that use
HasCacheKey._apply_params_to_element().

Fixed issue in new "loader criteria" method
:meth:`_orm.PropComparator.and_` where usage with a loader strategy like
:func:`_orm.selectinload` against a column that was a member of the ``.c.``
collection of a subquery object, where the subquery would be dynamically
added to the FROM clause of the statement, would be subject to stale
parameter values within the subquery in the SQL statement cache, as the
process used by the loader strategy to replace the parameters at execution
time would fail to accommodate the subquery when received in this form.

Fixes: #7489
Change-Id: Ibb3b6af140b8a62a2c8d05b2ac92e86ca3013c46

diff --git a/doc/build/changelog/unreleased_14/7489.rst b/doc/build/changelog/unreleased_14/7489.rst
new file mode 100644 (file)
index 0000000..4af33f4
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/7489.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 7489
+
+    Fixed issue in new "loader criteria" method
+    :meth:`_orm.PropComparator.and_` where usage with a loader strategy like
+    :func:`_orm.selectinload` against a column that was a member of the ``.c.``
+    collection of a subquery object, where the subquery would be dynamically
+    added to the FROM clause of the statement, would be subject to stale
+    parameter values within the subquery in the SQL statement cache, as the
+    process used by the loader strategy to replace the parameters at execution
+    time would fail to accommodate the subquery when received in this form.
+

diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 00270c9b5832f96b6b1e404b7dd7a62fa3fe47d5..75798502a7de4152931a396cada8e24723786cab 100644 (file)
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -439,6 +439,7 @@ class ClauseElement(
         return self._replace_params(False, optionaldict, kwargs)
 
     def _replace_params(self, unique, optionaldict, kwargs):
+
         if len(optionaldict) == 1:
             kwargs.update(optionaldict[0])
         elif len(optionaldict) > 1:
@@ -454,7 +455,9 @@ class ClauseElement(
                 bind._convert_to_unique()
 
         return cloned_traverse(
-            self, {"maintain_key": True}, {"bindparam": visit_bindparam}
+            self,
+            {"maintain_key": True, "detect_subquery_cols": True},
+            {"bindparam": visit_bindparam},
         )
 
     def compare(self, other, **kw):
@@ -4869,6 +4872,19 @@ class ColumnClause(
         else:
             return super(ColumnClause, self).entity_namespace
 
+    def _clone(self, detect_subquery_cols=False, **kw):
+        if (
+            detect_subquery_cols
+            and self.table is not None
+            and self.table._is_subquery
+        ):
+            clone = kw.pop("clone")
+            table = clone(self.table, **kw)
+            new = table.c.corresponding_column(self)
+            return new
+
+        return super(ColumnClause, self)._clone(**kw)
+
     @HasMemoized.memoized_attribute
     def _from_objects(self):
         t = self.table

diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index d58b5c2bb376326899c9fa5333d6767719ba6634..12a5486d223f18e0e59ed4432c9a2e126d4bd6f4 100644 (file)
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -271,7 +271,6 @@ class HasCacheKey:
                         result += meth(
                             attrname, obj, self, anon_map, bindparams
                         )
-
         return result
 
     def _generate_cache_key(self):

diff --git a/lib/sqlalchemy/sql/visitors.py b/lib/sqlalchemy/sql/visitors.py
index b0808075354280364238cbec0011ec06de17f83a..30103bc8ef144cbc03775c408f3432e551d1a981 100644 (file)
--- a/lib/sqlalchemy/sql/visitors.py
+++ b/lib/sqlalchemy/sql/visitors.py
@@ -774,7 +774,7 @@ def cloned_traverse(obj, opts, visitors):
                         cloned[id(elem)] = newelem
                         return newelem
 
-                cloned[id(elem)] = newelem = elem._clone(**kw)
+                cloned[id(elem)] = newelem = elem._clone(clone=clone, **kw)
                 newelem._copy_internals(clone=clone, **kw)
                 meth = visitors.get(newelem.__visit_name__, None)
                 if meth:

diff --git a/test/orm/test_relationship_criteria.py b/test/orm/test_relationship_criteria.py
index b2c16da05736d5bc5cae13080507aa50d117d4a1..b5c9be03829aa75d6b6a0d7508911de63b03818c 100644 (file)
--- a/test/orm/test_relationship_criteria.py
+++ b/test/orm/test_relationship_criteria.py
@@ -1240,6 +1240,66 @@ class RelationshipCriteriaTest(_Fixtures, testing.AssertsCompiledSQL):
                 ),
             )
 
+    def test_selectinload_local_criteria_subquery(self, user_address_fixture):
+        """test #7489"""
+        User, Address = user_address_fixture
+
+        s = Session(testing.db, future=True)
+
+        def go(value):
+            a1 = aliased(Address)
+            subq = select(a1.id).where(a1.email_address != value).subquery()
+            stmt = (
+                select(User)
+                .options(
+                    selectinload(User.addresses.and_(Address.id == subq.c.id)),
+                )
+                .order_by(User.id)
+            )
+            result = s.execute(stmt)
+            return result
+
+        for value in (
+            "ed@wood.com",
+            "ed@lala.com",
+            "ed@wood.com",
+            "ed@lala.com",
+        ):
+            s.close()
+            with self.sql_execution_asserter() as asserter:
+                result = go(value)
+
+                eq_(
+                    result.scalars().unique().all(),
+                    self._user_minus_edwood(*user_address_fixture)
+                    if value == "ed@wood.com"
+                    else self._user_minus_edlala(*user_address_fixture),
+                )
+
+            asserter.assert_(
+                CompiledSQL(
+                    "SELECT users.id, users.name FROM users ORDER BY users.id"
+                ),
+                CompiledSQL(
+                    "SELECT addresses.user_id AS addresses_user_id, "
+                    "addresses.id AS addresses_id, "
+                    "addresses.email_address AS addresses_email_address "
+                    # note the comma-separated FROM clause
+                    "FROM addresses, (SELECT addresses_1.id AS id FROM "
+                    "addresses AS addresses_1 "
+                    "WHERE addresses_1.email_address != :email_address_1) "
+                    "AS anon_1 WHERE addresses.user_id "
+                    "IN (__[POSTCOMPILE_primary_keys]) "
+                    "AND addresses.id = anon_1.id ORDER BY addresses.id",
+                    [
+                        {
+                            "primary_keys": [7, 8, 9, 10],
+                            "email_address_1": value,
+                        }
+                    ],
+                ),
+            )
+
     @testing.combinations((True,), (False,), argnames="use_compiled_cache")
     def test_selectinload_nested_criteria(
         self, user_order_item_fixture, use_compiled_cache

diff --git a/test/sql/test_external_traversal.py b/test/sql/test_external_traversal.py
index 0ac73fc165407352947ee3a7e4eca380ed5aaf03..ace618e501f62911d1bf0bb585713cd7f3beb654 100644 (file)
--- a/test/sql/test_external_traversal.py
+++ b/test/sql/test_external_traversal.py
@@ -827,6 +827,48 @@ class ClauseTest(fixtures.TestBase, AssertsCompiledSQL):
             sel._generate_cache_key()[1],
         )
 
+    def test_params_on_expr_against_subquery(self):
+        """test #7489"""
+
+        meta = MetaData()
+
+        b = Table("b", meta, Column("id", Integer), Column("data", String))
+
+        subq = select(b.c.id).where(b.c.data == "some data").subquery()
+        criteria = b.c.id == subq.c.id
+
+        stmt = select(b).where(criteria)
+        param_key = stmt._generate_cache_key()[1][0].key
+
+        self.assert_compile(
+            stmt,
+            "SELECT b.id, b.data FROM b, (SELECT b.id AS id "
+            "FROM b WHERE b.data = :data_1) AS anon_1 WHERE b.id = anon_1.id",
+            checkparams={"data_1": "some data"},
+        )
+        eq_(
+            [
+                eq_clause_element(bindparam(param_key, value="some data")),
+            ],
+            stmt._generate_cache_key()[1],
+        )
+
+        stmt = select(b).where(criteria.params({param_key: "some other data"}))
+        self.assert_compile(
+            stmt,
+            "SELECT b.id, b.data FROM b, (SELECT b.id AS id "
+            "FROM b WHERE b.data = :data_1) AS anon_1 WHERE b.id = anon_1.id",
+            checkparams={"data_1": "some other data"},
+        )
+        eq_(
+            [
+                eq_clause_element(
+                    bindparam(param_key, value="some other data")
+                ),
+            ],
+            stmt._generate_cache_key()[1],
+        )
+
     def test_params_subqueries_in_joins_one(self):
         """test #7055"""