- lazy loads for relationship attributes now use

the current state, not the "committed" state,
of foreign and primary key attributes
when issuing SQL, if a flush is not in process.
Previously, only the database-committed state would
be used.  In particular, this would cause a many-to-one
get()-on-lazyload operation to fail, as autoflush
is not triggered on these loads when the attributes are
determined and the "committed" state may not be
available.  [ticket:1910]

- A new flag on relationship(), load_on_pending, allows
the lazy loader to fire off on pending objects without a
flush taking place, as well as a transient object that's
been manually "attached" to the session. Note that this
flag blocks attribute events from taking place when an
object is loaded, so backrefs aren't available until
after a flush. The flag is only intended for very
specific use cases.

9 files changed, 130 insertions, 59 deletions

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index 879a7b0c1..11912e80b 100644
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -355,6 +355,23 @@ def relationship(argument, secondary=None, **kwargs):
       * None - a synonym for 'noload'
        
       Detailed discussion of loader strategies is at :ref:`loading_toplevel`.
+    
+    :param load_on_pending:
+      Indicates loading behavior for transient or pending parent objects.
+      
+      This is an advanced user feature that will cause the lazy-loader to
+      issue a query for a parent object that is not persistent, meaning it has
+      never been flushed. This may take effect for a pending object when
+      autoflush is disabled, or for a transient object that has been
+      "attached" to a :class:`.Session` but is not part of its pending
+      collection. Attachment of transient objects to the session without
+      moving to the "pending" state is not a supported behavior at this time.
+      
+      Note that the load of related objects on a pending or transient object
+      also does not trigger any attribute change events - no user-defined
+      events will be emitted for these attributes, and if and when the 
+      object is ultimately flushed, only the user-specific foreign key 
+      attributes will be part of the modified state.
       
     :param order_by:
       indicates the ordering that should be applied when loading these
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index bccdaeb4b..33069332d 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -42,9 +42,16 @@ 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 boe fired off.
+"""Symbol indicating that loader callables should not be fired off.
    Non-initialized attributes should be initialized to an empty value."""
 
+PASSIVE_ONLY_PERSISTENT = util.symbol('PASSIVE_ONLY_PERSISTENT')
+"""Symbol indicating that loader callables should only fire off for
+persistent objects.
+
+Loads of "previous" values during change events use this flag.
+"""
+
 PASSIVE_OFF = False #util.symbol('PASSIVE_OFF')
 """Symbol indicating that loader callables should be executed."""
 
@@ -593,10 +600,10 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
             return
 
         if self.active_history:
-            old = self.get(state, dict_)
+            old = self.get(state, dict_, passive=PASSIVE_ONLY_PERSISTENT)
         else:
             old = self.get(state, dict_, passive=PASSIVE_NO_FETCH)
-             
+        
         value = self.fire_replace_event(state, dict_, value, old, initiator)
         dict_[self.key] = value
 
@@ -777,15 +784,16 @@ class CollectionAttributeImpl(AttributeImpl):
         else:
             new_values = list(iterable)
 
-        old = self.get(state, dict_)
-
-        # ignore re-assignment of the current collection, as happens
-        # implicitly with in-place operators (foo.collection |= other)
-        if old is iterable:
+        old = self.get(state, dict_, passive=PASSIVE_ONLY_PERSISTENT)
+        if old is PASSIVE_NO_RESULT:
+            old = self.initialize(state, dict_)
+        elif old is iterable:
+            # ignore re-assignment of the current collection, as happens
+            # implicitly with in-place operators (foo.collection |= other)
             return
 
         state.modified_event(dict_, self, True, old)
-
+        
         old_collection = self.get_collection(state, dict_, old)
 
         dict_[self.key] = user_data
@@ -855,7 +863,7 @@ class GenericBackrefExtension(interfaces.AttributeExtension):
     def set(self, state, child, oldchild, initiator):
         if oldchild is child:
             return child
-            
+
         if oldchild is not None and oldchild is not PASSIVE_NO_RESULT:
             # With lazy=None, there's no guarantee that the full collection is
             # present when updating via a backref.
diff --git a/lib/sqlalchemy/orm/collections.py b/lib/sqlalchemy/orm/collections.py
index b80be970a..0789d9626 100644
--- a/lib/sqlalchemy/orm/collections.py
+++ b/lib/sqlalchemy/orm/collections.py
@@ -546,6 +546,7 @@ class CollectionAdapter(object):
 
     def append_with_event(self, item, initiator=None):
         """Add an entity to the collection, firing mutation events."""
+        
         getattr(self._data(), '_sa_appender')(item, _sa_initiator=initiator)
 
     def append_without_event(self, item):
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index b87bdc890..9e38ac811 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1295,8 +1295,8 @@ class Mapper(object):
                 column in self.primary_key]
 
     # TODO: improve names?
-    def _get_state_attr_by_column(self, state, dict_, column):
-        return self._columntoproperty[column]._getattr(state, dict_, column)
+    def _get_state_attr_by_column(self, state, dict_, column, passive=False):
+        return self._columntoproperty[column]._getattr(state, dict_, column, passive=passive)
 
     def _set_state_attr_by_column(self, state, dict_, column, value):
         return self._columntoproperty[column]._setattr(state, dict_, value, column)
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index 6098339d2..80443a7f3 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -116,8 +116,8 @@ class ColumnProperty(StrategizedProperty):
                         group=self.group, 
                         *self.columns)
 
-    def _getattr(self, state, dict_, column):
-        return state.get_impl(self.key).get(state, dict_)
+    def _getattr(self, state, dict_, column, passive=False):
+        return state.get_impl(self.key).get(state, dict_, passive=passive)
 
     def _getcommitted(self, state, dict_, column, passive=False):
         return state.get_impl(self.key).\
@@ -191,8 +191,8 @@ class CompositeProperty(ColumnProperty):
         # which issues assertions that do not apply to CompositeColumnProperty
         super(ColumnProperty, self).do_init()
 
-    def _getattr(self, state, dict_, column):
-        obj = state.get_impl(self.key).get(state, dict_)
+    def _getattr(self, state, dict_, column, passive=False):
+        obj = state.get_impl(self.key).get(state, dict_, passive=passive)
         return self.get_col_value(column, obj)
 
     def _getcommitted(self, state, dict_, column, passive=False):
@@ -444,6 +444,7 @@ class RelationshipProperty(StrategizedProperty):
         comparator_factory=None,
         single_parent=False, innerjoin=False,
         doc=None,
+        load_on_pending=False,
         strategy_class=None, _local_remote_pairs=None, query_class=None):
 
         self.uselist = uselist
@@ -468,6 +469,7 @@ class RelationshipProperty(StrategizedProperty):
         self.join_depth = join_depth
         self.local_remote_pairs = _local_remote_pairs
         self.extension = extension
+        self.load_on_pending = load_on_pending
         self.comparator_factory = comparator_factory or \
                                     RelationshipProperty.Comparator
         self.comparator = self.comparator_factory(self, None)
@@ -722,8 +724,7 @@ class RelationshipProperty(StrategizedProperty):
 
     def compare(self, op, value, 
                             value_is_parent=False, 
-                            alias_secondary=True,
-                            detect_transient_pending=False):
+                            alias_secondary=True):
         if op == operators.eq:
             if value is None:
                 if self.uselist:
@@ -731,26 +732,22 @@ class RelationshipProperty(StrategizedProperty):
                 else:
                     return self._optimized_compare(None, 
                                     value_is_parent=value_is_parent,
-                                    detect_transient_pending=detect_transient_pending,
                                     alias_secondary=alias_secondary)
             else:
                 return self._optimized_compare(value, 
                                 value_is_parent=value_is_parent,
-                                detect_transient_pending=detect_transient_pending,
                                 alias_secondary=alias_secondary)
         else:
             return op(self.comparator, value)
 
     def _optimized_compare(self, value, value_is_parent=False, 
                                     adapt_source=None, 
-                                    detect_transient_pending=False,
                                     alias_secondary=True):
         if value is not None:
             value = attributes.instance_state(value)
         return self._get_strategy(strategies.LazyLoader).lazy_clause(value,
                 reverse_direction=not value_is_parent,
                 alias_secondary=alias_secondary,
-                detect_transient_pending=detect_transient_pending,
                 adapt_source=adapt_source)
 
     def __str__(self):
diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index f6828f5a9..e6502df8c 100644
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -333,7 +333,7 @@ class InstanceState(object):
                         previous = dict_[attr.key]
                 else:
                     previous = attr.get(self, dict_)
-
+                
             if should_copy and previous not in (None, NO_VALUE, NEVER_SET):
                 previous = attr.copy(previous)
 
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 1b8cf0852..b0a18b7dd 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -256,8 +256,8 @@ class LoadDeferredColumns(object):
     def __init__(self, state, key):
         self.state, self.key = state, key
 
-    def __call__(self, **kw):
-        if kw.get('passive') is attributes.PASSIVE_NO_FETCH:
+    def __call__(self, passive=False):
+        if passive is attributes.PASSIVE_NO_FETCH:
             return attributes.PASSIVE_NO_RESULT
 
         state = self.state
@@ -405,8 +405,7 @@ class LazyLoader(AbstractRelationshipLoader):
 
     def lazy_clause(self, state, reverse_direction=False, 
                                 alias_secondary=False, 
-                                adapt_source=None,
-                                detect_transient_pending=False):
+                                adapt_source=None):
         if state is None:
             return self._lazy_none_clause(
                                         reverse_direction, 
@@ -431,27 +430,23 @@ class LazyLoader(AbstractRelationshipLoader):
         o = state.obj() # strong ref
         dict_ = attributes.instance_dict(o)
         
-        def visit_bindparam(bindparam):
-            if bindparam.key in bind_to_col:
-                # using a flag to enable "detect transient pending" so that
-                # the slightly different usage paradigm of "dynamic" loaders
-                # continue to work as expected, i.e. that all pending objects
-                # should use the "post flush" attributes, and to limit this 
-                # newer behavior to the query.with_parent() method.  
-                # It would be nice to do away with this flag.
-                
-                if detect_transient_pending and \
-                    (not state.key or not state.session_id):
-                    bindparam.value = mapper._get_state_attr_by_column(
-                                        state, dict_, bind_to_col[bindparam.key])
-                else:
-                    # send value as a lambda so that the value is
-                    # acquired after any autoflush occurs.
+        # use the "committed state" only if we're in a flush
+        # for this state.
+        
+        sess = sessionlib._state_session(state)
+        if sess is not None and sess._flushing:
+            def visit_bindparam(bindparam):
+                if bindparam.key in bind_to_col:
                     bindparam.value = \
                                 lambda: mapper._get_committed_state_attr_by_column(
                                         state, dict_, bind_to_col[bindparam.key])
-                    
-
+        else:
+            def visit_bindparam(bindparam):
+                if bindparam.key in bind_to_col:
+                    bindparam.value = lambda: mapper._get_state_attr_by_column(
+                                            state, dict_, bind_to_col[bindparam.key])
+        
+            
         if self.parent_property.secondary is not None and alias_secondary:
             criterion = sql_util.ClauseAdapter(
                                 self.parent_property.secondary.alias()).\
@@ -459,6 +454,7 @@ class LazyLoader(AbstractRelationshipLoader):
 
         criterion = visitors.cloned_traverse(
                                 criterion, {}, {'bindparam':visit_bindparam})
+
         if adapt_source:
             criterion = adapt_source(criterion)
         return criterion
@@ -482,7 +478,8 @@ class LazyLoader(AbstractRelationshipLoader):
         return criterion
         
     def _class_level_loader(self, state):
-        if not state.has_identity:
+        if not state.has_identity and \
+            (not self.parent_property.load_on_pending or not state.session_id):
             return None
 
         return LoadLazyAttribute(state, self.key)
@@ -572,16 +569,22 @@ class LoadLazyAttribute(object):
     def __setstate__(self, state):
         self.state, self.key = state
         
-    def __call__(self, **kw):
+    def __call__(self, passive=False):
         state = self.state
         instance_mapper = mapper._state_mapper(state)
         prop = instance_mapper.get_property(self.key)
         strategy = prop._get_strategy(LazyLoader)
-
-        if kw.get('passive') is attributes.PASSIVE_NO_FETCH and \
-                                    not strategy.use_get:
+        pending = not state.key
+        
+        if (
+                passive is attributes.PASSIVE_NO_FETCH and 
+                not strategy.use_get
+            ) or (
+                passive is attributes.PASSIVE_ONLY_PERSISTENT and 
+                pending
+            ):
             return attributes.PASSIVE_NO_RESULT
-
+            
         if strategy._should_log_debug():
             strategy.logger.debug("loading %s", 
                                     mapperutil.state_attribute_str(
@@ -597,21 +600,35 @@ class LoadLazyAttribute(object):
         
         q = session.query(prop.mapper)._adapt_all_clauses()
         
+        # don't autoflush on pending
+        # this would be something that's prominent in the 
+        # docs and such
+        if pending:
+            q = q.autoflush(False)
+            
         if state.load_path:
             q = q._with_current_path(state.load_path + (self.key,))
-            
+
         # if we have a simple primary key load, use mapper.get()
         # to possibly save a DB round trip
         if strategy.use_get:
             ident = []
             allnulls = True
+            if session._flushing:
+                get_attr = instance_mapper._get_committed_state_attr_by_column
+            else:
+                get_attr = instance_mapper._get_state_attr_by_column
+
+            # The many-to-one get is intended to be very fast.  Note
+            # that we don't want to autoflush() if the get() doesn't
+            # actually have to hit the DB.  It is now not necessary
+            # now that we use the pending attribute state.
             for primary_key in prop.mapper.primary_key: 
-                val = instance_mapper.\
-                                _get_committed_state_attr_by_column(
+                val = get_attr(
                                     state,
                                     state.dict,
                                     strategy._equated_columns[primary_key],
-                                    **kw)
+                                    passive=passive)
                 if val is attributes.PASSIVE_NO_RESULT:
                     return val
                 allnulls = allnulls and val is None
@@ -624,7 +641,7 @@ class LoadLazyAttribute(object):
                 q = q._conditional_options(*state.load_options)
 
             key = prop.mapper.identity_key_from_primary_key(ident)
-            return q._get(key, ident, **kw)
+            return q._get(key, ident, passive=passive)
             
 
         if prop.order_by:
@@ -640,8 +657,15 @@ class LoadLazyAttribute(object):
 
         if state.load_options:
             q = q._conditional_options(*state.load_options)
+        
+        lazy_clause = strategy.lazy_clause(state)
+        
+        if pending:
+            bind_values = sql_util.bind_values(lazy_clause)
+            if None in bind_values:
+                return None
             
-        q = q.filter(strategy.lazy_clause(state))
+        q = q.filter(lazy_clause)
 
         result = q.all()
         if strategy.uselist:
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index a5ddf2f6e..d68ff4473 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -515,8 +515,7 @@ def with_parent(instance, prop):
 
     return prop.compare(operators.eq, 
                         instance, 
-                        value_is_parent=True, 
-                        detect_transient_pending=True)
+                        value_is_parent=True)
 
 
 def _entity_info(entity, compile=True):
diff --git a/lib/sqlalchemy/sql/util.py b/lib/sqlalchemy/sql/util.py
index c999ab786..bd4f70247 100644
--- a/lib/sqlalchemy/sql/util.py
+++ b/lib/sqlalchemy/sql/util.py
@@ -92,6 +92,31 @@ def find_columns(clause):
     visitors.traverse(clause, {}, {'column':cols.add})
     return cols
 
+def bind_values(clause):
+    """Return an ordered list of "bound" values in the given clause.
+
+    E.g.::
+    
+        >>> expr = and_(
+        ...    table.c.foo==5, table.c.foo==7
+        ... )
+        >>> bind_values(expr)
+        [5, 7]
+    """
+    
+    v = []
+    def visit_bindparam(bind):
+        value = bind.value
+        
+        # evaluate callables
+        if callable(value):
+            value = value()
+            
+        v.append(value)
+        
+    visitors.traverse(clause, {}, {'bindparam':visit_bindparam})
+    return v
+
 def _quote_ddl_expr(element):
     if isinstance(element, basestring):
         element = element.replace("'", "''")