- Fixed bug where attribute "set" events or columns with

``@validates`` would have events triggered within the flush process,
when those columns were the targets of a "fetch and populate"
operation, such as an autoincremented primary key, a Python side
default, or a server-side default "eagerly" fetched via RETURNING.
fixes #3167

5 files changed, 83 insertions, 20 deletions

diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst
index fb639ddf7..1cbbec3b3 100644
--- a/doc/build/changelog/changelog_10.rst
+++ b/doc/build/changelog/changelog_10.rst
@@ -17,6 +17,16 @@
 	:version: 1.0.0
 
     .. change::
+        :tags: bug, orm
+        :tickets: 3167
+
+        Fixed bug where attribute "set" events or columns with
+        ``@validates`` would have events triggered within the flush process,
+        when those columns were the targets of a "fetch and populate"
+        operation, such as an autoincremented primary key, a Python side
+        default, or a server-side default "eagerly" fetched via RETURNING.
+
+    .. change::
         :tags: bug, orm, py3k
 
         The :class:`.IdentityMap` exposed from :class:`.Session.identity`
diff --git a/doc/build/orm/mapper_config.rst b/doc/build/orm/mapper_config.rst
index 9139b53f0..d0679c721 100644
--- a/doc/build/orm/mapper_config.rst
+++ b/doc/build/orm/mapper_config.rst
@@ -667,6 +667,12 @@ issued when the ORM is populating the object::
             assert '@' in address
             return address
 
+.. versionchanged:: 1.0.0 - validators are no longer triggered within
+   the flush process when the newly fetched values for primary key
+   columns as well as some python- or server-side defaults are fetched.
+   Prior to 1.0, validators may be triggered in those cases as well.
+
+
 Validators also receive collection append events, when items are added to a
 collection::
 
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index fc15769cd..1e1291857 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1189,14 +1189,6 @@ class Mapper(InspectionAttr):
                 util.ordered_column_set(t.c).\
                 intersection(all_cols)
 
-        # determine cols that aren't expressed within our tables; mark these
-        # as "read only" properties which are refreshed upon INSERT/UPDATE
-        self._readonly_props = set(
-            self._columntoproperty[col]
-            for col in self._columntoproperty
-            if not hasattr(col, 'table') or
-            col.table not in self._cols_by_table)
-
         # if explicit PK argument sent, add those columns to the
         # primary key mappings
         if self._primary_key_argument:
@@ -1247,6 +1239,15 @@ class Mapper(InspectionAttr):
             self.primary_key = tuple(primary_key)
             self._log("Identified primary key columns: %s", primary_key)
 
+        # determine cols that aren't expressed within our tables; mark these
+        # as "read only" properties which are refreshed upon INSERT/UPDATE
+        self._readonly_props = set(
+            self._columntoproperty[col]
+            for col in self._columntoproperty
+            if self._columntoproperty[col] not in self._primary_key_props and
+            (not hasattr(col, 'table') or
+                col.table not in self._cols_by_table))
+
     def _configure_properties(self):
 
         # Column and other ClauseElement objects which are mapped
@@ -2342,18 +2343,26 @@ class Mapper(InspectionAttr):
         dict_ = state.dict
         manager = state.manager
         return [
-            manager[self._columntoproperty[col].key].
+            manager[prop.key].
             impl.get(state, dict_,
                      attributes.PASSIVE_RETURN_NEVER_SET)
-            for col in self.primary_key
+            for prop in self._primary_key_props
         ]
 
+    @_memoized_configured_property
+    def _primary_key_props(self):
+        return [self._columntoproperty[col] for col in self.primary_key]
+
     def _get_state_attr_by_column(
             self, state, dict_, column,
             passive=attributes.PASSIVE_RETURN_NEVER_SET):
         prop = self._columntoproperty[column]
         return state.manager[prop.key].impl.get(state, dict_, passive=passive)
 
+    def _set_committed_state_attr_by_column(self, state, dict_, column, value):
+        prop = self._columntoproperty[column]
+        state.manager[prop.key].impl.set_committed_value(state, dict_, value)
+
     def _set_state_attr_by_column(self, state, dict_, column, value):
         prop = self._columntoproperty[column]
         state.manager[prop.key].impl.set(state, dict_, value, None)
diff --git a/lib/sqlalchemy/orm/persistence.py b/lib/sqlalchemy/orm/persistence.py
index 8c9b677fe..d511c0816 100644
--- a/lib/sqlalchemy/orm/persistence.py
+++ b/lib/sqlalchemy/orm/persistence.py
@@ -645,13 +645,7 @@ def _emit_insert_statements(base_mapper, uowtransaction,
                                        mapper._pks_by_table[table]):
                         prop = mapper_rec._columntoproperty[col]
                         if state_dict.get(prop.key) is None:
-                            # TODO: would rather say:
-                            # state_dict[prop.key] = pk
-                            mapper_rec._set_state_attr_by_column(
-                                state,
-                                state_dict,
-                                col, pk)
-
+                            state_dict[prop.key] = pk
                 _postfetch(
                     mapper_rec,
                     uowtransaction,
@@ -836,11 +830,11 @@ def _postfetch(mapper, uowtransaction, table,
             for col in returning_cols:
                 if col.primary_key:
                     continue
-                mapper._set_state_attr_by_column(state, dict_, col, row[col])
+                dict_[mapper._columntoproperty[col].key] = row[col]
 
     for c in prefetch_cols:
         if c.key in params and c in mapper._columntoproperty:
-            mapper._set_state_attr_by_column(state, dict_, c, params[c.key])
+            dict_[mapper._columntoproperty[c].key] = params[c.key]
 
     if postfetch_cols:
         state._expire_attributes(state.dict,
diff --git a/test/orm/test_unitofworkv2.py b/test/orm/test_unitofworkv2.py
index c643e6a87..122fe2514 100644
--- a/test/orm/test_unitofworkv2.py
+++ b/test/orm/test_unitofworkv2.py
@@ -9,8 +9,9 @@ from sqlalchemy import Integer, String, ForeignKey, func
 from sqlalchemy.orm import mapper, relationship, backref, \
     create_session, unitofwork, attributes,\
     Session, exc as orm_exc
-
+from sqlalchemy.testing.mock import Mock
 from sqlalchemy.testing.assertsql import AllOf, CompiledSQL
+from sqlalchemy import event
 
 
 class AssertsUOW(object):
@@ -1703,3 +1704,46 @@ class LoadersUsingCommittedTest(UOWTest):
             sess.flush()
         except AvoidReferencialError:
             pass
+
+
+class NoAttrEventInFlushTest(fixtures.MappedTest):
+    """test [ticket:3167]"""
+
+    __backend__ = True
+
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'test', metadata,
+            Column('id', Integer, primary_key=True,
+                   test_needs_autoincrement=True),
+            Column('prefetch_val', Integer, default=5),
+            Column('returning_val', Integer, server_default="5")
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Thing(cls.Basic):
+            pass
+
+    @classmethod
+    def setup_mappers(cls):
+        Thing = cls.classes.Thing
+
+        mapper(Thing, cls.tables.test, eager_defaults=True)
+
+    def test_no_attr_events_flush(self):
+        Thing = self.classes.Thing
+        mock = Mock()
+        event.listen(Thing.id, "set", mock.id)
+        event.listen(Thing.prefetch_val, "set", mock.prefetch_val)
+        event.listen(Thing.returning_val, "set", mock.prefetch_val)
+        t1 = Thing()
+        s = Session()
+        s.add(t1)
+        s.flush()
+
+        eq_(len(mock.mock_calls), 0)
+        eq_(t1.id, 1)
+        eq_(t1.prefetch_val, 5)
+        eq_(t1.returning_val, 5)