Added a new exception to detect the case where two

subclasses are being loaded using with_polymorphic(),
each subclass contains a relationship attribute of the same
name, and eager loading is being applied to one or both.
This is an ongoing bug which can't be immediately fixed,
so since the results are usually wrong in any case it raises an
exception for now.   0.7 has the same issue, so an exception
raise here probably means the code was returning the wrong
data in 0.7.  [ticket:2614]

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 8046a88920ff91f42a87a7e6b4a722a033f5332a..45a612b4915fe16039d2413c61139ad5f27004d7 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -6,6 +6,20 @@
 .. changelog::
     :version: 0.8.0b2
 
+    .. change::
+        :tags: orm, bug
+        :ticket: 2614
+
+      Added a new exception to detect the case where two
+      subclasses are being loaded using with_polymorphic(),
+      each subclass contains a relationship attribute of the same
+      name, and eager loading is being applied to one or both.
+      This is an ongoing bug which can't be immediately fixed,
+      so since the results are usually wrong in any case it raises an
+      exception for now.   0.7 has the same issue, so an exception
+      raise here probably means the code was returning the wrong
+      data in 0.7.
+
     .. change::
         :tags: sql, bug
 

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 39329f9b1175e2e0a2c45f6a2a7c03f785390405..586ec4b4e9e01903d30f49234fee345634bf17fc 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -303,6 +303,17 @@ class AbstractRelationshipLoader(LoaderStrategy):
         self.uselist = self.parent_property.uselist
 
 
+    def _warn_existing_path(self):
+        raise sa_exc.InvalidRequestError(
+            "Eager loading cannot currently function correctly when two or "
+            "more "
+            "same-named attributes associated with multiple polymorphic "
+            "classes "
+            "of the same base are present.   Encountered more than one "
+            r"eager path for attribute '%s' on mapper '%s'." %
+            (self.key, self.parent.base_mapper, ))
+
+
 class NoLoader(AbstractRelationshipLoader):
     """Provide loading behavior for a :class:`.RelationshipProperty`
     with "lazy=None".
@@ -746,7 +757,9 @@ class SubqueryLoader(AbstractRelationshipLoader):
 
         # add new query to attributes to be picked up
         # by create_row_processor
-        path.set(context, "subquery", q)
+        existing = path.replace(context, "subquery", q)
+        if existing:
+            self._warn_existing_path()
 
     def _get_leftmost(self, subq_path):
         subq_path = subq_path.path
@@ -1091,7 +1104,9 @@ class JoinedLoader(AbstractRelationshipLoader):
         )
 
         add_to_collection = context.secondary_columns
-        path.set(context, "eager_row_processor", clauses)
+        existing = path.replace(context, "eager_row_processor", clauses)
+        if existing:
+            self._warn_existing_path()
         return clauses, adapter, add_to_collection, allow_innerjoin
 
     def _create_eager_join(self, context, entity,

diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 97baffc9aa3aac2d90b5bd19c17969e29b291e32..e5e725138cb200d173f5f84c7a72eb9693f2c3b1 100644 (file)
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -279,6 +279,12 @@ class PathRegistry(object):
     def set(self, reg, key, value):
         reg._attributes[(key, self.reduced_path)] = value
 
+    def replace(self, reg, key, value):
+        path_key = (key, self.reduced_path)
+        existing = reg._attributes.get(path_key, None)
+        reg._attributes[path_key] = value
+        return existing
+
     def setdefault(self, reg, key, value):
         reg._attributes.setdefault((key, self.reduced_path), value)
 

diff --git a/test/orm/test_eager_relations.py b/test/orm/test_eager_relations.py
index 39a7a7301daa21ca982e91fb8362d55840fd932b..ae3853e75fde2de3860737d5181bc227bb310106 100644 (file)
--- a/test/orm/test_eager_relations.py
+++ b/test/orm/test_eager_relations.py
@@ -2659,4 +2659,131 @@ class CyclicalInheritingEagerTestTwo(fixtures.DeclarativeMappedTest,
 
         session.close_all()
         d = session.query(Director).options(joinedload('*')).first()
-        assert len(list(session)) == 3
\ No newline at end of file
+        assert len(list(session)) == 3
+
+
+
+class WarnFor2614TestBase(object):
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table('a', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('type', String(50)),
+        )
+        Table('b', metadata,
+            Column('id', Integer, ForeignKey('a.id'), primary_key=True),
+        )
+        Table('c', metadata,
+            Column('id', Integer, ForeignKey('a.id'), primary_key=True),
+        )
+        Table('d', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('bid', Integer, ForeignKey('b.id')),
+            Column('cid', Integer, ForeignKey('c.id')),
+        )
+
+    def _mapping(self, lazy_b=True, lazy_c=True):
+        class A(object):
+            pass
+
+        class B(A):
+            pass
+
+        class C(A):
+            pass
+
+        class D(object):
+            pass
+
+        mapper(A, self.tables.a, polymorphic_on=self.tables.a.c.type)
+        mapper(B, self.tables.b, inherits=A, polymorphic_identity='b',
+                    properties={
+                        'ds': relationship(D, lazy=lazy_b)
+                    })
+        mapper(C, self.tables.c, inherits=A, polymorphic_identity='c',
+                    properties={
+                        'ds': relationship(D, lazy=lazy_c)
+                    })
+        mapper(D, self.tables.d)
+
+
+        return  A, B, C, D
+
+    def _assert_raises(self, fn):
+        assert_raises_message(
+            sa.exc.InvalidRequestError,
+            "Eager loading cannot currently function correctly when two or more "
+            r"same\-named attributes associated with multiple polymorphic classes "
+            "of the same base are present.   Encountered more than one "
+            r"eager path for attribute 'ds' on mapper 'Mapper\|A\|a'.",
+            fn
+        )
+
+    def test_poly_both_eager(self):
+        A, B, C, D = self._mapping(lazy_b=self.eager_name,
+                                    lazy_c=self.eager_name)
+
+        session = Session()
+        self._assert_raises(
+            session.query(A).with_polymorphic('*').all
+        )
+
+    def test_poly_one_eager(self):
+        A, B, C, D = self._mapping(lazy_b=self.eager_name, lazy_c=True)
+
+        session = Session()
+        session.query(A).with_polymorphic('*').all()
+
+    def test_poly_both_option(self):
+        A, B, C, D = self._mapping()
+
+        session = Session()
+        self._assert_raises(
+            session.query(A).with_polymorphic('*').options(
+                        self.eager_option(B.ds), self.eager_option(C.ds)).all
+        )
+
+    def test_poly_one_option_bs(self):
+        A, B, C, D = self._mapping()
+
+        session = Session()
+
+        # sucks, can't even do eager() on just one of them, as B.ds
+        # hits for both
+        self._assert_raises(
+            session.query(A).with_polymorphic('*').\
+                options(self.eager_option(B.ds)).all
+        )
+
+    def test_poly_one_option_cs(self):
+        A, B, C, D = self._mapping()
+
+        session = Session()
+
+        # sucks, can't even do eager() on just one of them, as B.ds
+        # hits for both
+        self._assert_raises(
+            session.query(A).with_polymorphic('*').\
+                options(self.eager_option(C.ds)).all
+        )
+
+    def test_single_poly_one_option_bs(self):
+        A, B, C, D = self._mapping()
+
+        session = Session()
+
+        session.query(A).with_polymorphic(B).\
+            options(self.eager_option(B.ds)).all()
+
+    def test_lazy_True(self):
+        A, B, C, D = self._mapping()
+
+        session = Session()
+        session.query(A).with_polymorphic('*').all()
+
+class WarnFor2614Test(WarnFor2614TestBase, fixtures.MappedTest):
+    eager_name = "joined"
+
+    def eager_option(self, arg):
+        return joinedload(arg)

diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index f6e2299b427ea89d1ad4eb3b126143369d5aecf2..000f4abaf896ddceb15bb3a89b997aa651278e21 100644 (file)
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -1382,4 +1382,12 @@ class CyclicalInheritingEagerTestTwo(fixtures.DeclarativeMappedTest,
 
         session.close_all()
         d = session.query(Director).options(subqueryload('*')).first()
-        assert len(list(session)) == 3
\ No newline at end of file
+        assert len(list(session)) == 3
+
+from . import test_eager_relations
+
+class WarnFor2614Test(test_eager_relations.WarnFor2614TestBase, fixtures.MappedTest):
+    eager_name = "subquery"
+
+    def eager_option(self, arg):
+        return subqueryload(arg)