Commits

Mike Bayer  committed 8b14582

- Feature enhancement: joined and subquery
loading will now traverse already-present related
objects and collections in search of unpopulated
attributes throughout the scope of the eager load
being defined, so that the eager loading that is
specified via mappings or query options
unconditionally takes place for the full depth,
populating whatever is not already populated.
Previously, this traversal would stop if a related
object or collection were already present leading
to inconsistent behavior (though would save on
loads/cycles for an already-loaded graph). For a
subqueryload, this means that the additional
SELECT statements emitted by subqueryload will
invoke unconditionally, no matter how much of the
existing graph is already present (hence the
controversy). The previous behavior of "stopping"
is still in effect when a query is the result of
an attribute-initiated lazyload, as otherwise an
"N+1" style of collection iteration can become
needlessly expensive when the same related object
is encountered repeatedly. There's also an
as-yet-not-public generative Query method
_with_invoke_all_eagers()
which selects old/new behavior [ticket:2213]

  • Participants
  • Parent commits d20d846

Comments (0)

Files changed (7)

 0.7.2
 =====
 - orm
+  - Feature enhancement: joined and subquery
+    loading will now traverse already-present related
+    objects and collections in search of unpopulated
+    attributes throughout the scope of the eager load
+    being defined, so that the eager loading that is
+    specified via mappings or query options
+    unconditionally takes place for the full depth,
+    populating whatever is not already populated.
+    Previously, this traversal would stop if a related
+    object or collection were already present leading
+    to inconsistent behavior (though would save on
+    loads/cycles for an already-loaded graph). For a
+    subqueryload, this means that the additional
+    SELECT statements emitted by subqueryload will
+    invoke unconditionally, no matter how much of the
+    existing graph is already present (hence the
+    controversy). The previous behavior of "stopping"
+    is still in effect when a query is the result of
+    an attribute-initiated lazyload, as otherwise an
+    "N+1" style of collection iteration can become
+    needlessly expensive when the same related object
+    is encountered repeatedly. There's also an 
+    as-yet-not-public generative Query method 
+    _with_invoke_all_eagers()
+    which selects old/new behavior [ticket:2213]
+
   - Fixed subtle bug that caused SQL to blow
     up if: column_property() against subquery +
     joinedload + LIMIT + order by the column

File lib/sqlalchemy/orm/interfaces.py

       on a particular mapped instance.
 
     """
-
     def __init__(self, parent):
         self.parent_property = parent
         self.is_class_level = False

File lib/sqlalchemy/orm/mapper.py

 
         new_populators = []
         existing_populators = []
+        eager_populators = []
         load_path = context.query._current_path + path
 
         def populate_state(state, dict_, row, isnew, only_load_props):
             if not new_populators:
                 self._populators(context, path, reduced_path, row, adapter,
                                 new_populators,
-                                existing_populators
+                                existing_populators,
+                                eager_populators
                 )
 
             if isnew:
             else:
                 populators = existing_populators
 
-            if only_load_props:
+            if only_load_props is None:
+                for key, populator in populators:
+                    populator(state, dict_, row)
+            elif only_load_props:
                 for key, populator in populators:
                     if key in only_load_props:
                         populator(state, dict_, row)
-            else:
-                for key, populator in populators:
-                    populator(state, dict_, row)
 
         session_identity_map = context.session.identity_map
 
         populate_instance = listeners.populate_instance or None
         append_result = listeners.append_result or None
         populate_existing = context.populate_existing or self.always_refresh
+        invoke_all_eagers = context.invoke_all_eagers
+
         if self.allow_partial_pks:
             is_not_primary_key = _none_set.issuperset
         else:
             is_not_primary_key = _none_set.issubset
 
         def _instance(row, result):
+            if not new_populators and invoke_all_eagers:
+                self._populators(context, path, reduced_path, row, adapter,
+                                new_populators,
+                                existing_populators,
+                                eager_populators
+                )
+
             if translate_row:
                 for fn in translate_row:
                     ret = fn(self, context, row)
                 elif isnew:
                     state.manager.dispatch.refresh(state, context, only_load_props)
 
-            elif state in context.partials or state.unloaded:
+            elif state in context.partials or state.unloaded or eager_populators:
                 # state is having a partial set of its attributes
                 # refreshed.  Populate those attributes,
                 # and add to the "context.partials" collection.
-
                 if state in context.partials:
                     isnew = False
                     (d_, attrs) = context.partials[state]
                 else:
                     populate_state(state, dict_, row, isnew, attrs)
 
+                for key, pop in eager_populators:
+                    if key not in state.unloaded:
+                        pop(state, dict_, row)
+
                 if isnew:
                     state.manager.dispatch.refresh(state, context, attrs)
 
         return _instance
 
     def _populators(self, context, path, reduced_path, row, adapter,
-            new_populators, existing_populators):
+            new_populators, existing_populators, eager_populators):
         """Produce a collection of attribute level row processor callables."""
 
         delayed_populators = []
+        pops = (new_populators, existing_populators, delayed_populators, eager_populators)
         for prop in self._props.itervalues():
-            newpop, existingpop, delayedpop = prop.create_row_processor(
-                                                    context, path, 
-                                                    reduced_path,
-                                                    self, row, adapter)
-            if newpop:
-                new_populators.append((prop.key, newpop))
-            if existingpop:
-                existing_populators.append((prop.key, existingpop))
-            if delayedpop:
-                delayed_populators.append((prop.key, delayedpop))
+            for i, pop in enumerate(prop.create_row_processor(
+                                        context, path, 
+                                        reduced_path,
+                                        self, row, adapter)):
+                if pop is not None:
+                    pops[i].append((prop.key, pop))
+
         if delayed_populators:
             new_populators.extend(delayed_populators)
 

File lib/sqlalchemy/orm/query.py

     _statement = None
     _correlate = frozenset()
     _populate_existing = False
+    _invoke_all_eagers = True
     _version_check = False
     _autoflush = True
     _current_path = ()
         """
         self._populate_existing = True
 
+    @_generative()
+    def _with_invoke_all_eagers(self, value):
+        """Set the 'invoke all eagers' flag which causes joined- and
+        subquery loaders to traverse into already-loaded related objects
+        and collections.
+        
+        Default is that of :attr:`.Query._invoke_all_eagers`.
+
+        """
+        self._invoke_all_eagers = value
+
     def with_parent(self, instance, property=None):
         """Add filtering criterion that relates the given instance
         to a child object or collection, using its attribute state 
         self.query = query
         self.session = query.session
         self.populate_existing = query._populate_existing
+        self.invoke_all_eagers = query._invoke_all_eagers
         self.version_check = query._version_check
         self.refresh_state = query._refresh_state
         self.primary_columns = []

File lib/sqlalchemy/orm/strategies.py

 
         q = session.query(prop_mapper)._adapt_all_clauses()
 
+        q = q._with_invoke_all_eagers(False)
+
         # don't autoflush on pending
         if pending:
             q = q.autoflush(False)
     using joined eager loading.
     
     """
-
     def init(self):
         super(JoinedLoader, self).init()
         self.join_depth = self.parent_property.join_depth
                                 our_reduced_path + (self.mapper.base_mapper,),
                                 eager_adapter)
 
+            def eager_exec(state, dict_, row):
+                _instance(row, None)
+
             if not self.uselist:
                 def new_execute(state, dict_, row):
                     # set a scalar object instance directly on the parent
                             "Multiple rows returned with "
                             "uselist=False for eagerly-loaded attribute '%s' "
                             % self)
-                return new_execute, existing_execute, None
+                return new_execute, existing_execute, None, eager_exec
             else:
                 def new_execute(state, dict_, row):
                     collection = attributes.init_state_collection(
                                                 'append_without_event')
                         context.attributes[(state, key)] = result_list
                     _instance(row, result_list)
-            return new_execute, existing_execute, None
+            return new_execute, existing_execute, None, eager_exec
         else:
             return self.parent_property.\
                             _get_strategy(LazyLoader).\
     else:
         return LazyLoader
 
-
-
 class EagerJoinOption(PropertyOption):
 
     def __init__(self, key, innerjoin, chained=False):

File test/orm/test_eager_relations.py

 import sqlalchemy as sa
 from test.lib import testing
 from sqlalchemy.orm import joinedload, deferred, undefer, \
-    joinedload_all, backref, eagerload
+    joinedload_all, backref, eagerload, Session, immediateload
 from sqlalchemy import Integer, String, Date, ForeignKey, and_, select, \
     func
 from test.lib.schema import Table, Column
             use_default_dialect=True
         )
 
+
+
 class SubqueryAliasingTest(fixtures.MappedTest, testing.AssertsCompiledSQL):
     """test #2188"""
 
         )
 
 
+class LoadOnExistingTest(_fixtures.FixtureTest):
+    """test that loaders from a base Query fully populate."""
+
+    run_inserts = 'once'
+    run_deletes = None
+
+    def _collection_to_scalar_fixture(self):
+        User, Address, Dingaling = self.classes.User, \
+            self.classes.Address, self.classes.Dingaling
+        mapper(User, self.tables.users, properties={
+            'addresses':relationship(Address),
+        })
+        mapper(Address, self.tables.addresses, properties={
+            'dingaling':relationship(Dingaling)
+        })
+        mapper(Dingaling, self.tables.dingalings)
+
+        sess = Session(autoflush=False)
+        return User, Address, Dingaling, sess
+
+    def _collection_to_collection_fixture(self):
+        User, Order, Item = self.classes.User, \
+            self.classes.Order, self.classes.Item
+        mapper(User, self.tables.users, properties={
+            'orders':relationship(Order), 
+        })
+        mapper(Order, self.tables.orders, properties={
+            'items':relationship(Item, secondary=self.tables.order_items),
+        })
+        mapper(Item, self.tables.items)
+
+        sess = Session(autoflush=False)
+        return User, Order, Item, sess
+
+    def _eager_config_fixture(self):
+        User, Address = self.classes.User, self.classes.Address
+        mapper(User, self.tables.users, properties={
+            'addresses':relationship(Address, lazy="joined"),
+        })
+        mapper(Address, self.tables.addresses)
+        sess = Session(autoflush=False)
+        return User, Address, sess
+
+    def test_no_query_on_refresh(self):
+        User, Address, sess = self._eager_config_fixture()
+
+        u1 = sess.query(User).get(8)
+        assert 'addresses' in u1.__dict__
+        sess.expire(u1)
+        def go():
+            eq_(u1.id, 8)
+        self.assert_sql_count(testing.db, go, 1)
+        assert 'addresses' not in u1.__dict__
+
+    def test_loads_second_level_collection_to_scalar(self):
+        User, Address, Dingaling, sess = self._collection_to_scalar_fixture()
+
+        u1 = sess.query(User).get(8)
+        a1 = Address()
+        u1.addresses.append(a1)
+        a2 = u1.addresses[0]
+        a2.email_address = 'foo'
+        sess.query(User).options(joinedload_all("addresses.dingaling")).\
+                            filter_by(id=8).all()
+        assert u1.addresses[-1] is a1
+        for a in u1.addresses:
+            if a is not a1:
+                assert 'dingaling' in a.__dict__
+            else:
+                assert 'dingaling' not in a.__dict__
+            if a is a2:
+                eq_(a2.email_address, 'foo')
+
+    def test_loads_second_level_collection_to_collection(self):
+        User, Order, Item, sess = self._collection_to_collection_fixture()
+
+        u1 = sess.query(User).get(7)
+        u1.orders
+        o1 = Order()
+        u1.orders.append(o1)
+        sess.query(User).options(joinedload_all("orders.items")).\
+                            filter_by(id=7).all()
+        for o in u1.orders:
+            if o is not o1:
+                assert 'items' in o.__dict__
+            else:
+                assert 'items' not in o.__dict__
+
+    def test_load_two_levels_collection_to_scalar(self):
+        User, Address, Dingaling, sess = self._collection_to_scalar_fixture()
+
+        u1 = sess.query(User).filter_by(id=8).options(joinedload("addresses")).one()
+        sess.query(User).filter_by(id=8).options(joinedload_all("addresses.dingaling")).first()
+        assert 'dingaling' in u1.addresses[0].__dict__
+
+    def test_load_two_levels_collection_to_collection(self):
+        User, Order, Item, sess = self._collection_to_collection_fixture()
+
+        u1 = sess.query(User).filter_by(id=7).options(joinedload("orders")).one()
+        sess.query(User).filter_by(id=7).options(joinedload_all("orders.items")).first()
+        assert 'items' in u1.orders[0].__dict__
 
 
 class AddEntityTest(_fixtures.FixtureTest):

File test/orm/test_subquery_relations.py

 from sqlalchemy import Integer, String, ForeignKey, bindparam
 from sqlalchemy.orm import backref, subqueryload, subqueryload_all, \
     mapper, relationship, clear_mappers, create_session, lazyload, \
-    aliased, joinedload, deferred, undefer, eagerload_all
+    aliased, joinedload, deferred, undefer, eagerload_all,\
+    Session
 from test.lib.testing import eq_, assert_raises, \
     assert_raises_message
 from test.lib.assertsql import CompiledSQL
         assert_raises(sa.exc.SAWarning,
                 s.query(User).options(subqueryload(User.order)).all)
 
+class LoadOnExistingTest(_fixtures.FixtureTest):
+    """test that loaders from a base Query fully populate."""
+
+    run_inserts = 'once'
+    run_deletes = None
+
+    def _collection_to_scalar_fixture(self):
+        User, Address, Dingaling = self.classes.User, \
+            self.classes.Address, self.classes.Dingaling
+        mapper(User, self.tables.users, properties={
+            'addresses':relationship(Address),
+        })
+        mapper(Address, self.tables.addresses, properties={
+            'dingaling':relationship(Dingaling)
+        })
+        mapper(Dingaling, self.tables.dingalings)
+
+        sess = Session(autoflush=False)
+        return User, Address, Dingaling, sess
+
+    def _collection_to_collection_fixture(self):
+        User, Order, Item = self.classes.User, \
+            self.classes.Order, self.classes.Item
+        mapper(User, self.tables.users, properties={
+            'orders':relationship(Order), 
+        })
+        mapper(Order, self.tables.orders, properties={
+            'items':relationship(Item, secondary=self.tables.order_items),
+        })
+        mapper(Item, self.tables.items)
+
+        sess = Session(autoflush=False)
+        return User, Order, Item, sess
+
+    def _eager_config_fixture(self):
+        User, Address = self.classes.User, self.classes.Address
+        mapper(User, self.tables.users, properties={
+            'addresses':relationship(Address, lazy="subquery"),
+        })
+        mapper(Address, self.tables.addresses)
+        sess = Session(autoflush=False)
+        return User, Address, sess
+
+    def _deferred_config_fixture(self):
+        User, Address = self.classes.User, self.classes.Address
+        mapper(User, self.tables.users, properties={
+            'name':deferred(self.tables.users.c.name),
+            'addresses':relationship(Address, lazy="subquery"),
+        })
+        mapper(Address, self.tables.addresses)
+        sess = Session(autoflush=False)
+        return User, Address, sess
+
+    def test_no_query_on_refresh(self):
+        User, Address, sess = self._eager_config_fixture()
+
+        u1 = sess.query(User).get(8)
+        assert 'addresses' in u1.__dict__
+        sess.expire(u1)
+        def go():
+            eq_(u1.id, 8)
+        self.assert_sql_count(testing.db, go, 1)
+        assert 'addresses' not in u1.__dict__
+
+    def test_no_query_on_deferred(self):
+        User, Address, sess = self._deferred_config_fixture()
+        u1 = sess.query(User).get(8)
+        assert 'addresses' in u1.__dict__
+        sess.expire(u1, ['addresses'])
+        def go():
+            eq_(u1.name, 'ed')
+        self.assert_sql_count(testing.db, go, 1)
+        assert 'addresses' not in u1.__dict__
+
+    def test_loads_second_level_collection_to_scalar(self):
+        User, Address, Dingaling, sess = self._collection_to_scalar_fixture()
+
+        u1 = sess.query(User).get(8)
+        a1 = Address()
+        u1.addresses.append(a1)
+        a2 = u1.addresses[0]
+        a2.email_address = 'foo'
+        sess.query(User).options(subqueryload_all("addresses.dingaling")).\
+                            filter_by(id=8).all()
+        assert u1.addresses[-1] is a1
+        for a in u1.addresses:
+            if a is not a1:
+                assert 'dingaling' in a.__dict__
+            else:
+                assert 'dingaling' not in a.__dict__
+            if a is a2:
+                eq_(a2.email_address, 'foo')
+
+    def test_loads_second_level_collection_to_collection(self):
+        User, Order, Item, sess = self._collection_to_collection_fixture()
+
+        u1 = sess.query(User).get(7)
+        u1.orders
+        o1 = Order()
+        u1.orders.append(o1)
+        sess.query(User).options(subqueryload_all("orders.items")).\
+                            filter_by(id=7).all()
+        for o in u1.orders:
+            if o is not o1:
+                assert 'items' in o.__dict__
+            else:
+                assert 'items' not in o.__dict__
+
+    def test_load_two_levels_collection_to_scalar(self):
+        User, Address, Dingaling, sess = self._collection_to_scalar_fixture()
+
+        u1 = sess.query(User).filter_by(id=8).options(subqueryload("addresses")).one()
+        sess.query(User).filter_by(id=8).options(subqueryload_all("addresses.dingaling")).first()
+        assert 'dingaling' in u1.addresses[0].__dict__
+
+    def test_load_two_levels_collection_to_collection(self):
+        User, Order, Item, sess = self._collection_to_collection_fixture()
+
+        u1 = sess.query(User).filter_by(id=7).options(subqueryload("orders")).one()
+        sess.query(User).filter_by(id=7).options(subqueryload_all("orders.items")).first()
+        assert 'items' in u1.orders[0].__dict__
+
 class OrderBySecondaryTest(fixtures.MappedTest):
     @classmethod
     def define_tables(cls, metadata):