From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 8 Feb 2024 13:45:22 +0000 (-0500)
Subject: handle case where neither side has a cache key
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d97679e0926b829592bf5962d9dae5f2fe99503f;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

handle case where neither side has a cache key

Fixed issue where an assertion within the implementation for
:func:`_orm.with_expression` would raise if a SQL expression that was not
cacheable were used; this was a 2.0 regression since 1.4.

Fixes: #10990
Change-Id: I6541189d29d2e860df7fbab187bfcc6f4dcbfc76
---

diff --git a/doc/build/changelog/unreleased_20/10990.rst b/doc/build/changelog/unreleased_20/10990.rst
new file mode 100644
index 0000000000..ac887c8364
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/10990.rst
@@ -0,0 +1,7 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 10990
+
+    Fixed issue where an assertion within the implementation for
+    :func:`_orm.with_expression` would raise if a SQL expression that was not
+    cacheable were used; this was a 2.0 regression since 1.4.
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index bdf6802f99..d69fa6edb4 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1064,15 +1064,15 @@ class Load(_AbstractLoad):
                 orig_cache_key = orig_query._generate_cache_key()
                 replacement_cache_key = context.query._generate_cache_key()
 
+            if replacement_cache_key is not None:
                 assert orig_cache_key is not None
-                assert replacement_cache_key is not None
 
-            opt._extra_criteria = tuple(
-                replacement_cache_key._apply_params_to_element(
-                    orig_cache_key, crit
+                opt._extra_criteria = tuple(
+                    replacement_cache_key._apply_params_to_element(
+                        orig_cache_key, crit
+                    )
+                    for crit in opt._extra_criteria
                 )
-                for crit in opt._extra_criteria
-            )
 
             return opt
 
diff --git a/test/orm/test_deferred.py b/test/orm/test_deferred.py
index 66e3104a95..dbfe3ef797 100644
--- a/test/orm/test_deferred.py
+++ b/test/orm/test_deferred.py
@@ -10,6 +10,7 @@ from sqlalchemy import null
 from sqlalchemy import select
 from sqlalchemy import String
 from sqlalchemy import testing
+from sqlalchemy import TypeDecorator
 from sqlalchemy import union_all
 from sqlalchemy import util
 from sqlalchemy.orm import aliased
@@ -2215,9 +2216,21 @@ class WithExpressionTest(fixtures.DeclarativeMappedTest):
 
             c_expr = query_expression(literal(1))
 
+        class CustomTimeStamp(TypeDecorator):
+            cache_ok = False
+            impl = Integer
+
+        class HasNonCacheable(ComparableEntity, Base):
+            __tablename__ = "non_cacheable"
+
+            id = Column(Integer, primary_key=True)
+            created = Column(CustomTimeStamp)
+            msg_translated = query_expression()
+
     @classmethod
     def insert_data(cls, connection):
         A, A_default, B, C = cls.classes("A", "A_default", "B", "C")
+        (HasNonCacheable,) = cls.classes("HasNonCacheable")
         s = Session(connection)
 
         s.add_all(
@@ -2230,6 +2243,7 @@ class WithExpressionTest(fixtures.DeclarativeMappedTest):
                 C(id=2, x=2),
                 A_default(id=1, x=1, y=2),
                 A_default(id=2, x=2, y=3),
+                HasNonCacheable(id=1, created=12345),
             ]
         )
 
@@ -2269,6 +2283,30 @@ class WithExpressionTest(fixtures.DeclarativeMappedTest):
         )
         eq_(c2.all(), [C(c_expr=4)])
 
+    def test_non_cacheable_expr(self):
+        """test #10990"""
+
+        HasNonCacheable = self.classes.HasNonCacheable
+
+        for i in range(3):
+            s = fixture_session()
+
+            stmt = (
+                select(HasNonCacheable)
+                .where(HasNonCacheable.created > 10)
+                .options(
+                    with_expression(
+                        HasNonCacheable.msg_translated,
+                        HasNonCacheable.created + 10,
+                    )
+                )
+            )
+
+            eq_(
+                s.scalars(stmt).all(),
+                [HasNonCacheable(id=1, created=12345, msg_translated=12355)],
+            )
+
     def test_reuse_expr(self):
         A = self.classes.A