From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Thu, 17 Mar 2011 20:22:25 +0000 (-0400)
Subject: - Fixed bug in query.options() whereby a path
X-Git-Tag: rel_0_7b3~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cec64cf86d12cabb2dc8026a7cff50e5308923d5;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- Fixed bug in query.options() whereby a path
applied to a lazyload using string keys could
overlap a same named attribute on the wrong
entity.  Note 0.6 has a more conservative fix
to this.  [ticket:2098]
---

diff --git a/CHANGES b/CHANGES
index 660c4a860d..96c5b3bd99 100644
--- a/CHANGES
+++ b/CHANGES
@@ -56,6 +56,12 @@ CHANGES
     where the parent entity is not fully present.  
     [ticket:2069]
 
+  - Fixed bug in query.options() whereby a path 
+    applied to a lazyload using string keys could 
+    overlap a same named attribute on the wrong 
+    entity.  Note 0.6 has a more conservative fix 
+    to this.  [ticket:2098]
+
 - sql
   - Added a fully descriptive error message for the
     case where Column is subclassed and _make_proxy()
diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 8055aa504b..fad8a49dd6 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -465,9 +465,8 @@ class PropertyOption(MapperOption):
         l = []
         mappers = []
 
-        # _current_path implies we're in a secondary load with an
-        # existing path
-
+        # _current_path implies we're in a 
+        # secondary load with an existing path
         current_path = list(query._current_path)
 
         tokens = deque(self.key)
@@ -477,13 +476,21 @@ class PropertyOption(MapperOption):
                 sub_tokens = token.split(".", 1)
                 token = sub_tokens[0]
                 tokens.extendleft(sub_tokens[1:])
+
+                # exhaust current_path before
+                # matching tokens to entities
+                if current_path:
+                    if current_path[1] == token:
+                        current_path = current_path[2:]
+                        continue
+                    else:
+                        return [], []
+
                 if not entity:
-                    if current_path:
-                        if current_path[1] == token:
-                            current_path = current_path[2:]
-                            continue
-                    entity = self._find_entity_basestring(query, 
-                            token, raiseerr)
+                    entity = self._find_entity_basestring(
+                                        query, 
+                                        token, 
+                                        raiseerr)
                     path_element = entity.path_entity
                     mapper = entity.mapper
                 mappers.append(mapper)
@@ -493,22 +500,32 @@ class PropertyOption(MapperOption):
                     prop = None
             elif isinstance(token, PropComparator):
                 prop = token.property
+
+                # exhaust current_path before
+                # matching tokens to entities
+                if current_path:
+                    if current_path[0:2] == \
+                            [token.parententity, prop.key]:
+                        current_path = current_path[2:]
+                        continue
+                    else:
+                        return [], []
+
                 if not entity:
-                    if current_path:
-                        if current_path[0:2] == [token.parententity,
-                                prop.key]:
-                            current_path = current_path[2:]
-                            continue
-                    entity = self._find_entity_prop_comparator(query,
-                            prop.key, token.parententity, raiseerr)
+                    entity = self._find_entity_prop_comparator(
+                                            query,
+                                            prop.key, 
+                                            token.parententity, 
+                                            raiseerr)
                     if not entity:
                         return [], []
                     path_element = entity.path_entity
                     mapper = entity.mapper
                 mappers.append(prop.parent)
             else:
-                raise sa_exc.ArgumentError('mapper option expects '
-                        'string key or list of attributes')
+                raise sa_exc.ArgumentError(
+                        "mapper option expects "
+                        "string key or list of attributes")
             if prop is None:
                 return [], []
             path = build_path(path_element, prop.key, path)
@@ -521,7 +538,11 @@ class PropertyOption(MapperOption):
                 path_element = path_element
 
         if current_path:
+            # ran out of tokens before 
+            # current_path was exhausted.
+            assert not tokens
             return [], []
+
         return l, mappers
 
 
diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 8fdb844b4e..3ff910dd3f 100644
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -268,15 +268,15 @@ class EagerTest(_fixtures.FixtureTest, testing.AssertsCompiledSQL):
             ((joinedload("orders.items"), ), 10),
             ((
                 joinedload(User.orders, ), 
-                joinedload(User.orders, Order.items), 
+               joinedload(User.orders, Order.items), 
                 joinedload(User.orders, Order.items, Item.keywords), 
-            ), 1),
+           ), 1),
             ((
                 joinedload(User.orders, Order.items, Item.keywords), 
             ), 10),
             ((
-                joinedload(User.orders, Order.items), 
-                joinedload(User.orders, Order.items, Item.keywords), 
+               joinedload(User.orders, Order.items), 
+               joinedload(User.orders, Order.items, Item.keywords), 
             ), 5),
         ]:
             sess = create_session()
diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 6a7e7d6246..6c9895977f 100644
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -1956,6 +1956,16 @@ class OptionsTest(QueryTest):
         opt = self._option_fixture(User.addresses)
         self._assert_path_result(opt, q, [(User, 'addresses')], [User])
 
+    def test_path_on_entity_but_doesnt_match_currentpath(self):
+        # ensure "current path" is fully consumed before
+        # matching against current entities.
+        # see [ticket:2098]
+        sess = Session()
+        q = sess.query(User)
+        opt = self._option_fixture('email_address', 'id')
+        q = sess.query(Address)._with_current_path([class_mapper(User), 'addresses'])
+        self._assert_path_result(opt, q, [], [])
+
     def test_get_path_one_level_with_unrelated(self):
         sess = Session()
         q = sess.query(Order)