- [bug] Fixed bug in expression annotation

mechanics which could lead to incorrect
rendering of SELECT statements with aliases
and joins, particularly when using
column_property().  [ticket:2453]

diff --git a/CHANGES b/CHANGES
index 03173beed54441f1e4ccf6784cfd68874e310345..2dcceddb6ee1fbbbe487131a34e59608e461ea6b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -18,6 +18,12 @@ CHANGES
     in a merge() operation, raising an error.
     [ticket:2449]
 
+  - [bug] Fixed bug in expression annotation
+    mechanics which could lead to incorrect
+    rendering of SELECT statements with aliases
+    and joins, particularly when using 
+    column_property().  [ticket:2453]
+
 - postgresql
   - [feature] Added new for_update/with_lockmode()
     options for Postgresql: for_update="read"/

diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index 97975441e48e2414f204d4659b04b52b6656076c..8d2b5ecfd82eec1c300d5de258ad12eb48ad94b3 100644 (file)
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -404,22 +404,30 @@ for cls in expression.__dict__.values() + [schema.Column, schema.Table]:
         exec "annotated_classes[cls] = Annotated%s" % (cls.__name__)
 
 def _deep_annotate(element, annotations, exclude=None):
-    """Deep copy the given ClauseElement, annotating each element with the given annotations dictionary.
+    """Deep copy the given ClauseElement, annotating each element 
+    with the given annotations dictionary.
 
     Elements within the exclude collection will be cloned but not annotated.
 
     """
+    cloned = util.column_dict()
+
     def clone(elem):
         # check if element is present in the exclude list.
         # take into account proxying relationships.
-        if exclude and \
+        if elem in cloned:
+            return cloned[elem]
+        elif exclude and \
                     hasattr(elem, 'proxy_set') and \
                     elem.proxy_set.intersection(exclude):
-            elem = elem._clone()
+            newelem = elem._clone()
         elif annotations != elem._annotations:
-            elem = elem._annotate(annotations.copy())
-        elem._copy_internals(clone=clone)
-        return elem
+            newelem = elem._annotate(annotations)
+        else:
+            newelem = elem
+        newelem._copy_internals(clone=clone)
+        cloned[elem] = newelem
+        return newelem
 
     if element is not None:
         element = clone(element)
@@ -428,10 +436,14 @@ def _deep_annotate(element, annotations, exclude=None):
 def _deep_deannotate(element):
     """Deep copy the given element, removing all annotations."""
 
+    cloned = util.column_dict()
+
     def clone(elem):
-        elem = elem._deannotate()
-        elem._copy_internals(clone=clone)
-        return elem
+        if elem not in cloned:
+            newelem = elem._deannotate()
+            newelem._copy_internals(clone=clone)
+            cloned[elem] = newelem
+        return cloned[elem]
 
     if element is not None:
         element = clone(element)

diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index 9048192152e26157cbddaa7da8c8e4fe955f6db1..7befa8283f976becf1024e2f02303587fa169557 100644 (file)
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -1134,6 +1134,30 @@ class AnnotationsTest(fixtures.TestBase):
         assert b4.left is bin.left  # since column is immutable
         assert b4.right is not bin.right is not b2.right is not b3.right
 
+    def test_annotate_unique_traversal(self):
+        """test that items are copied only once during
+        annotate, deannotate traversal"""
+        table1 = table('table1', column('x'))
+        table2 = table('table1', column('y'))
+        a1 = table1.alias()
+        s = select([a1.c.x]).select_from(
+                a1.join(table2, a1.c.x==table2.c.y)
+            )
+
+        for sel in (
+            sql_util._deep_deannotate(s),
+            sql_util._deep_annotate(s, {'foo':'bar'}),
+            visitors.cloned_traverse(s, {}, {}),
+            visitors.replacement_traverse(s, {}, lambda x:None)
+        ):
+            # the columns clause isn't changed at all
+            assert sel._raw_columns[0].table is a1
+            # the from objects are internally consistent,
+            # i.e. the Alias at position 0 is the same
+            # Alias in the Join object in position 1
+            assert sel._froms[0] is sel._froms[1].left
+            eq_(str(s), str(sel))
+
     def test_bind_unique_test(self):
         t1 = table('t', column('a'), column('b'))