Don't assume key when matching cloned columns in _make_proxy

Fixed issue where internal cloning of SELECT constructs could lead to a key
error if the copy of the SELECT changed its state such that its list of
columns changed.  This was observed to be occurring in some ORM scenarios
which may be unique to 1.3 and above, so is partially a regression fix.

For 1.4, the _is_clone_of key will be removed entirely as it seems to
have no purpose.  This commit is the initial backport to 1.3 which
includes tests.

Fixes: #4780
Change-Id: I0c64962a2eba3763bea3107fc7c7d7aed8244430
(cherry picked from commit 896d47f318c5c27620fd6da805f98811941b88c5)

diff --git a/doc/build/changelog/unreleased_13/4780.rst b/doc/build/changelog/unreleased_13/4780.rst
new file mode 100644 (file)
index 0000000..55886f3
--- /dev/null
+++ b/doc/build/changelog/unreleased_13/4780.rst
@@ -0,0 +1,10 @@
+.. change::
+    :tags: bug, sql
+    :tickets: 4780
+
+    Fixed issue where internal cloning of SELECT constructs could lead to a key
+    error if the copy of the SELECT changed its state such that its list of
+    columns changed.  This was observed to be occurring in some ORM scenarios
+    which may be unique to 1.3 and above, so is partially a regression fix.
+
+

diff --git a/lib/sqlalchemy/sql/schema.py b/lib/sqlalchemy/sql/schema.py
index 34cef6db3ff1ee5fd8f13712861cff27fe68a2ba..30bf72385c6065d272141d4fd04398a199833d0b 100644 (file)
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -1607,7 +1607,7 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause):
         c.table = selectable
         selectable._columns.add(c)
         if selectable._is_clone_of is not None:
-            c._is_clone_of = selectable._is_clone_of.columns[c.key]
+            c._is_clone_of = selectable._is_clone_of.columns.get(c.key)
         if self.primary_key:
             selectable.primary_key.add(c)
         c.dispatch.after_parent_attach(c, selectable)

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 3d5f1e66243c0543b536f25285b96eb1cd521518..71532601d12be1953c10386f2a06bce1cc8ac18d 100644 (file)
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -274,6 +274,28 @@ class SelectableTest(
         cloned.append_column(func.foo())
         eq_(list(cloned.c.keys()), ["a", "b", "foo()"])
 
+    def test_clone_col_list_changes_then_proxy(self):
+        t = table("t", column("q"), column("p"))
+        stmt = select([t.c.q]).alias()
+
+        def add_column(stmt):
+            stmt.append_column(t.c.p)
+
+        stmt2 = visitors.cloned_traverse(stmt, {}, {"select": add_column})
+        eq_(list(stmt.c.keys()), ["q"])
+        eq_(list(stmt2.c.keys()), ["q", "p"])
+
+    def test_clone_col_list_changes_then_schema_proxy(self):
+        t = Table("t", MetaData(), Column("q", Integer), Column("p", Integer))
+        stmt = select([t.c.q]).alias()
+
+        def add_column(stmt):
+            stmt.append_column(t.c.p)
+
+        stmt2 = visitors.cloned_traverse(stmt, {}, {"select": add_column})
+        eq_(list(stmt.c.keys()), ["q"])
+        eq_(list(stmt2.c.keys()), ["q", "p"])
+
     def test_append_column_after_replace_selectable(self):
         basesel = select([literal_column("1").label("a")])
         tojoin = select(