Merge "Modernize deferred callable for many-to-one comparison"

2 files changed, 115 insertions, 14 deletions

diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index 818f1c0ae..e92d10a5b 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -25,6 +25,8 @@ from ..sql.util import (
     join_condition, _shallow_annotate, visit_binary_product,
     _deep_deannotate, selectables_overlap, adapt_criterion_to_null
 )
+from .base import state_str
+
 from ..sql import operators, expression, visitors
 from .interfaces import (MANYTOMANY, MANYTOONE, ONETOMANY,
                          StrategizedProperty, PropComparator)
@@ -1274,9 +1276,7 @@ class RelationshipProperty(StrategizedProperty):
                     return sql.bindparam(
                         x, unique=True,
                         callable_=self.property._get_attr_w_warn_on_none(
-                            col,
-                            self.property.mapper._get_state_attr_by_column,
-                            state, dict_, col, passive=attributes.PASSIVE_OFF
+                            self.property.mapper, state, dict_, col
                         )
                     )
 
@@ -1406,11 +1406,8 @@ class RelationshipProperty(StrategizedProperty):
         def visit_bindparam(bindparam):
             if bindparam._identifying_key in bind_to_col:
                 bindparam.callable = self._get_attr_w_warn_on_none(
-                    bind_to_col[bindparam._identifying_key],
-                    mapper._get_state_attr_by_column,
-                    state, dict_,
-                    bind_to_col[bindparam._identifying_key],
-                    passive=attributes.PASSIVE_OFF)
+                    mapper, state, dict_,
+                    bind_to_col[bindparam._identifying_key])
 
         if self.secondary is not None and alias_secondary:
             criterion = ClauseAdapter(
@@ -1424,16 +1421,94 @@ class RelationshipProperty(StrategizedProperty):
             criterion = adapt_source(criterion)
         return criterion
 
-    def _get_attr_w_warn_on_none(self, column, fn, *arg, **kw):
+    def _get_attr_w_warn_on_none(self, mapper, state, dict_, column):
+        """Create the callable that is used in a many-to-one expression.
+
+        E.g.::
+
+            u1 = s.query(User).get(5)
+
+            expr = Address.user == u1
+
+        Above, the SQL should be "address.user_id = 5". The callable
+        returned by this method produces the value "5" based on the identity
+        of ``u1`.
+
+        """
+
+        # in this callable, we're trying to thread the needle through
+        # a wide variety of scenarios, including:
+        #
+        # * the object hasn't been flushed yet and there's no value for
+        #   the attribute as of yet
+        #
+        # * the object hasn't been flushed yet but it has a user-defined
+        #   value
+        #
+        # * the object has a value but it's expired and not locally present
+        #
+        # * the object has a value but it's expired and not locally present,
+        #   and the object is also detached
+        #
+        # * The object hadn't been flushed yet, there was no value, but
+        #   later, the object has been expired and detached, and *now*
+        #   they're trying to evaluate it
+        #
+        # * the object had a value, but it was changed to a new value, and
+        #   then expired
+        #
+        # * the object had a value, but it was changed to a new value, and
+        #   then expired, then the object was detached
+        #
+        # * the object has a user-set value, but it's None and we don't do
+        #   the comparison correctly for that so warn
+        #
+
+        prop = mapper.get_property_by_column(column)
+
+        # by invoking this method, InstanceState will track the last known
+        # value for this key each time the attribute is to be expired.
+        # this feature was added explicitly for use in this method.
+        state._track_last_known_value(prop.key)
+
         def _go():
-            value = fn(*arg, **kw)
-            if value is None:
+            last_known = to_return = state._last_known_values[prop.key]
+            existing_is_available = last_known is not attributes.NO_VALUE
+
+            # we support that the value may have changed.  so here we
+            # try to get the most recent value including re-fetching.
+            # only if we can't get a value now due to detachment do we return
+            # the last known value
+            current_value = mapper._get_state_attr_by_column(
+                state, dict_, column,
+                passive=attributes.PASSIVE_RETURN_NEVER_SET
+                if state.persistent
+                else attributes.PASSIVE_NO_FETCH ^ attributes.INIT_OK)
+
+            if current_value is attributes.NEVER_SET:
+                if not existing_is_available:
+                    raise sa_exc.InvalidRequestError(
+                        "Can't resolve value for column %s on object "
+                        "%s; no value has been set for this column" % (
+                            column, state_str(state))
+                    )
+            elif current_value is attributes.PASSIVE_NO_RESULT:
+                if not existing_is_available:
+                    raise sa_exc.InvalidRequestError(
+                        "Can't resolve value for column %s on object "
+                        "%s; the object is detached and the value was "
+                        "expired" % (
+                            column, state_str(state))
+                    )
+            else:
+                to_return = current_value
+            if to_return is None:
                 util.warn(
                     "Got None for value of column %s; this is unsupported "
                     "for a relationship comparison and will not "
                     "currently produce an IS comparison "
                     "(but may in a future release)" % column)
-            return value
+            return to_return
         return _go
 
     def _lazy_none_clause(self, reverse_direction=False, adapt_source=None):
diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index 935e7df19..944dc8177 100644
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -64,6 +64,7 @@ class InstanceState(interfaces.InspectionAttrInfo):
     _orphaned_outside_of_session = False
     is_instance = True
     identity_token = None
+    _last_known_values = ()
 
     callables = ()
     """A namespace where a per-state loader callable can be associated.
@@ -229,6 +230,18 @@ class InstanceState(interfaces.InspectionAttrInfo):
         return self.session_id is not None and \
             self.session_id in sessionlib._sessions
 
+    def _track_last_known_value(self, key):
+        """Track the last known value of a particular key after expiration
+        operations.
+
+        .. versionadded:: 1.3
+
+        """
+
+        if key not in self._last_known_values:
+            self._last_known_values = dict(self._last_known_values)
+            self._last_known_values[key] = NO_VALUE
+
     @property
     @util.dependencies("sqlalchemy.orm.session")
     def session(self, sessionlib):
@@ -569,6 +582,12 @@ class InstanceState(interfaces.InspectionAttrInfo):
             collection = dict_.pop(k)
             collection._sa_adapter.invalidated = True
 
+        if self._last_known_values:
+            self._last_known_values.update(
+                (k, dict_[k]) for k in self._last_known_values
+                if k in dict_
+            )
+
         for key in self.manager._all_key_set.intersection(dict_):
             del dict_[key]
 
@@ -591,10 +610,14 @@ class InstanceState(interfaces.InspectionAttrInfo):
                 self.expired_attributes.add(key)
                 if callables and key in callables:
                     del callables[key]
-            old = dict_.pop(key, None)
-            if impl.collection and old is not None:
+            old = dict_.pop(key, NO_VALUE)
+            if impl.collection and old is not NO_VALUE:
                 impl._invalidate_collection(old)
 
+            if self._last_known_values and key in self._last_known_values \
+                    and old is not NO_VALUE:
+                self._last_known_values[key] = old
+
             self.committed_state.pop(key, None)
             if pending:
                 pending.pop(key, None)
@@ -690,6 +713,9 @@ class InstanceState(interfaces.InspectionAttrInfo):
                         previous = attr.copy(previous)
                 self.committed_state[attr.key] = previous
 
+            if attr.key in self._last_known_values:
+                self._last_known_values[attr.key] = NO_VALUE
+
         # assert self._strong_obj is None or self.modified
 
         if (self.session_id and self._strong_obj is None) \