From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 17 Dec 2013 00:17:41 +0000 (-0500)
Subject: - An adjustment to the :func:`.subqueryload` strategy which ensures that
X-Git-Tag: rel_0_8_5~73
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dcb7e7759ae85b2cc4d6a93fffd9746365ffe45a;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

- An adjustment to the :func:`.subqueryload` strategy which ensures that
the query runs after the loading process has begun; this is so that
the subqueryload takes precedence over other loaders that may be
hitting the same attribute due to other eager/noload situations
at the wrong time. [ticket:2887]
---

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 3b30333f28..9e8d2af1d6 100644
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -11,6 +11,17 @@
 .. changelog::
     :version: 0.8.5
 
+    .. change::
+        :tags: bug, orm
+        :versions: 0.9.0b2
+        :tickets: 2887
+
+        An adjustment to the :func:`.subqueryload` strategy which ensures that
+        the query runs after the loading process has begun; this is so that
+        the subqueryload takes precedence over other loaders that may be
+        hitting the same attribute due to other eager/noload situations
+        at the wrong time.
+
     .. change::
         :tags: bug, orm
         :versions: 0.9.0b2
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index b8ab55da4f..8b4c4f098e 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -931,6 +931,35 @@ class SubqueryLoader(AbstractRelationshipLoader):
             q = q.order_by(*eager_order_by)
         return q
 
+    class _SubqCollections(object):
+        """Given a :class:`.Query` used to emit the "subquery load",
+        provide a load interface that executes the query at the
+        first moment a value is needed.
+
+        """
+        _data = None
+
+        def __init__(self, subq):
+            self.subq = subq
+
+        def get(self, key, default):
+            if self._data is None:
+                self._load()
+            return self._data.get(key, default)
+
+        def _load(self):
+            self._data = dict(
+                (k, [vv[0] for vv in v])
+                for k, v in itertools.groupby(
+                    self.subq,
+                    lambda x: x[1:]
+                )
+            )
+
+        def loader(self, state, dict_, row):
+            if self._data is None:
+                self._load()
+
     def create_row_processor(self, context, path,
                                     mapper, row, adapter):
         if not self.parent.class_manager[self.key].impl.supports_population:
@@ -953,12 +982,7 @@ class SubqueryLoader(AbstractRelationshipLoader):
         # call upon create_row_processor again
         collections = path.get(context, "collections")
         if collections is None:
-            collections = dict(
-                    (k, [v[0] for v in v])
-                    for k, v in itertools.groupby(
-                        subq,
-                        lambda x: x[1:]
-                    ))
+            collections = self._SubqCollections(subq)
             path.set(context, 'collections', collections)
 
         if adapter:
@@ -978,7 +1002,7 @@ class SubqueryLoader(AbstractRelationshipLoader):
             state.get_impl(self.key).\
                     set_committed_value(state, dict_, collection)
 
-        return load_collection_from_subq, None, None
+        return load_collection_from_subq, None, None, collections.loader
 
     def _create_scalar_loader(self, collections, local_cols):
         def load_scalar_from_subq(state, dict_, row):
@@ -996,7 +1020,7 @@ class SubqueryLoader(AbstractRelationshipLoader):
             state.get_impl(self.key).\
                     set_committed_value(state, dict_, scalar)
 
-        return load_scalar_from_subq, None, None
+        return load_scalar_from_subq, None, None, collections.loader
 
 
 log.class_logger(SubqueryLoader)
diff --git a/test/orm/test_subquery_relations.py b/test/orm/test_subquery_relations.py
index cad29ebfa7..26feff689e 100644
--- a/test/orm/test_subquery_relations.py
+++ b/test/orm/test_subquery_relations.py
@@ -10,6 +10,7 @@ from sqlalchemy.testing import eq_, assert_raises, \
     assert_raises_message
 from sqlalchemy.testing.assertsql import CompiledSQL
 from sqlalchemy.testing import fixtures
+from sqlalchemy.testing.entities import ComparableEntity
 from test.orm import _fixtures
 import sqlalchemy as sa
 
@@ -1764,3 +1765,56 @@ class SubqueryloadDistinctTest(fixtures.DeclarativeMappedTest,
                     (1, 'Woody Allen', 1)
             ]
         )
+
+
+class JoinedNoLoadConflictTest(fixtures.DeclarativeMappedTest):
+    """test for [ticket:2887]"""
+
+    @classmethod
+    def setup_classes(cls):
+        Base = cls.DeclarativeBasic
+
+        class Parent(ComparableEntity, Base):
+            __tablename__ = 'parent'
+
+            id = Column(Integer, primary_key=True)
+            name = Column(String(20))
+
+            children = relationship('Child',
+                                back_populates='parent',
+                                lazy='noload'
+                            )
+
+        class Child(ComparableEntity, Base):
+            __tablename__ = 'child'
+
+            id = Column(Integer, primary_key=True)
+            name = Column(String(20))
+            parent_id = Column(Integer, ForeignKey('parent.id'))
+
+            parent = relationship('Parent', back_populates='children', lazy='joined')
+
+    @classmethod
+    def insert_data(cls):
+        Parent = cls.classes.Parent
+        Child = cls.classes.Child
+
+        s = Session()
+        s.add(Parent(name='parent', children=[Child(name='c1')]))
+        s.commit()
+
+    def test_subqueryload_on_joined_noload(self):
+        Parent = self.classes.Parent
+        Child = self.classes.Child
+
+        s = Session()
+
+        # here we have Parent->subqueryload->Child->joinedload->parent->noload->children.
+        # the actual subqueryload has to emit *after* we've started populating
+        # Parent->subqueryload->child.
+        parent = s.query(Parent).options([subqueryload('children')]).first()
+        eq_(
+            parent.children,
+            [Child(name='c1')]
+        )
+