Adjustments to _copy_internals()

We are looking to build a generalization of copy_internals(),
so move out any special logic from these methods.   Re-implement
and clarify rationale for the Alias doesnt copy a TableClause rule as
part of the adaption
traversal, establish that we forgot to build out comparison and cache
key for CTE, remove incomplete _copy_internals() from GenerativeSelect
(it doesn't handle the order_by_clause or group_by_clause, so is incomplete)

Change-Id: I95039f042503171aade4ba0fabc9b1598e3c49cf

diff --git a/lib/sqlalchemy/sql/clause_compare.py b/lib/sqlalchemy/sql/clause_compare.py
index 50b1df99ed6134cffc031753b2fa9d4e3116d332..30a90348c9c0e0f7e28dc2f4eaa23a2552f921d2 100644 (file)
--- a/lib/sqlalchemy/sql/clause_compare.py
+++ b/lib/sqlalchemy/sql/clause_compare.py
@@ -208,6 +208,9 @@ class StructureComparatorStrategy(object):
             else isinstance(right.name, elements._anonymous_label)
         )
 
+    def compare_cte(self, elements, left, right, **kw):
+        raise NotImplementedError("TODO")
+
     def compare_extract(self, left, right, **kw):
         return left.field == right.field
 

diff --git a/lib/sqlalchemy/sql/selectable.py b/lib/sqlalchemy/sql/selectable.py
index 97c49f8fcc98e6b888ab80b5122172ac90519f30..00d3826b21f2a4b9660b105685fc1343ccd8c1c6 100644 (file)
--- a/lib/sqlalchemy/sql/selectable.py
+++ b/lib/sqlalchemy/sql/selectable.py
@@ -1247,13 +1247,10 @@ class AliasedReturnsRows(FromClause):
         self.element._generate_fromclause_column_proxies(self)
 
     def _copy_internals(self, clone=_clone, **kw):
-        # don't apply anything to an aliased Table
-        # for now.   May want to drive this from
-        # the given **kw.
-        if isinstance(self.element, TableClause):
-            return
-        self._reset_exported()
-        self.element = clone(self.element, **kw)
+        element = clone(self.element, **kw)
+        if element is not self.element:
+            self._reset_exported()
+        self.element = element
 
     def get_children(self, column_collections=True, **kw):
         if column_collections:
@@ -1509,6 +1506,9 @@ class CTE(Generative, HasSuffixes, AliasedReturnsRows):
             [clone(elem, **kw) for elem in self._restates]
         )
 
+    def _cache_key(self, *arg, **kw):
+        raise NotImplementedError("TODO")
+
     def alias(self, name=None, flat=False):
         """Return an :class:`.Alias` of this :class:`.CTE`.
 
@@ -2738,10 +2738,7 @@ class GenerativeSelect(SelectBase):
         raise NotImplementedError()
 
     def _copy_internals(self, clone=_clone, **kw):
-        if self._limit_clause is not None:
-            self._limit_clause = clone(self._limit_clause, **kw)
-        if self._offset_clause is not None:
-            self._offset_clause = clone(self._offset_clause, **kw)
+        raise NotImplementedError()
 
 
 class CompoundSelect(GenerativeSelect):
@@ -2987,12 +2984,13 @@ class CompoundSelect(GenerativeSelect):
         return self.selects[0].selected_columns
 
     def _copy_internals(self, clone=_clone, **kw):
-        super(CompoundSelect, self)._copy_internals(clone, **kw)
         self._reset_memoizations()
         self.selects = [clone(s, **kw) for s in self.selects]
         if hasattr(self, "_col_map"):
             del self._col_map
         for attr in (
+            "_limit_clause",
+            "_offset_clause",
             "_order_by_clause",
             "_group_by_clause",
             "_for_update_arg",
@@ -3568,7 +3566,6 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect):
         return False
 
     def _copy_internals(self, clone=_clone, **kw):
-        super(Select, self)._copy_internals(clone, **kw)
 
         # Select() object has been cloned and probably adapted by the
         # given clone function.  Apply the cloning function to internal
@@ -3614,6 +3611,8 @@ class Select(HasPrefixes, HasSuffixes, GenerativeSelect):
         # present here.
         self._raw_columns = [clone(c, **kw) for c in self._raw_columns]
         for attr in (
+            "_limit_clause",
+            "_offset_clause",
             "_whereclause",
             "_having",
             "_order_by_clause",

diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index dd2c7c1fb5514ca74fe06bf02594241c3d8311bb..fe83b163cd24f505b41ef4022a7e40c0b8d96a40 100644 (file)
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -806,10 +806,25 @@ class ClauseAdapter(visitors.ReplacingCloningVisitor):
         return newcol
 
     def replace(self, col):
-        if isinstance(col, FromClause) and self.selectable.is_derived_from(
-            col
-        ):
-            return self.selectable
+        if isinstance(col, FromClause):
+            if self.selectable.is_derived_from(col):
+                return self.selectable
+            elif isinstance(col, Alias) and isinstance(
+                col.element, TableClause
+            ):
+                # we are a SELECT statement and not derived from an alias of a
+                # table (which nonetheless may be a table our SELECT derives
+                # from), so return the alias to prevent futher traversal
+                # or
+                # we are an alias of a table and we are not derived from an
+                # alias of a table (which nonetheless may be the same table
+                # as ours) so, same thing
+                return col
+            else:
+                # other cases where we are a selectable and the element
+                # is another join or selectable that contains a table which our
+                # selectable derives from, that we want to process
+                return None
         elif not isinstance(col, ColumnElement):
             return None
         elif self.include_fn and not self.include_fn(col):

diff --git a/test/sql/test_generative.py b/test/sql/test_generative.py
index 8f06cf3db1a9932a684ea2a1c13d7139ea7dd7b4..262333dc5594b3235d780bcec4ab82ebdbdb0b0b 100644 (file)
--- a/test/sql/test_generative.py
+++ b/test/sql/test_generative.py
@@ -1015,10 +1015,9 @@ class ClauseAdapterTest(fixtures.TestBase, AssertsCompiledSQL):
         )
         s = vis.traverse(s)
 
-        assert t2alias not in s._froms  # not present because it's been
-        # cloned
+        assert t2alias in s._froms  # present because it was not cloned
         assert t1alias in s._froms  # present because the adapter placed
-        # it there
+        # it there and was also not cloned
 
         # correlate list on "s" needs to take into account the full
         # _cloned_set for each element in _froms when correlating
@@ -1592,7 +1591,18 @@ class ClauseAdapterTest(fixtures.TestBase, AssertsCompiledSQL):
         s2 = select([s1]).limit(5).offset(10).alias()
         talias = t1.alias("bar")
 
+        # here is the problem.   s2 is derived from s1 which is derived
+        # from t1
+        assert s2.is_derived_from(t1)
+
+        # however, s2 is not derived from talias, which *is* derived from t1
         assert not s2.is_derived_from(talias)
+
+        # therefore, talias gets its table replaced, except for a rule
+        # we added to ClauseAdapter to stop traversal if the selectable is
+        # not derived from an alias of a table.  This rule was previously
+        # in Alias._copy_internals().
+
         j = s1.outerjoin(talias, s1.c.col1 == talias.c.col1)
 
         self.assert_compile(