- Additional tuning to "many-to-one" relationship

loads during a flush().   A change in version 0.6.6
([ticket:2002]) required that more "unnecessary" m2o
loads during a flush could occur.   Extra loading modes have
been added so that the SQL emitted in this
specific use case is trimmed back, while still
retrieving the information the flush needs in order
to not miss anything.  [ticket:2049]

diff --git a/CHANGES b/CHANGES
index f1897c85cbad5e0230414a28a1ddec0c0c0c3f80..8dfc8fd01cdad2b2fc1136bcad6fb11870834ac6 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -122,6 +122,15 @@ CHANGES
     as *args, interpreted by the Postgresql dialect
     as DISTINCT ON (<expr>). [ticket:1069]
 
+  - Additional tuning to "many-to-one" relationship
+    loads during a flush().   A change in version 0.6.6
+    ([ticket:2002]) required that more "unnecessary" m2o 
+    loads during a flush could occur.   Extra loading modes have
+    been added so that the SQL emitted in this 
+    specific use case is trimmed back, while still
+    retrieving the information the flush needs in order
+    to not miss anything.  [ticket:2049]
+
 - sql
   - Added over() function, method to FunctionElement
     classes, produces the _Over() construct which 

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index ca1137c3f75459a28e8073f9bfb3bcc540d8acc9..c0ec474cb5f4f3c25c471c55df5b0159d17afa81 100644 (file)
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -37,9 +37,19 @@ PASSIVE_NO_INITIALIZE = True #util.symbol('PASSIVE_NO_INITIALIZE')
 
 # this is used by backrefs.
 PASSIVE_NO_FETCH = util.symbol('PASSIVE_NO_FETCH')
-"""Symbol indicating that loader callables should not be fired off.
+"""Symbol indicating that loader callables should not emit SQL.
    Non-initialized attributes should be initialized to an empty value."""
 
+PASSIVE_NO_FETCH_RELATED = util.symbol('PASSIVE_NO_FETCH_RELATED')
+"""Symbol indicating that loader callables should not emit SQL for
+   the related object, but can refresh the attributes of the local
+   instance.
+   Non-initialized attributes should be initialized to an empty value.
+   
+   The unit of work uses this mode to check if history is present
+   with minimal SQL emitted.
+   """
+
 PASSIVE_ONLY_PERSISTENT = util.symbol('PASSIVE_ONLY_PERSISTENT')
 """Symbol indicating that loader callables should only fire off for
 persistent objects.

diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index e3e2f5d567b1e5859ae7b219dfe346336d118b03..dde0d94c8a361996a893f6826b8af8e0b91b1ff1 100644 (file)
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -219,7 +219,12 @@ class DependencyProcessor(object):
         pass
 
     def prop_has_changes(self, uowcommit, states, isdelete):
-        passive = not isdelete or self.passive_deletes
+        if not isdelete or self.passive_deletes:
+            passive = attributes.PASSIVE_NO_INITIALIZE
+        elif self.direction is MANYTOONE:
+            passive = attributes.PASSIVE_NO_FETCH_RELATED
+        else:
+            passive = attributes.PASSIVE_OFF
 
         for s in states:
             # TODO: add a high speed method 

diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py
index 71a6edd31004936919bb2c301a58b433a0add7fc..43699b4d9e7d9af2ca6f0233bae628fc96b3d0bf 100644 (file)
--- a/lib/sqlalchemy/orm/query.py
+++ b/lib/sqlalchemy/orm/query.py
@@ -1947,6 +1947,10 @@ class Query(object):
                 if passive is attributes.PASSIVE_NO_FETCH:
                     # TODO: no coverage here
                     return attributes.PASSIVE_NO_RESULT
+                elif passive is attributes.PASSIVE_NO_FETCH_RELATED:
+                    # this mode is used within a flush and the instance's
+                    # expired state will be checked soon enough, if necessary
+                    return instance
                 try:
                     state(passive)
                 except orm_exc.ObjectDeletedError:

diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index bf7f04995e1aafa82224a7156fadce81dd727d07..8d5649a39a969e66152ce670b53d116c6b6777d2 100644 (file)
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -451,7 +451,8 @@ class LazyLoader(AbstractRelationshipLoader):
         pending = not state.key
 
         if (
-                passive is attributes.PASSIVE_NO_FETCH and 
+                (passive is attributes.PASSIVE_NO_FETCH or \
+                    passive is attributes.PASSIVE_NO_FETCH_RELATED) and 
                 not self.use_get
             ) or (
                 passive is attributes.PASSIVE_ONLY_PERSISTENT and 
@@ -476,12 +477,17 @@ class LazyLoader(AbstractRelationshipLoader):
                 get_attr = instance_mapper._get_state_attr_by_column
 
             dict_ = state.dict
+            if passive is attributes.PASSIVE_NO_FETCH_RELATED:
+                attr_passive = attributes.PASSIVE_OFF
+            else:
+                attr_passive = passive
+
             ident = [
                 get_attr(
                         state,
                         state.dict,
                         self._equated_columns[pk],
-                        passive=passive)
+                        passive=attr_passive)
                 for pk in prop_mapper.primary_key
             ]
             if attributes.PASSIVE_NO_RESULT in ident:
@@ -494,7 +500,8 @@ class LazyLoader(AbstractRelationshipLoader):
             instance = Query._get_from_identity(session, ident_key, passive)
             if instance is not None:
                 return instance
-            elif passive is attributes.PASSIVE_NO_FETCH:
+            elif passive is attributes.PASSIVE_NO_FETCH or \
+                passive is attributes.PASSIVE_NO_FETCH_RELATED:
                 return attributes.PASSIVE_NO_RESULT
 
         q = session.query(prop_mapper)._adapt_all_clauses()

diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index 7018f23381ae8887b0bb740f984ef5df40d0c847..20b643b4de36b9f865dd68d4aeba4c230e9f0f56 100644 (file)
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -259,9 +259,11 @@ class RudimentaryFlushTest(UOWTest):
             testing.db,
             session.flush,
             AllOf(
-                # ensure all three m2os are loaded.
+                # [ticket:2002] - ensure the m2os are loaded.
                 # the selects here are in fact unexpiring
                 # each row - the m2o comes from the identity map.
+                # the User row might be handled before or the addresses
+                # are loaded so need to use AllOf
                 CompiledSQL(
                     "SELECT addresses.id AS addresses_id, addresses.user_id AS "
                     "addresses_user_id, addresses.email_address AS "
@@ -281,14 +283,120 @@ class RudimentaryFlushTest(UOWTest):
                     "FROM users WHERE users.id = :param_1",
                     lambda ctx: {'param_1': pid}
                 ),
+                CompiledSQL(
+                    "DELETE FROM addresses WHERE addresses.id = :id",
+                    lambda ctx: [{'id': c1id}, {'id': c2id}]
+                ),
+                CompiledSQL(
+                    "DELETE FROM users WHERE users.id = :id",
+                    lambda ctx: {'id': pid}
+                ),
+            ),
+        )
+
+    def test_many_to_one_delete_childonly_unloaded(self):
+        mapper(User, users)
+        mapper(Address, addresses, properties={
+            'parent':relationship(User)
+        })
+
+        parent = User(name='p1')
+        c1, c2 = Address(email_address='c1', parent=parent), \
+                    Address(email_address='c2', parent=parent)
+
+        session = Session()
+        session.add_all([c1, c2])
+        session.add(parent)
+
+        session.flush()
+
+        pid = parent.id
+        c1id = c1.id
+        c2id = c2.id
+
+        session.expire(c1)
+        session.expire(c2)
+
+        session.delete(c1)
+        session.delete(c2)
+
+        self.assert_sql_execution(
+            testing.db,
+            session.flush,
+            AllOf(
+                # [ticket:2049] - we aren't deleting User,
+                # relationship is simple m2o, no SELECT should be emitted for it.
+                CompiledSQL(
+                    "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                    "addresses_user_id, addresses.email_address AS "
+                    "addresses_email_address FROM addresses WHERE addresses.id = "
+                    ":param_1",
+                    lambda ctx: {'param_1': c1id}
+                ),
+                CompiledSQL(
+                    "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                    "addresses_user_id, addresses.email_address AS "
+                    "addresses_email_address FROM addresses WHERE addresses.id = "
+                    ":param_1",
+                    lambda ctx: {'param_1': c2id}
+                ),
             ),
             CompiledSQL(
                 "DELETE FROM addresses WHERE addresses.id = :id",
                 lambda ctx: [{'id': c1id}, {'id': c2id}]
             ),
+        )
+
+    def test_many_to_one_delete_childonly_unloaded_expired(self):
+        mapper(User, users)
+        mapper(Address, addresses, properties={
+            'parent':relationship(User)
+        })
+
+        parent = User(name='p1')
+        c1, c2 = Address(email_address='c1', parent=parent), \
+                    Address(email_address='c2', parent=parent)
+
+        session = Session()
+        session.add_all([c1, c2])
+        session.add(parent)
+
+        session.flush()
+
+        pid = parent.id
+        c1id = c1.id
+        c2id = c2.id
+
+        session.expire(parent)
+        session.expire(c1)
+        session.expire(c2)
+
+        session.delete(c1)
+        session.delete(c2)
+
+        self.assert_sql_execution(
+            testing.db,
+            session.flush,
+            AllOf(
+                # the parent User is expired, so it gets loaded here.
+                CompiledSQL(
+                    "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                    "addresses_user_id, addresses.email_address AS "
+                    "addresses_email_address FROM addresses WHERE addresses.id = "
+                    ":param_1",
+                    lambda ctx: {'param_1': c1id}
+                ),
+                CompiledSQL(
+                    "SELECT addresses.id AS addresses_id, addresses.user_id AS "
+                    "addresses_user_id, addresses.email_address AS "
+                    "addresses_email_address FROM addresses WHERE addresses.id = "
+                    ":param_1",
+                    lambda ctx: {'param_1': c2id}
+                ),
+            ),
             CompiledSQL(
-                "DELETE FROM users WHERE users.id = :id",
-                lambda ctx: {'id': pid}
+                "DELETE FROM addresses WHERE addresses.id = :id",
+                lambda ctx: [{'id': c1id}, {'id': c2id}]
             ),
         )
 
@@ -708,14 +816,14 @@ class SingleCycleTest(UOWTest):
                     "WHERE nodes.id = :param_1",
                     lambda ctx: {'param_1': c2id}
                 ),
-            ),
-            CompiledSQL(
-                "DELETE FROM nodes WHERE nodes.id = :id",
-                lambda ctx: [{'id': c1id}, {'id': c2id}]
-            ),
-            CompiledSQL(
-                "DELETE FROM nodes WHERE nodes.id = :id",
-                lambda ctx: {'id': pid}
+                CompiledSQL(
+                    "DELETE FROM nodes WHERE nodes.id = :id",
+                    lambda ctx: [{'id': c1id}, {'id': c2id}]
+                ),
+                CompiledSQL(
+                    "DELETE FROM nodes WHERE nodes.id = :id",
+                    lambda ctx: {'id': pid}
+                ),
             ),
         )