- Adjustment to attribute mechanics concerning when a value is

implicitly initialized to None via first access; this action,
which has always resulted in a population of the attribute,
now emits an attribute event just like any other attribute set
operation and generates the same kind of history as one.  Additionally,
many mapper internal operations will no longer implicitly generate
these "None" values when various never-set attributes are checked.
These are subtle behavioral fixes to attribute mechanics which provide
a better solution to the problem of :ticket:`3060`, which also
involves recognition of attributes explicitly set to ``None``
vs. attributes that were never set.
fixes #3061

11 files changed, 61 insertions, 64 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 09d6e988d..306c86e3b 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -23,7 +23,8 @@ from .base import PASSIVE_NO_RESULT, ATTR_WAS_SET, ATTR_EMPTY, NO_VALUE,\
             NEVER_SET, NO_CHANGE, CALLABLES_OK, SQL_OK, RELATED_OBJECT_OK,\
             INIT_OK, NON_PERSISTENT_OK, LOAD_AGAINST_COMMITTED, PASSIVE_OFF,\
             PASSIVE_RETURN_NEVER_SET, PASSIVE_NO_INITIALIZE, PASSIVE_NO_FETCH,\
-            PASSIVE_NO_FETCH_RELATED, PASSIVE_ONLY_PERSISTENT, NO_AUTOFLUSH
+            PASSIVE_NO_FETCH_RELATED, PASSIVE_ONLY_PERSISTENT, NO_AUTOFLUSH,\
+            _none_tuple
 from .base import state_str, instance_str
 
 @inspection._self_inspects
@@ -355,7 +356,10 @@ class Event(object):
         self.op = op
         self.parent_token = self.impl.parent_token
 
-
+    def __eq__(self, other):
+        return isinstance(other, Event) and \
+            other.impl is self.impl and \
+            other.op == self.op
     @property
     def key(self):
         return self.impl.key
@@ -553,8 +557,15 @@ class AttributeImpl(object):
     def initialize(self, state, dict_):
         """Initialize the given state's attribute with an empty value."""
 
-        dict_[self.key] = None
-        return None
+        old = NEVER_SET
+        value = None
+        if self.dispatch.set:
+            value = self.fire_replace_event(state, dict_,
+                                                None, old, None)
+        state._modified_event(dict_, self, old)
+
+        dict_[self.key] = value
+        return value
 
     def get(self, state, dict_, passive=PASSIVE_OFF):
         """Retrieve a value from the given object.
@@ -763,14 +774,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl):
         if self.dispatch._active_history:
             old = self.get(state, dict_, passive=PASSIVE_ONLY_PERSISTENT | NO_AUTOFLUSH)
         else:
-            # would like to call with PASSIVE_NO_FETCH ^ INIT_OK.  However,
-            # we have a long-standing behavior that a "get()" on never set
-            # should implicitly set the value to None.  Leaving INIT_OK
-            # set here means we are consistent whether or not we did a get
-            # first.
-            # see test_use_object_set_None vs. test_use_object_get_first_set_None
-            # in test_attributes.py
-            old = self.get(state, dict_, passive=PASSIVE_NO_FETCH)
+            old = self.get(state, dict_, passive=PASSIVE_NO_FETCH ^ INIT_OK)
 
         if check_old is not None and \
              old is not PASSIVE_NO_RESULT and \
@@ -1087,7 +1091,7 @@ def backref_listeners(attribute, key, uselist):
     def emit_backref_from_scalar_set_event(state, child, oldchild, initiator):
         if oldchild is child:
             return child
-        if oldchild is not None and oldchild not in (PASSIVE_NO_RESULT, NEVER_SET):
+        if oldchild not in _none_tuple:
             # With lazy=None, there's no guarantee that the full collection is
             # present when updating via a backref.
             old_state, old_dict = instance_state(oldchild),\
@@ -1175,7 +1179,7 @@ _NO_STATE_SYMBOLS = frozenset([
 
 History = util.namedtuple("History", [
         "added", "unchanged", "deleted"
-    ])
+])
 
 
 class History(History):
diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py
index e81375787..896041980 100644
--- a/lib/sqlalchemy/orm/base.py
+++ b/lib/sqlalchemy/orm/base.py
@@ -151,7 +151,8 @@ NOT_EXTENSION = util.symbol('NOT_EXTENSION',
 
 """)
 
-_none_set = frozenset([None])
+_none_set = frozenset([None, NEVER_SET, PASSIVE_NO_RESULT])
+_none_tuple = tuple(_none_set)  # for "in" checks that won't trip __hash__
 
 
 def _generative(*assertions):
diff --git a/lib/sqlalchemy/orm/dependency.py b/lib/sqlalchemy/orm/dependency.py
index 0d5a4f909..68ae0a0e4 100644
--- a/lib/sqlalchemy/orm/dependency.py
+++ b/lib/sqlalchemy/orm/dependency.py
@@ -745,11 +745,6 @@ class ManyToOneDP(DependencyProcessor):
                     for child in history.added:
                         self._synchronize(state, child, None, False,
                                                 uowcommit, "add")
-                elif history.unchanged == [None]:
-                    # this is to appease the case where our row
-                    # here is in fact going to be a so-called "row switch",
-                    # where an INSERT becomes an UPDATE.  See #3060.
-                    self._synchronize(state, None, None, True, uowcommit)
                 if self.post_update:
                     self._post_update(state, uowcommit, history.sum())
 
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 1fe924d96..dad1f1a0a 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -329,7 +329,7 @@ def instance_processor(mapper, context, path, adapter,
     if mapper.allow_partial_pks:
         is_not_primary_key = _none_set.issuperset
     else:
-        is_not_primary_key = _none_set.issubset
+        is_not_primary_key = _none_set.intersection
 
     def _instance(row, result):
         if not new_populators and invoke_all_eagers:
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 3e93840a1..373e18271 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -24,6 +24,7 @@ from .. import sql, util, log, exc as sa_exc, event, schema, inspection
 from ..sql import expression, visitors, operators, util as sql_util
 from . import instrumentation, attributes, exc as orm_exc, loading
 from . import properties
+from . import util as orm_util
 from .interfaces import MapperProperty, _InspectionAttr, _MappedAttribute
 
 from .base import _class_to_mapper, _state_mapper, class_mapper, \
@@ -2270,7 +2271,7 @@ class Mapper(_InspectionAttr):
         manager = state.manager
         return self._identity_class, tuple([
             manager[self._columntoproperty[col].key].\
-                impl.get(state, dict_, attributes.PASSIVE_OFF)
+                impl.get(state, dict_, attributes.PASSIVE_RETURN_NEVER_SET)
             for col in self.primary_key
         ])
 
@@ -2292,12 +2293,13 @@ class Mapper(_InspectionAttr):
         manager = state.manager
         return [
             manager[self._columntoproperty[col].key].\
-                impl.get(state, dict_, attributes.PASSIVE_OFF)
+                impl.get(state, dict_,
+                                attributes.PASSIVE_RETURN_NEVER_SET)
             for col in self.primary_key
         ]
 
     def _get_state_attr_by_column(self, state, dict_, column,
-                                    passive=attributes.PASSIVE_OFF):
+                        passive=attributes.PASSIVE_RETURN_NEVER_SET):
         prop = self._columntoproperty[column]
         return state.manager[prop.key].impl.get(state, dict_, passive=passive)
 
@@ -2311,7 +2313,8 @@ class Mapper(_InspectionAttr):
         return self._get_committed_state_attr_by_column(state, dict_, column)
 
     def _get_committed_state_attr_by_column(self, state, dict_,
-                        column, passive=attributes.PASSIVE_OFF):
+                        column,
+                        passive=attributes.PASSIVE_RETURN_NEVER_SET):
 
         prop = self._columntoproperty[column]
         return state.manager[prop.key].impl.\
@@ -2352,7 +2355,7 @@ class Mapper(_InspectionAttr):
                                     state, state.dict,
                                     leftcol,
                                     passive=attributes.PASSIVE_NO_INITIALIZE)
-                if leftval is attributes.PASSIVE_NO_RESULT or leftval is None:
+                if leftval in orm_util._none_set:
                     raise ColumnsNotAvailable()
                 binary.left = sql.bindparam(None, leftval,
                                             type_=binary.right.type)
@@ -2361,8 +2364,7 @@ class Mapper(_InspectionAttr):
                                     state, state.dict,
                                     rightcol,
                                     passive=attributes.PASSIVE_NO_INITIALIZE)
-                if rightval is attributes.PASSIVE_NO_RESULT or \
-                            rightval is None:
+                if rightval in orm_util._none_set:
                     raise ColumnsNotAvailable()
                 binary.right = sql.bindparam(None, rightval,
                                             type_=binary.right.type)
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 49d9d11b9..b4c31a027 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -386,11 +386,6 @@ def _collect_update_commands(base_mapper, uowtransaction,
                         hasnull = True
                     params[col._label] = value
 
-                # see #3060.   Need to consider an "unchanged" None
-                # as potentially history for now.
-                elif row_switch and history.unchanged == [None]:
-                    params[col.key] = None
-                    hasdata = True
         if hasdata:
             if hasnull:
                 raise orm_exc.FlushError(
diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index 89d9946ee..3cc03a2d4 100644
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -518,28 +518,28 @@ class Session(_SessionClassMethods):
             :ref:`session_autocommit`
 
         :param autoflush: When ``True``, all query operations will issue a
-           :meth:`~.Session.flush` call to this ``Session`` before proceeding. 
-           This is a convenience feature so that :meth:`~.Session.flush` need 
-           not be called repeatedly in order for database queries to retrieve 
+           :meth:`~.Session.flush` call to this ``Session`` before proceeding.
+           This is a convenience feature so that :meth:`~.Session.flush` need
+           not be called repeatedly in order for database queries to retrieve
            results. It's typical that ``autoflush`` is used in conjunction with
            ``autocommit=False``. In this scenario, explicit calls to
-           :meth:`~.Session.flush` are rarely needed; you usually only need to 
+           :meth:`~.Session.flush` are rarely needed; you usually only need to
            call :meth:`~.Session.commit` (which flushes) to finalize changes.
 
-        :param bind: An optional :class:`.Engine` or :class:`.Connection` to 
-           which this ``Session`` should be bound. When specified, all SQL 
-           operations performed by this session will execute via this 
+        :param bind: An optional :class:`.Engine` or :class:`.Connection` to
+           which this ``Session`` should be bound. When specified, all SQL
+           operations performed by this session will execute via this
            connectable.
 
         :param binds: An optional dictionary which contains more granular
            "bind" information than the ``bind`` parameter provides. This
-           dictionary can map individual :class`.Table` 
-           instances as well as :class:`~.Mapper` instances to individual 
-           :class:`.Engine` or :class:`.Connection` objects. Operations which 
-           proceed relative to a particular :class:`.Mapper` will consult this 
-           dictionary for the direct :class:`.Mapper` instance as 
-           well as the mapper's ``mapped_table`` attribute in order to locate a 
-           connectable to use. The full resolution is described in the 
+           dictionary can map individual :class`.Table`
+           instances as well as :class:`~.Mapper` instances to individual
+           :class:`.Engine` or :class:`.Connection` objects. Operations which
+           proceed relative to a particular :class:`.Mapper` will consult this
+           dictionary for the direct :class:`.Mapper` instance as
+           well as the mapper's ``mapped_table`` attribute in order to locate a
+           connectable to use. The full resolution is described in the
            :meth:`.Session.get_bind`.
            Usage looks like::
 
@@ -566,8 +566,8 @@ class Session(_SessionClassMethods):
            :meth:`~.Session.begin`, all of which are interdependent.
 
         :param expire_on_commit:  Defaults to ``True``. When ``True``, all
-           instances will be fully expired after each :meth:`~.commit`, 
-           so that all attribute/object access subsequent to a completed 
+           instances will be fully expired after each :meth:`~.commit`,
+           so that all attribute/object access subsequent to a completed
            transaction will load from the most recent database state.
 
         :param extension: An optional
@@ -585,16 +585,16 @@ class Session(_SessionClassMethods):
            .. versionadded:: 0.9.0
 
         :param query_cls:  Class which should be used to create new Query
-           objects, as returned by the :meth:`~.Session.query` method. Defaults 
+           objects, as returned by the :meth:`~.Session.query` method. Defaults
            to :class:`.Query`.
 
         :param twophase:  When ``True``, all transactions will be started as
             a "two phase" transaction, i.e. using the "two phase" semantics
-            of the database in use along with an XID.  During a 
-            :meth:`~.commit`, after :meth:`~.flush` has been issued for all 
+            of the database in use along with an XID.  During a
+            :meth:`~.commit`, after :meth:`~.flush` has been issued for all
             attached databases, the :meth:`~.TwoPhaseTransaction.prepare` method
-            on each database's :class:`.TwoPhaseTransaction` will be called. 
-            This allows each database to roll back the entire transaction, 
+            on each database's :class:`.TwoPhaseTransaction` will be called.
+            This allows each database to roll back the entire transaction,
             before each transaction is committed.
 
         :param weak_identity_map:  Defaults to ``True`` - when set to
@@ -1048,7 +1048,7 @@ class Session(_SessionClassMethods):
         bind
           Any Connectable: a :class:`.Engine` or :class:`.Connection`.
 
-        All subsequent operations involving this :class:`.Table` will use the 
+        All subsequent operations involving this :class:`.Table` will use the
         given `bind`.
 
         """
@@ -1149,7 +1149,7 @@ class Session(_SessionClassMethods):
             ', '.join(context)))
 
     def query(self, *entities, **kwargs):
-        """Return a new :class:`.Query` object corresponding to this 
+        """Return a new :class:`.Query` object corresponding to this
         :class:`.Session`."""
 
         return self._query_cls(entities, self, **kwargs)
@@ -1403,7 +1403,7 @@ class Session(_SessionClassMethods):
 
                 instance_key = mapper._identity_key_from_state(state)
 
-                if _none_set.issubset(instance_key[1]) and \
+                if _none_set.intersection(instance_key[1]) and \
                     not mapper.allow_partial_pks or \
                     _none_set.issuperset(instance_key[1]):
                     raise exc.FlushError(
@@ -1635,7 +1635,7 @@ class Session(_SessionClassMethods):
             self._update_impl(merged_state)
             new_instance = True
 
-        elif not _none_set.issubset(key[1]) or \
+        elif not _none_set.intersection(key[1]) or \
                     (mapper.allow_partial_pks and
                     not _none_set.issuperset(key[1])):
             merged = self.query(mapper.class_).get(key[1])
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index c8946a0c0..2674b9c6f 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -564,7 +564,7 @@ class LazyLoader(AbstractRelationshipLoader):
 
         if pending:
             bind_values = sql_util.bind_values(lazy_clause)
-            if None in bind_values:
+            if orm_util._none_set.intersection(bind_values):
                 return None
 
         q = q.filter(lazy_clause)
diff --git a/lib/sqlalchemy/orm/sync.py b/lib/sqlalchemy/orm/sync.py
index aed98bdf0..7556e9f71 100644
--- a/lib/sqlalchemy/orm/sync.py
+++ b/lib/sqlalchemy/orm/sync.py
@@ -48,7 +48,7 @@ def clear(dest, dest_mapper, synchronize_pairs):
     for l, r in synchronize_pairs:
         if r.primary_key and \
             dest_mapper._get_state_attr_by_column(
-                                        dest, dest.dict, r) is not None:
+                                dest, dest.dict, r) not in orm_util._none_set:
 
             raise AssertionError(
                 "Dependency rule tried to blank-out primary key "
diff --git a/lib/sqlalchemy/orm/unitofwork.py b/lib/sqlalchemy/orm/unitofwork.py
index 2964705a2..3ef2b2edf 100644
--- a/lib/sqlalchemy/orm/unitofwork.py
+++ b/lib/sqlalchemy/orm/unitofwork.py
@@ -86,9 +86,8 @@ def track_cascade_events(descriptor, prop):
                     not sess._contains_state(newvalue_state):
                     sess._save_or_update_state(newvalue_state)
 
-            if oldvalue is not None and \
-                oldvalue is not attributes.PASSIVE_NO_RESULT and \
-                prop._cascade.delete_orphan:
+            if oldvalue not in orm_util._none_tuple and \
+                    prop._cascade.delete_orphan:
                 # possible to reach here with attributes.NEVER_SET ?
                 oldvalue_state = attributes.instance_state(oldvalue)
 
diff --git a/lib/sqlalchemy/orm/util.py b/lib/sqlalchemy/orm/util.py
index 8694705a4..fd902adaf 100644
--- a/lib/sqlalchemy/orm/util.py
+++ b/lib/sqlalchemy/orm/util.py
@@ -12,7 +12,8 @@ from . import attributes
 import re
 
 from .base import instance_str, state_str, state_class_str, attribute_str, \
-        state_attribute_str, object_mapper, object_state, _none_set
+        state_attribute_str, object_mapper, object_state, _none_set, \
+        _none_tuple
 from .base import class_mapper, _class_to_mapper
 from .base import _InspectionAttr
 from .path_registry import PathRegistry