1. Michael Bayer
  2. sqlalchemy
  3. Issues

Issues

Issue #1763 open

run eager loaders on unexpire

Michael Bayer
repo owner created an issue

this patch begins to provide this behavior. However a lazyloader from an expired object emits a second unnecessary load (or an unnecessary join, depending on how you look at it), and polymorphic loading breaks entirely, probably due to an incompatibility with the partial joined inheritance loader.

diff -r cfdc41c75ce2e2228d71733212bbaca0fe17ce35 lib/sqlalchemy/orm/state.py
--- a/lib/sqlalchemy/orm/state.py   Mon Apr 05 13:16:29 2010 -0400
+++ b/lib/sqlalchemy/orm/state.py   Mon Apr 05 15:43:40 2010 -0400
@@ -262,9 +262,10 @@
         if kw.get('passive') is attributes.PASSIVE_NO_FETCH:
             return attributes.PASSIVE_NO_RESULT

-        toload = self.expired_attributes.\
-                        intersection(self.unmodified)
-        
+        toload = self.expired_attributes.intersection(self.unmodified).union(
+            self.unloaded.intersection(self.expire_on_missing)
+        )
+
         self.manager.deferred_scalar_loader(self, toload)

         # if the loader failed, or this 
@@ -277,6 +278,10 @@
         return ATTR_WAS_SET

     @property
+    def expire_on_missing(self):
+        return set(key for key in self.manager if self.manager[key](key).impl.expire_missing)
+        
+    @property
     def unmodified(self):
         """Return the set of keys which have no uncommitted changes"""

diff -r cfdc41c75ce2e2228d71733212bbaca0fe17ce35 test/orm/test_expire.py
--- a/test/orm/test_expire.py   Mon Apr 05 13:16:29 2010 -0400
+++ b/test/orm/test_expire.py   Mon Apr 05 15:43:40 2010 -0400
@@ -425,17 +425,7 @@
         assert len(u.addresses) == 2

     @testing.resolve_artifact_names
-    def test_joinedload_props_dontload(self):
-        # relationships currently have to load separately from scalar instances.
-        # the use case is: expire "addresses".  then access it.  lazy load
-        # fires off to load "addresses", but needs foreign key or primary key
-        # attributes in order to lazy load; hits those attributes, such as
-        # below it hits "u.id".  "u.id" triggers full unexpire operation,
-        # joinedloads addresses since lazy='joined'.  this is all wihtin lazy load
-        # which fires unconditionally; so an unnecessary joinedload (or
-        # lazyload) was issued.  would prefer not to complicate lazyloading to
-        # "figure out" that the operation should be aborted right now.
-
+    def test_joinedload_props_load(self):
         mapper(User, users, properties={
             'addresses':relationship(Address, backref='user', lazy='joined'),
             })
@@ -444,11 +434,30 @@
         u = sess.query(User).get(8)
         sess.expire(u)
         u.id
-        assert 'addresses' not in u.__dict__
+        assert 'addresses' in u.__dict__
         u.addresses
         assert 'addresses' in u.__dict__

     @testing.resolve_artifact_names
+    def test_joinedload_props_load_two(self):
+        mapper(User, users, properties={
+            'addresses':relationship(Address, backref='user', lazy='joined'),
+            })
+        mapper(Address, addresses)
+        sess = create_session()
+        u = sess.query(User).get(8)
+        sess.expire(u)
+
+        # current bug: u.addresses unexpires attributes,
+        # eagerlaods u.addresses.   lazyloader for u.addresses
+        # then runs anyway.
+        def go():
+            u.addresses
+            assert 'addresses' in u.__dict__
+            assert 'id' in u.__dict__
+        self.assert_sql_count(testing.db, go, 1)
+        
+    @testing.resolve_artifact_names
     def test_expire_synonym(self):
         mapper(User, users, properties={
             'uname': sa.orm.synonym('name')

Comments (6)

  1. Michael Bayer reporter

    excess complexity for the "unexpire" use case, not clear that this case benefits from supplied eager loaders firing off and this would add to the maintenance burden.

  2. Michael Bayer reporter
    • changed milestone to 1.2
    • edited description

    Let's look at what might be involved for 1.2, including that an in-place unexpire as well as get() would work here. For refresh(), that might be diffferent if I recall correctly. Re-target out to 1.3 if it's not a quick fix at this point.

  3. Log in to comment