- attributes now has an "active_history" flag. This flag indicates that when new value is set or the existing value is deleted, we absolutely need the previous value to be present, including if it requires hitting a lazy loader. Since somewhere around 0.4 we had not been loading the previous value as a performance optimization.

- the flag is set by a ColumnLoader which contains a primary key column.  This allows the mapper to have an accurate record of a primary key column when _save_obj() performs an UPDATE.
- the definition of who gets "active_history" may be expanded to include ForeignKey and any columns participating in a primaryjoin/seconddary join, so that lazyloaders can execute correctly on an expired object with pending changes to those attributes.
- expire-on-commit is why this is becoming a more important issue as of late
- fixes [ticket:1151], but unit tests, CHANGES note is pending

2 files changed, 22 insertions, 8 deletions

diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index ff6c7b02c..6fd134242 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -197,7 +197,7 @@ def proxied_attribute_factory(descriptor):
 class AttributeImpl(object):
     """internal implementation for instrumented attributes."""
 
-    def __init__(self, class_, key, callable_, class_manager, trackparent=False, extension=None, compare_function=None, **kwargs):
+    def __init__(self, class_, key, callable_, class_manager, trackparent=False, extension=None, compare_function=None, active_history=False, **kwargs):
         """Construct an AttributeImpl.
 
         \class_
@@ -231,6 +231,7 @@ class AttributeImpl(object):
         self.callable_ = callable_
         self.class_manager = class_manager
         self.trackparent = trackparent
+        self.active_history = active_history
         if compare_function is None:
             self.is_equal = operator.eq
         else:
@@ -364,11 +365,16 @@ class ScalarAttributeImpl(AttributeImpl):
     uses_objects = False
 
     def delete(self, state):
-        state.modified_event(self, False, state.dict.get(self.key, NO_VALUE))
 
         # TODO: catch key errors, convert to attributeerror?
-        if self.extensions:
+        if self.active_history or self.extensions:
             old = self.get(state)
+        else:
+            old = state.dict.get(self.key, NO_VALUE)
+
+        state.modified_event(self, False, old)
+
+        if self.extensions:
             del state.dict[self.key]
             self.fire_remove_event(state, old, None)
         else:
@@ -382,10 +388,14 @@ class ScalarAttributeImpl(AttributeImpl):
         if initiator is self:
             return
 
-        state.modified_event(self, False, state.dict.get(self.key, NO_VALUE))
+        if self.active_history or self.extensions:
+            old = self.get(state)
+        else:
+            old = state.dict.get(self.key, NO_VALUE)
+            
+        state.modified_event(self, False, old)
 
         if self.extensions:
-            old = self.get(state)
             state.dict[self.key] = value
             self.fire_replace_event(state, value, old, initiator)
         else:
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index e2adf701d..d6b53b9c8 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -20,7 +20,7 @@ from sqlalchemy.orm import util as mapperutil
 
 
 class DefaultColumnLoader(LoaderStrategy):
-    def _register_attribute(self, compare_function, copy_function, mutable_scalars, comparator_factory, callable_=None, proxy_property=None):
+    def _register_attribute(self, compare_function, copy_function, mutable_scalars, comparator_factory, callable_=None, proxy_property=None, active_history=False):
         self.logger.info("%s register managed attribute" % self)
 
         for mapper in self.parent.polymorphic_iterator():
@@ -36,7 +36,8 @@ class DefaultColumnLoader(LoaderStrategy):
                     comparator=comparator_factory(self.parent_property, mapper), 
                     parententity=mapper,
                     callable_=callable_,
-                    proxy_property=proxy_property
+                    proxy_property=proxy_property,
+                    active_history=active_history
                     )
 
 DefaultColumnLoader.logger = log.class_logger(DefaultColumnLoader)
@@ -57,12 +58,15 @@ class ColumnLoader(DefaultColumnLoader):
     def init_class_attribute(self):
         self.is_class_level = True
         coltype = self.columns[0].type
+        active_history = self.columns[0].primary_key  # TODO: check all columns ?  check for foreign Key as well?
         
         self._register_attribute(
             coltype.compare_values,
             coltype.copy_value,
             self.columns[0].type.is_mutable(),
-            self.parent_property.comparator_factory
+            self.parent_property.comparator_factory,
+            active_history = active_history
+            
        )
         
     def create_row_processor(self, selectcontext, path, mapper, row, adapter):