- Fixed more regressions caused by NEVER_SET; comparisons

to transient objects with attributes unset would leak NEVER_SET,
and negated_contains_or_equals would do so for any transient
object as the comparison used only the committed value.
Repaired the NEVER_SET cases, fixes #3371, and also made
negated_contains_or_equals() use state_attr_by_column() just
like a non-negated comparison, fixes #3374

6 files changed, 48 insertions, 25 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 41803c8bf..a45c22394 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -619,7 +619,7 @@ class AttributeImpl(object):
 
         if self.key in state.committed_state:
             value = state.committed_state[self.key]
-            if value is NO_VALUE:
+            if value in (NO_VALUE, NEVER_SET):
                 return None
             else:
                 return value
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 4554f78f9..924130874 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -2377,15 +2377,15 @@ class Mapper(InspectionAttr):
 
         """
         state = attributes.instance_state(instance)
-        return self._primary_key_from_state(state)
+        return self._primary_key_from_state(state, attributes.PASSIVE_OFF)
 
-    def _primary_key_from_state(self, state):
+    def _primary_key_from_state(
+            self, state, passive=attributes.PASSIVE_RETURN_NEVER_SET):
         dict_ = state.dict
         manager = state.manager
         return [
             manager[prop.key].
-            impl.get(state, dict_,
-                     attributes.PASSIVE_RETURN_NEVER_SET)
+            impl.get(state, dict_, passive)
             for prop in self._identity_key_props
         ]
 
@@ -2428,7 +2428,8 @@ class Mapper(InspectionAttr):
     def _get_committed_attr_by_column(self, obj, column):
         state = attributes.instance_state(obj)
         dict_ = attributes.instance_dict(obj)
-        return self._get_committed_state_attr_by_column(state, dict_, column)
+        return self._get_committed_state_attr_by_column(
+            state, dict_, column, passive=attributes.PASSIVE_OFF)
 
     def _get_committed_state_attr_by_column(
             self, state, dict_, column,
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index c2527ee3d..34c37dab6 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -531,7 +531,7 @@ def _collect_post_update_commands(base_mapper, uowtransaction, table,
                 params[col._label] = \
                     mapper._get_state_attr_by_column(
                         state,
-                        state_dict, col)
+                        state_dict, col, passive=attributes.PASSIVE_OFF)
 
             elif col in post_update_cols:
                 prop = mapper._columntoproperty[col]
diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py
index b649c9e21..d1c5e5e51 100644
--- a/lib/sqlalchemy/orm/relationships.py
+++ b/lib/sqlalchemy/orm/relationships.py
@@ -1233,11 +1233,15 @@ class RelationshipProperty(StrategizedProperty):
                 state = attributes.instance_state(other)
 
                 def state_bindparam(x, state, col):
-                    o = state.obj()  # strong ref
+                    dict_ = state.dict
                     return sql.bindparam(
-                        x, unique=True, callable_=lambda:
-                        self.property.mapper.
-                        _get_committed_attr_by_column(o, col))
+                        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
+                        )
+                    )
 
                 def adapt(col):
                     if self.adapter:
@@ -1252,13 +1256,14 @@ class RelationshipProperty(StrategizedProperty):
                             adapt(x) == None)
                         for (x, y) in self.property.local_remote_pairs])
 
-            criterion = sql.and_(*[x == y for (x, y) in
-                                   zip(
-                self.property.mapper.primary_key,
-                self.property.
-                mapper.
-                primary_key_from_instance(other))
+            criterion = sql.and_(*[
+                x == y for (x, y) in
+                zip(
+                    self.property.mapper.primary_key,
+                    self.property.mapper.primary_key_from_instance(other)
+                )
             ])
+
             return ~self._criterion_exists(criterion)
 
         def __ne__(self, other):
@@ -1357,10 +1362,12 @@ class RelationshipProperty(StrategizedProperty):
 
         def visit_bindparam(bindparam):
             if bindparam._identifying_key in bind_to_col:
-                bindparam.callable = \
-                    lambda: mapper._get_state_attr_by_column(
-                        state, dict_,
-                        bind_to_col[bindparam._identifying_key])
+                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)
 
         if self.secondary is not None and alias_secondary:
             criterion = ClauseAdapter(
@@ -1374,6 +1381,18 @@ class RelationshipProperty(StrategizedProperty):
             criterion = adapt_source(criterion)
         return criterion
 
+    def _get_attr_w_warn_on_none(self, column, fn, *arg, **kw):
+        def _go():
+            value = fn(*arg, **kw)
+            if value 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 _go
+
     def _lazy_none_clause(self, reverse_direction=False, adapt_source=None):
         if not reverse_direction:
             criterion, bind_to_col = \
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index 5c6618686..85d233a05 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -456,15 +456,18 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots):
         o = state.obj()  # strong ref
         dict_ = attributes.instance_dict(o)
 
+        if passive & attributes.INIT_OK:
+            passive ^= attributes.INIT_OK
+
         params = {}
         for key, ident, value in param_keys:
             if ident is not None:
                 if passive and passive & attributes.LOAD_AGAINST_COMMITTED:
                     value = mapper._get_committed_state_attr_by_column(
-                        state, dict_, ident)
+                        state, dict_, ident, passive)
                 else:
                     value = mapper._get_state_attr_by_column(
-                        state, dict_, ident)
+                        state, dict_, ident, passive)
 
             params[key] = value
 
diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py
index e9a745cc0..e8e273a86 100644
--- a/lib/sqlalchemy/orm/sync.py
+++ b/lib/sqlalchemy/orm/sync.py
@@ -85,7 +85,7 @@ def update(source, source_mapper, dest, old_prefix, synchronize_pairs):
             oldvalue = source_mapper._get_committed_attr_by_column(
                 source.obj(), l)
             value = source_mapper._get_state_attr_by_column(
-                source, source.dict, l)
+                source, source.dict, l, passive=attributes.PASSIVE_OFF)
         except exc.UnmappedColumnError:
             _raise_col_to_prop(False, source_mapper, l, None, r)
         dest[r.key] = value
@@ -96,7 +96,7 @@ def populate_dict(source, source_mapper, dict_, synchronize_pairs):
     for l, r in synchronize_pairs:
         try:
             value = source_mapper._get_state_attr_by_column(
-                source, source.dict, l)
+                source, source.dict, l, passive=attributes.PASSIVE_OFF)
         except exc.UnmappedColumnError:
             _raise_col_to_prop(False, source_mapper, l, None, r)