- rewritten ClauseAdapter merged from the eager_minus_join branch; this is a much simpler

and "correct" version which will copy all elements exactly once, except for those which were
replaced with target elements.  It also can match a wider variety of target elements including
joins and selects on identity alone.

1 files changed, 68 insertions, 73 deletions

diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 8876f42ba..eed06cfc3 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -3,7 +3,6 @@ from sqlalchemy.sql import expression, visitors
 
 """Utility functions that build upon SQL and Schema constructs."""
 
-
 class TableCollection(object):
     def __init__(self, tables=None):
         self.tables = tables or []
@@ -110,87 +109,86 @@ class ColumnsInClause(visitors.ClauseVisitor):
         if self.selectable.c.get(column.key) is column:
             self.result = True
 
-class AbstractClauseProcessor(visitors.NoColumnVisitor):
-    """Traverse a clause and attempt to convert the contents of container elements
-    to a converted element.
-
-    The conversion operation is defined by subclasses.
+class AbstractClauseProcessor(object):
+    """Traverse and copy a ClauseElement, replacing selected elements based on rules.
+    
+    This class implements its own visit-and-copy strategy but maintains the
+    same public interface as visitors.ClauseVisitor.
     """
-
+    
+    __traverse_options__ = {'column_collections':False}
+    
     def convert_element(self, elem):
         """Define the *conversion* method for this ``AbstractClauseProcessor``."""
 
         raise NotImplementedError()
 
-    def copy_and_process(self, list_):
-        """Copy the container elements in the given list to a new list and
-        process the new list.
-        """
-
+    def chain(self, visitor):
+        # chaining AbstractClauseProcessor and other ClauseVisitor
+        # objects separately.  All the ACP objects are chained on 
+        # their convert_element() method whereas regular visitors
+        # chain on their visit_XXX methods.
+        if isinstance(visitor, AbstractClauseProcessor):
+            attr = '_next_acp'
+        else:
+            attr = '_next'
+        
+        tail = self
+        while getattr(tail, attr, None) is not None:
+            tail = getattr(tail, attr)
+        setattr(tail, attr, visitor)
+        return self
+
+    def copy_and_process(self, list_, stop_on=None):
+        """Copy the given list to a new list, with each element traversed individually."""
+        
         list_ = list(list_)
-        self.process_list(list_)
+        stop_on = util.Set()
+        for i in range(0, len(list_)):
+            list_[i] = self.traverse(list_[i], stop_on=stop_on)
         return list_
 
-    def process_list(self, list_):
-        """Process all elements of the given list in-place."""
-
-        for i in range(0, len(list_)):
-            elem = self.convert_element(list_[i])
-            if elem is not None:
-                list_[i] = elem
-            else:
-                list_[i] = self.traverse(list_[i], clone=True)
-    
-    def visit_grouping(self, grouping):
-        elem = self.convert_element(grouping.elem)
-        if elem is not None:
-            grouping.elem = elem
+    def _convert_element(self, elem, stop_on):
+        v = self
+        while v is not None:
+            newelem = v.convert_element(elem)
+            if newelem:
+                stop_on.add(newelem)
+                return newelem
+            v = getattr(v, '_next_acp', None)
+        return elem._clone()
+        
+    def traverse(self, elem, clone=True, stop_on=None, _clone_toplevel=True):
+        if not clone:
+            raise exceptions.ArgumentError("AbstractClauseProcessor 'clone' argument must be True")
             
-    def visit_clauselist(self, clist):
-        for i in range(0, len(clist.clauses)):
-            n = self.convert_element(clist.clauses[i])
-            if n is not None:
-                clist.clauses[i] = n
-    
-    def visit_unary(self, unary):
-        elem = self.convert_element(unary.element)
-        if elem is not None:
-            unary.element = elem
+        if stop_on is None:
+            stop_on = util.Set()
             
-    def visit_binary(self, binary):
-        elem = self.convert_element(binary.left)
-        if elem is not None:
-            binary.left = elem
-        elem = self.convert_element(binary.right)
-        if elem is not None:
-            binary.right = elem
-    
-    def visit_join(self, join):
-        elem = self.convert_element(join.left)
-        if elem is not None:
-            join.left = elem
-        elem = self.convert_element(join.right)
-        if elem is not None:
-            join.right = elem
-        join._init_primary_key()
+        if elem in stop_on:
+            return elem
+        
+        if _clone_toplevel:
+            elem = self._convert_element(elem, stop_on)
+            if elem in stop_on:
+                return elem
             
-    def visit_select(self, select):
-        fr = util.OrderedSet()
-        for elem in select._froms:
-            n = self.convert_element(elem)
-            if n is not None:
-                fr.add((elem, n))
-        select._recorrelate_froms(fr)
-
-        col = []
-        for elem in select._raw_columns:
-            n = self.convert_element(elem)
-            if n is None:
-                col.append(elem)
-            else:
-                col.append(n)
-        select._raw_columns = col
-    
+        def clone(element):
+            return self._convert_element(element, stop_on)
+        elem._copy_internals(clone=clone)
+        
+        v = getattr(self, '_next', None)
+        while v is not None:
+            meth = getattr(v, "visit_%s" % elem.__visit_name__, None)
+            if meth:
+                meth(elem)
+            v = getattr(v, '_next', None)
+        
+        for e in elem.get_children(**self.__traverse_options__):
+            if e not in stop_on:
+                self.traverse(e, stop_on=stop_on, _clone_toplevel=False)
+        return elem
+        
 class ClauseAdapter(AbstractClauseProcessor):
     """Given a clause (like as in a WHERE criterion), locate columns
     which are embedded within a given selectable, and changes those
@@ -243,9 +241,6 @@ class ClauseAdapter(AbstractClauseProcessor):
                 newcol = self.selectable.corresponding_column(equiv, raiseerr=False, require_embedded=True, keys_ok=False)
                 if newcol:
                     return newcol
-        #if newcol is None:
-        #    self.traverse(col)
-        #    return col
         return newcol