- [bug] Raise an exception if xyzload_all() is

used inappropriately with two non-connected
relationships.  [ticket:2370]

diff --git a/CHANGES b/CHANGES
index 0c42b1f78374fe7a357ac78ffcacfdf9ea37e184..afe9168cd807e62a349d7d4ede969fbe820fcbb0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -21,6 +21,10 @@ CHANGES
     superclass as "polymorphic_on" failed to resolve
     the underlying Column.  [ticket:2345]
 
+  - [bug] Raise an exception if xyzload_all() is
+    used inappropriately with two non-connected
+    relationships.  [ticket:2370]
+
   - [feature] Added "class_registry" argument to
     declarative_base().  Allows two or more declarative
     bases to share the same registry of class names.

diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 7f2875e5a1de9504ed45e28da4aecb2d6dbd6834..bda48cbb130f5d96c3b45d28cd0a36a3ba21028e 100644 (file)
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -568,7 +568,12 @@ class PropertyOption(MapperOption):
                         "mapper option expects "
                         "string key or list of attributes")
             assert prop is not None
+            if raiseerr and not prop.parent.common_parent(mapper):
+                raise sa_exc.ArgumentError("Attribute '%s' does not "
+                            "link from element '%s'" % (token, path_element))
+
             path = build_path(path_element, prop.key, path)
+
             l.append(path)
             if getattr(token, '_of_type', None):
                 path_element = mapper = token._of_type
@@ -580,8 +585,6 @@ class PropertyOption(MapperOption):
                         "refer to a mapped entity" %
                         (token, entity)
                     )
-            if path_element:
-                path_element = path_element
 
         if current_path:
             # ran out of tokens before 

diff --git a/test/lib/pickleable.py b/test/lib/pickleable.py
index 550e0e5020e7ce0756652d05d01d0960353c18ad..98d104f1d9d50af6ee0b558d42f225de01acbb46 100644 (file)
--- a/test/lib/pickleable.py
+++ b/test/lib/pickleable.py
@@ -8,6 +8,9 @@ class User(fixtures.ComparableEntity):
 class Order(fixtures.ComparableEntity):
     pass
 
+class Dingaling(fixtures.ComparableEntity):
+    pass
+
 class EmailUser(User):
     pass
 

diff --git a/test/orm/test_pickled.py b/test/orm/test_pickled.py
index 920159dc1820437ea5c0ff4a364cef0a844a9242..aa560a2e09f32e508c560c81ee34021bddc012ac 100644 (file)
--- a/test/orm/test_pickled.py
+++ b/test/orm/test_pickled.py
@@ -13,7 +13,8 @@ from sqlalchemy.orm import mapper, relationship, create_session, \
                             lazyload, aliased
 from test.lib import fixtures
 from test.orm import _fixtures
-from test.lib.pickleable import User, Address, Order, Child1, Child2, Parent, Screen, EmailUser
+from test.lib.pickleable import User, Address, Dingaling, Order, \
+    Child1, Child2, Parent, Screen, EmailUser
 
 
 class PickleTest(fixtures.MappedTest):
@@ -42,6 +43,13 @@ class PickleTest(fixtures.MappedTest):
                   test_needs_acid=True,
                   test_needs_fk=True
         )
+        Table("dingalings", metadata,
+              Column('id', Integer, primary_key=True, test_needs_autoincrement=True),
+              Column('address_id', None, ForeignKey('addresses.id')),
+              Column('data', String(30)),
+              test_needs_acid=True,
+              test_needs_fk=True
+        )
 
 
     def test_transient(self):
@@ -261,13 +269,17 @@ class PickleTest(fixtures.MappedTest):
 
 
     def test_options_with_descriptors(self):
-        users, addresses = (self.tables.users,
-                                self.tables.addresses)
+        users, addresses, dingalings = (self.tables.users,
+                                self.tables.addresses,
+                                self.tables.dingalings)
 
         mapper(User, users, properties={
             'addresses':relationship(Address, backref="user")
         })
-        mapper(Address, addresses)
+        mapper(Address, addresses, properties={
+            'dingaling':relationship(Dingaling)
+        })
+        mapper(Dingaling, dingalings)
         sess = create_session()
         u1 = User(name='ed')
         u1.addresses.append(Address(email_address='ed@bar.com'))
@@ -280,13 +292,12 @@ class PickleTest(fixtures.MappedTest):
             sa.orm.joinedload("addresses"),
             sa.orm.defer("name"),
             sa.orm.defer(User.name),
-            sa.orm.joinedload("addresses", User.addresses),
+            sa.orm.joinedload("addresses", Address.dingaling),
         ]:
             opt2 = pickle.loads(pickle.dumps(opt))
             eq_(opt.key, opt2.key)
 
         u1 = sess.query(User).options(opt).first()
-
         u2 = pickle.loads(pickle.dumps(u1))
 
     def test_collection_setstate(self):

diff --git a/test/orm/test_query.py b/test/orm/test_query.py
index 8a8ef0d4e561e010c513a788484f658265313d1d..24974ae7e3f22fb2ba8388923df543e157c7d876 100644 (file)
--- a/test/orm/test_query.py
+++ b/test/orm/test_query.py
@@ -2454,7 +2454,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_nonexistent_PropComparator(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword],
             (joinedload(Item.keywords), ),
             r"Can't find property 'keywords' on any entity specified "
@@ -2464,7 +2464,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
 
     def test_option_against_nonexistent_basestring(self):
         Item = self.classes.Item
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Item],
             (joinedload("foo"), ),
             r"Can't find property named 'foo' on the mapped "
@@ -2473,7 +2473,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
 
     def test_option_against_nonexistent_twolevel_basestring(self):
         Item = self.classes.Item
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Item],
             (joinedload("keywords.foo"), ),
             r"Can't find property named 'foo' on the mapped entity "
@@ -2482,7 +2482,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
 
     def test_option_against_nonexistent_twolevel_all(self):
         Item = self.classes.Item
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Item],
             (joinedload_all("keywords.foo"), ),
             r"Can't find property named 'foo' on the mapped entity "
@@ -2494,7 +2494,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_non_relation_basestring(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all("keywords"), ),
             r"Attribute 'keywords' of entity 'Mapper\|Keyword\|keywords' "
@@ -2506,7 +2506,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_multi_non_relation_basestring(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all("keywords"), ),
             r"Attribute 'keywords' of entity 'Mapper\|Keyword\|keywords' "
@@ -2515,7 +2515,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
 
     def test_option_against_wrong_entity_type_basestring(self):
         Item = self.classes.Item
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Item],
             (joinedload_all("id", "keywords"), ),
             r"Attribute 'id' of entity 'Mapper\|Item\|items' does not "
@@ -2525,7 +2525,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_multi_non_relation_twolevel_basestring(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all("id", "keywords"), ),
             r"Attribute 'id' of entity 'Mapper\|Keyword\|keywords' "
@@ -2535,7 +2535,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_multi_nonexistent_basestring(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all("description"), ),
             r"Can't find property named 'description' on the mapped "
@@ -2545,7 +2545,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_multi_no_entities_basestring(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword.id, Item.id],
             (joinedload_all("keywords"), ),
             r"Query has only expression-based entities - can't find property "
@@ -2555,7 +2555,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_wrong_multi_entity_type_attr_one(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all(Keyword.id, Item.keywords), ),
             r"Attribute 'Keyword.id' of entity 'Mapper\|Keyword\|keywords' "
@@ -2565,7 +2565,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_wrong_multi_entity_type_attr_two(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword, Item],
             (joinedload_all(Keyword.keywords, Item.keywords), ),
             r"Attribute 'Keyword.keywords' of entity 'Mapper\|Keyword\|keywords' "
@@ -2575,7 +2575,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_option_against_wrong_multi_entity_type_attr_three(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Keyword.id, Item.id],
             (joinedload_all(Keyword.keywords, Item.keywords), ),
             r"Query has only expression-based entities - "
@@ -2585,14 +2585,33 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
     def test_wrong_type_in_option(self):
         Item = self.classes.Item
         Keyword = self.classes.Keyword
-        self._assert_eager_not_found_exception(
+        self._assert_eager_with_entity_exception(
             [Item],
             (joinedload_all(Keyword), ),
             r"mapper option expects string key or list of attributes"
         )
 
+    def test_non_contiguous_all_option(self):
+        User = self.classes.User
+        self._assert_eager_with_entity_exception(
+            [User],
+            (joinedload_all(User.addresses, User.orders), ),
+            r"Attribute 'User.orders' does not link "
+            "from element 'Mapper|Address|addresses'"
+        )
+
     @classmethod
     def setup_mappers(cls):
+        users, User, addresses, Address, orders, Order = (
+                    cls.tables.users, cls.classes.User, 
+                    cls.tables.addresses, cls.classes.Address,
+                    cls.tables.orders, cls.classes.Order)
+        mapper(User, users, properties={
+            'addresses':relationship(Address),
+            'orders':relationship(Order)
+        })
+        mapper(Address, addresses)
+        mapper(Order, orders)
         keywords, items, item_keywords, Keyword, Item = (cls.tables.keywords,
                                 cls.tables.items,
                                 cls.tables.item_keywords,
@@ -2613,7 +2632,7 @@ class OptionsNoPropTest(_fixtures.FixtureTest):
         key = ('loaderstrategy', (class_mapper(Item), 'keywords'))
         assert key in q._attributes
 
-    def _assert_eager_not_found_exception(self, entity_list, options, 
+    def _assert_eager_with_entity_exception(self, entity_list, options, 
                                 message):
         assert_raises_message(sa.exc.ArgumentError, 
                                 message,