From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 2 Jul 2019 02:33:26 +0000 (-0400)
Subject: Clear proxy_set cache when creating an annotated column
X-Git-Tag: rel_1_4_0b1~815
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6636cd9d256ccbad651eba6553ec46391380cc93;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Clear proxy_set cache when creating an annotated column

Fixed an unlikely issue where the "corresponding column" routine for unions
and other :class:`.CompoundSelect` objects could return the wrong column in
some overlapping column situtations, thus potentially impacting some ORM
operations when set operations are in use, if the underlying
:func:`.select` constructs were used previously in other similar kinds of
routines, due to a cached value not being cleared.

Fixes: #4747
Change-Id: I7fb134cac3604f8fe62e220fb24a0945d0a1c56f
---

diff --git a/doc/build/changelog/unreleased_13/4747.rst b/doc/build/changelog/unreleased_13/4747.rst
new file mode 100644
index 0000000000..dc0a628525
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4747.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 4747
+
+    Fixed an unlikely issue where the "corresponding column" routine for unions
+    and other :class:`.CompoundSelect` objects could return the wrong column in
+    some overlapping column situtations, thus potentially impacting some ORM
+    operations when set operations are in use, if the underlying
+    :func:`.select` constructs were used previously in other similar kinds of
+    routines, due to a cached value not being cleared.
diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py
index cc57e58e5d..aa7b7f6888 100644
--- a/lib/sqlalchemy/sql/elements.py
+++ b/lib/sqlalchemy/sql/elements.py
@@ -635,6 +635,7 @@ class ColumnElement(
     __visit_name__ = "column_element"
     primary_key = False
     foreign_keys = []
+    _proxies = ()
 
     _label = None
     """The named label that can be used to target
@@ -783,16 +784,13 @@ class ColumnElement(
 
     @util.memoized_property
     def base_columns(self):
-        return util.column_set(
-            c for c in self.proxy_set if not hasattr(c, "_proxies")
-        )
+        return util.column_set(c for c in self.proxy_set if not c._proxies)
 
     @util.memoized_property
     def proxy_set(self):
         s = util.column_set([self])
-        if hasattr(self, "_proxies"):
-            for c in self._proxies:
-                s.update(c.proxy_set)
+        for c in self._proxies:
+            s.update(c.proxy_set)
         return s
 
     def shares_lineage(self, othercolumn):
@@ -4388,6 +4386,8 @@ class AnnotatedColumnElement(Annotated):
     def __init__(self, element, values):
         Annotated.__init__(self, element, values)
         ColumnElement.comparator._reset(self)
+        if self._proxies:
+            ColumnElement.proxy_set._reset(self)
         for attr in ("name", "key", "table"):
             if self.__dict__.get(attr, False) is None:
                 self.__dict__.pop(attr)
diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 62ff25a646..d39bc9832b 100644
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -1927,7 +1927,7 @@ class ForeignKey(DialectKWArgs, SchemaItem):
 
         parenttable = self.parent.table
 
-        # assertion, can be commented out.
+        # assertion
         # basically Column._make_proxy() sends the actual
         # target Column to the ForeignKey object, so the
         # string resolution here is never called.
diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index e38de66e0d..2c898e8359 100644
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -2759,7 +2759,6 @@ class CompoundSelect(GenerativeSelect):
             # to how low in the list of select()s the column occurs, so
             # that the corresponding_column() operation can resolve
             # conflicts
-
             proxy._proxies = [
                 c._annotate({"weight": i + 1}) for (i, c) in enumerate(cols)
             ]
diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index bfa96d766e..1a53cd1d67 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -433,6 +433,18 @@ class SelectableTest(
         assert u1.corresponding_column(table1.c.colx) is u1.c.col2
         assert u1.corresponding_column(table1.c.col3) is u1.c.col1
 
+    def test_proxy_set_pollution(self):
+        s1 = select([table1.c.col1, table1.c.col2])
+        s2 = select([table1.c.col2, table1.c.col1])
+
+        for c in s1.c:
+            c.proxy_set
+        for c in s2.c:
+            c.proxy_set
+
+        u1 = union(s1, s2)
+        assert u1.corresponding_column(table1.c.col2) is u1.c.col2
+
     def test_singular_union(self):
         u = union(
             select([table1.c.col1, table1.c.col2, table1.c.col3]),
@@ -1936,6 +1948,27 @@ class AnnotationsTest(fixtures.TestBase):
         assert x_p.compare(x_p_a)
         assert not x_p_a.compare(x_a)
 
+    def test_proxy_set_iteration_includes_annotated(self):
+        from sqlalchemy.schema import Column
+
+        c1 = Column("foo", Integer)
+
+        stmt = select([c1]).alias()
+        proxy = stmt.c.foo
+
+        proxy.proxy_set
+
+        # create an annotated of the column
+        p2 = proxy._annotate({"weight": 10})
+
+        # now see if our annotated version is in that column's
+        # proxy_set, as corresponding_column iterates through proxy_set
+        # in this way
+        d = {}
+        for col in p2.proxy_set:
+            d.update(col._annotations)
+        eq_(d, {"weight": 10})
+
     def test_late_name_add(self):
         from sqlalchemy.schema import Column