From: Mike Bayer Date: Fri, 6 Sep 2019 16:45:53 +0000 (-0400) Subject: Adjustments to _copy_internals() X-Git-Tag: rel_1_4_0b1~730 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fc48050e5d0c56fc5a6cd85679859e71961c59eb;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git 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 50b1df99ed..30a90348c9 100644 --- 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 97c49f8fcc..00d3826b21 100644 --- 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 dd2c7c1fb5..fe83b163cd 100644 --- 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 8f06cf3db1..262333dc55 100644 --- 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(