- 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 660c4a860d73f486aa4f8641ccd17ee1aac91e76..96c5b3bd990dea20ad39e8b759ec7b39c75321d4 100644 (file)
--- 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 8055aa504bb878a78c9191363e85c2cc2a075dbc..fad8a49dd62bc87f035d34088e4457d15ce61c15 100644 (file)
--- 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 8fdb844b4e2d87f7846763882991b187b6f21b6f..3ff910dd3f0db575797a0d64560f6310f9365db4 100644 (file)
--- 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 6a7e7d6246404a8d77a123d884e82caa2761ad03..6c9895977f870eb1dbbe87541c4a08f6de226cae 100644 (file)
--- 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)