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 (file)
index 0000000..4a27470
--- /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 f2644548c111b618dace2b1d677f6bf6be58f8bb..2c10ec55afa5dbfcd7602cce8c755d009f928837 100644 (file)
--- 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 81da16914b769fb1db20b1489cb8c96ed7a6a591..3fc82db694498028174850523a47824eae674de0 100644 (file)
--- 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 6e5d29fe97b1ed5221d5be5a540ffa27516952cb..3f7b08b470ce510ed8e8b6970ecfd3cf8d0f2570 100644 (file)
--- 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}})