From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 10 Jan 2020 15:30:13 +0000 (-0500)
Subject: Alter unique bound parameter key on deserialize
X-Git-Tag: rel_1_3_13~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1561787f417eadd1728aca1d07f76c025288491e;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Alter unique bound parameter key on deserialize

Fixed bug in sqlalchemy.ext.serializer where a unique
:class:`.BindParameter` object could conflict with itself if it were
present in the mapping itself, as well as the filter condition of the
query, as one side would be used against the non-deserialized version and
the other side would use the deserialized version.  Logic is added to
:class:`.BindParameter` similar to its "clone" method which will uniquify
the parameter name upon deserialize so that it doesn't conflict with its
original.

Fixes: #5086
Change-Id: Ie1edce137e92ac496c822831d038999be5d1fc2d
(cherry picked from commit 4b17d0306421cab9821125fb774d1ff89b36e77e)
---

diff --git a/doc/build/changelog/unreleased_13/5086.rst b/doc/build/changelog/unreleased_13/5086.rst
new file mode 100644
index 0000000000..f52f9b194c
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/5086.rst
@@ -0,0 +1,13 @@
+.. change::
+    :tags: bug, ext
+    :tickets: 5086
+
+    Fixed bug in sqlalchemy.ext.serializer where a unique
+    :class:`.BindParameter` object could conflict with itself if it were
+    present in the mapping itself, as well as the filter condition of the
+    query, as one side would be used against the non-deserialized version and
+    the other side would use the deserialized version.  Logic is added to
+    :class:`.BindParameter` similar to its "clone" method which will uniquify
+    the parameter name upon deserialize so that it doesn't conflict with its
+    original.
+
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index 0fd1f40582..c1fc581ef3 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -1278,6 +1278,13 @@ class BindParameter(ColumnElement):
         d["value"] = v
         return d
 
+    def __setstate__(self, state):
+        if state.get("unique", False):
+            state["key"] = _anonymous_label(
+                "%%(%d %s)s" % (id(self), state.get("_orig_key", "param"))
+            )
+        self.__dict__.update(state)
+
     def __repr__(self):
         return "BindParameter(%r, %r, type_=%r)" % (
             self.key,
diff --git a/test/ext/test_serializer.py b/test/ext/test_serializer.py
index e6c7b717f9..ac5133fdaf 100644
--- a/test/ext/test_serializer.py
+++ b/test/ext/test_serializer.py
@@ -13,6 +13,7 @@ from sqlalchemy import testing
 from sqlalchemy.ext import serializer
 from sqlalchemy.orm import aliased
 from sqlalchemy.orm import class_mapper
+from sqlalchemy.orm import column_property
 from sqlalchemy.orm import configure_mappers
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm import mapper
@@ -40,9 +41,6 @@ class Address(fixtures.ComparableEntity):
     pass
 
 
-users = addresses = Session = None
-
-
 class SerializeTest(AssertsCompiledSQL, fixtures.MappedTest):
 
     run_setup_mappers = "once"
@@ -292,5 +290,59 @@ class SerializeTest(AssertsCompiledSQL, fixtures.MappedTest):
         )
 
 
+class ColumnPropertyWParamTest(
+    AssertsCompiledSQL, fixtures.DeclarativeMappedTest
+):
+    __dialect__ = "default"
+
+    run_create_tables = None
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        global TestTable
+
+        class TestTable(Base):
+            __tablename__ = "test"
+
+            id = Column(Integer, primary_key=True, autoincrement=True)
+            _some_id = Column("some_id", String)
+            some_primary_id = column_property(
+                func.left(_some_id, 6).cast(Integer)
+            )
+
+    def test_deserailize_colprop(self):
+        TestTable = self.classes.TestTable
+
+        s = scoped_session(sessionmaker())
+
+        expr = s.query(TestTable).filter(TestTable.some_primary_id == 123456)
+
+        expr2 = serializer.loads(serializer.dumps(expr), TestTable.metadata, s)
+
+        # note in the original, the same bound parameter is used twice
+        self.assert_compile(
+            expr,
+            "SELECT test.some_id AS test_some_id, "
+            "CAST(left(test.some_id, :left_1) AS INTEGER) AS anon_1, "
+            "test.id AS test_id FROM test WHERE "
+            "CAST(left(test.some_id, :left_1) AS INTEGER) = :param_1",
+            checkparams={"left_1": 6, "param_1": 123456},
+        )
+
+        # in the deserialized, it's two separate parameter objects which
+        # need to have different anonymous names.  they still have
+        # the same value however
+        self.assert_compile(
+            expr2,
+            "SELECT test.some_id AS test_some_id, "
+            "CAST(left(test.some_id, :left_1) AS INTEGER) AS anon_1, "
+            "test.id AS test_id FROM test WHERE "
+            "CAST(left(test.some_id, :left_2) AS INTEGER) = :param_1",
+            checkparams={"left_1": 6, "left_2": 6, "param_1": 123456},
+        )
+
+
 if __name__ == "__main__":
     testing.main()