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

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 c63a3160f5b43f6e76105b6806cc671be60730bc..beebc5b1fc03f1419d14449f07a733629f39c22f 100644 (file)
--- a/lib/sqlalchemy/sql/schema.py
+++ b/lib/sqlalchemy/sql/schema.py
@@ -1608,7 +1608,7 @@ class Column(DialectKWArgs, SchemaItem, ColumnClause):
 
         c.table = selectable
         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)
         if fk:

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index dfd35908bdc5a49ebf336422d724def8785a7972..189436192db53e6277dc858581e7142f50b9dd4f 100644 (file)
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -346,6 +346,28 @@ class SelectableTest(
         cloned.append_column(func.foo())
         eq_(list(cloned.selected_columns.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]).subquery()
+
+        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]).subquery()
+
+        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_visitor_replace(self):
         # test for a supported idiom that matches the deprecated / removed
         # replace_selectable method