From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Sat, 7 Sep 2024 21:41:16 +0000 (-0400)
Subject: test for Concatenable in ORM evaluator for concat_op
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b08e9ba2420e856c5073129b351cfd5cf95422b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

test for Concatenable in ORM evaluator for concat_op

Fixed issue in ORM evaluator where two datatypes being evaluated with the
SQL concatenator operator would not be checked for
:class:`.UnevaluatableError` based on their datatype; this missed the case
of :class:`_postgresql.JSONB` values being used in a concatenate operation
which is supported by PostgreSQL as well as how SQLAlchemy renders the SQL
for this operation, but does not work at the Python level. By implementing
:class:`.UnevaluatableError` for this combination, ORM update statements
will now fall back to "expire" when a concatenated JSON value used in a SET
clause is to be synchronized to a Python object.

Fixes: #11849
Change-Id: Iccd97edf57b99b9a606ab3a47d2e3e5b63f0db07
---

diff --git a/doc/build/changelog/unreleased_20/11849.rst b/doc/build/changelog/unreleased_20/11849.rst
new file mode 100644
index 0000000000..4a274702ec
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/11849.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: orm, bug
+    :tickets: 11849
+
+    Fixed issue in ORM evaluator where two datatypes being evaluated with the
+    SQL concatenator operator would not be checked for
+    :class:`.UnevaluatableError` based on their datatype; this missed the case
+    of :class:`_postgresql.JSONB` values being used in a concatenate operation
+    which is supported by PostgreSQL as well as how SQLAlchemy renders the SQL
+    for this operation, but does not work at the Python level. By implementing
+    :class:`.UnevaluatableError` for this combination, ORM update statements
+    will now fall back to "expire" when a concatenated JSON value used in a SET
+    clause is to be synchronized to a Python object.
diff --git a/lib/sqlalchemy/orm/evaluator.py b/lib/sqlalchemy/orm/evaluator.py
index f2644548c1..2c10ec55af 100644
--- a/lib/sqlalchemy/orm/evaluator.py
+++ b/lib/sqlalchemy/orm/evaluator.py
@@ -28,6 +28,7 @@ from .. import exc
 from .. import inspect
 from ..sql import and_
 from ..sql import operators
+from ..sql.sqltypes import Concatenable
 from ..sql.sqltypes import Integer
 from ..sql.sqltypes import Numeric
 from ..util import warn_deprecated
@@ -311,6 +312,16 @@ class _EvaluatorCompiler:
     def visit_concat_op_binary_op(
         self, operator, eval_left, eval_right, clause
     ):
+
+        if not issubclass(
+            clause.left.type._type_affinity, Concatenable
+        ) or not issubclass(clause.right.type._type_affinity, Concatenable):
+            raise UnevaluatableError(
+                f"Cannot evaluate concatenate operator "
+                f'"{operator.__name__}" for '
+                f"datatypes {clause.left.type}, {clause.right.type}"
+            )
+
         return self._straight_evaluate(
             lambda a, b: a + b, eval_left, eval_right, clause
         )
diff --git a/test/orm/dml/test_evaluator.py b/test/orm/dml/test_evaluator.py
index 81da16914b..3fc82db694 100644
--- a/test/orm/dml/test_evaluator.py
+++ b/test/orm/dml/test_evaluator.py
@@ -370,6 +370,14 @@ class EvaluateTest(fixtures.MappedTest):
             r"Cannot evaluate math operator \"add\" for "
             r"datatypes JSON, INTEGER",
         ),
+        (
+            lambda User: User.json + {"bar": "bat"},
+            "json",
+            {"foo": "bar"},
+            evaluator.UnevaluatableError,
+            r"Cannot evaluate concatenate operator \"concat_op\" for "
+            r"datatypes JSON, JSON",
+        ),
         (
             lambda User: User.json - 12,
             "json",
diff --git a/test/orm/dml/test_update_delete_where.py b/test/orm/dml/test_update_delete_where.py
index 6e5d29fe97..3f7b08b470 100644
--- a/test/orm/dml/test_update_delete_where.py
+++ b/test/orm/dml/test_update_delete_where.py
@@ -3294,3 +3294,44 @@ class LoadFromReturningTest(fixtures.MappedTest):
             )
 
             # TODO: state of above objects should be "deleted"
+
+
+class PGIssue11849Test(fixtures.DeclarativeMappedTest):
+    __backend__ = True
+    __only_on__ = ("postgresql",)
+
+    @classmethod
+    def setup_classes(cls):
+
+        from sqlalchemy.dialects.postgresql import JSONB
+
+        Base = cls.DeclarativeBasic
+
+        class TestTbl(Base):
+            __tablename__ = "testtbl"
+
+            test_id = Column(Integer, primary_key=True)
+            test_field = Column(JSONB)
+
+    def test_issue_11849(self):
+        TestTbl = self.classes.TestTbl
+
+        session = fixture_session()
+
+        obj = TestTbl(
+            test_id=1, test_field={"test1": 1, "test2": "2", "test3": [3, "3"]}
+        )
+        session.add(obj)
+
+        query = (
+            update(TestTbl)
+            .where(TestTbl.test_id == 1)
+            .values(test_field=TestTbl.test_field + {"test3": {"test4": 4}})
+        )
+        session.execute(query)
+
+        # not loaded
+        assert "test_field" not in obj.__dict__
+
+        # synchronizes on load
+        eq_(obj.test_field, {"test1": 1, "test2": "2", "test3": {"test4": 4}})