- don't load deferred columns on unexpire for merge with load=False,

fixes #3488

Change-Id: Ic9577b800e4a4e2465ec7f3a2e95bd231f5337ee
Co-Authored-By: Mike Bayer <mike_mp@zzzcomputing.com>

doc/build/changelog/changelog_11.rst

@@ -21,6 +21,14 @@
 .. changelog::
     :version: 1.1.0b1
 
+    .. change::
+        :tags: bug, orm
+        :tickets: 3488
+
+        Fixed bug where deferred columns would inadvertently be set up
+        for database load on the next object-wide unexpire, when the object
+        were merged into the session with ``session.merge(obj, load=False)``.
+
     .. change::
         :tags: feature, sql
         :pullreq: github:231

lib/sqlalchemy/orm/properties.py

@@ -218,7 +218,8 @@ class ColumnProperty(StrategizedProperty):
                 impl = dest_state.get_impl(self.key)
                 impl.set(dest_state, dest_dict, value, None)
         elif dest_state.has_identity and self.key not in dest_dict:
-            dest_state._expire_attributes(dest_dict, [self.key])
+            dest_state._expire_attributes(
+                dest_dict, [self.key], no_loader=True)
 
     class Comparator(util.MemoizedSlots, PropComparator):
         """Produce boolean, comparison, and other operators for

lib/sqlalchemy/orm/session.py

@@ -1842,6 +1842,13 @@ class Session(_SessionClassMethods):
             merged_state.load_path = state.load_path
             merged_state.load_options = state.load_options
 
+            # since we are copying load_options, we need to copy
+            # the callables_ that would have been generated by those
+            # load_options.
+            # assumes that the callables we put in state.callables_
+            # are not instance-specific (which they should not be)
+            merged_state._copy_callables(state)
+
             for prop in mapper.iterate_properties:
                 prop.merge(self, state, state_dict,
                            merged_state, merged_dict,

lib/sqlalchemy/orm/state.py

@@ -485,6 +485,10 @@ class InstanceState(interfaces.InspectionAttr):
         if self.callables:
             self.callables.pop(key, None)
 
+    def _copy_callables(self, from_):
+        if 'callables' in from_.__dict__:
+            self.callables = dict(from_.callables)
+
     @classmethod
     def _instance_level_callable_processor(cls, manager, fn, key):
         impl = manager[key].impl
@@ -537,7 +541,7 @@ class InstanceState(interfaces.InspectionAttr):
 
         self.manager.dispatch.expire(self, None)
 
-    def _expire_attributes(self, dict_, attribute_names):
+    def _expire_attributes(self, dict_, attribute_names, no_loader=False):
         pending = self.__dict__.get('_pending_mutations', None)
 
         callables = self.callables
@@ -545,6 +549,12 @@ class InstanceState(interfaces.InspectionAttr):
         for key in attribute_names:
             impl = self.manager[key].impl
             if impl.accepts_scalar_loader:
+                if no_loader and (
+                    impl.callable_ or
+                    key in callables
+                ):
+                    continue
+
                 self.expired_attributes.add(key)
                 if callables and key in callables:
                     del callables[key]

test/orm/test_merge.py

@@ -1,15 +1,15 @@
-from sqlalchemy.testing import assert_raises, assert_raises_message
+from sqlalchemy.testing import assert_raises_message
 import sqlalchemy as sa
-from sqlalchemy import Integer, PickleType, String, ForeignKey
+from sqlalchemy import Integer, PickleType, String, ForeignKey, Text
 import operator
 from sqlalchemy import testing
 from sqlalchemy.util import OrderedSet
 from sqlalchemy.orm import mapper, relationship, create_session, \
     PropComparator, synonym, comparable_property, sessionmaker, \
-    attributes, Session, backref, configure_mappers, foreign
+    attributes, Session, backref, configure_mappers, foreign, deferred, defer
 from sqlalchemy.orm.collections import attribute_mapped_collection
 from sqlalchemy.orm.interfaces import MapperOption
-from sqlalchemy.testing import eq_, ne_
+from sqlalchemy.testing import eq_, in_, not_in_
 from sqlalchemy.testing import fixtures
 from test.orm import _fixtures
 from sqlalchemy import event, and_, case
@@ -1378,6 +1378,102 @@ class M2ONoUseGetLoadingTest(fixtures.MappedTest):
             assert a2.user is u2
         self.assert_sql_count(testing.db, go, 5)
 
+
+class DeferredMergeTest(fixtures.MappedTest):
+    @classmethod
+    def define_tables(cls, metadata):
+        Table(
+            'book', metadata,
+            Column('id', Integer, primary_key=True),
+            Column('title', String(200), nullable=False),
+            Column('summary', String(2000)),
+            Column('excerpt', Text),
+        )
+
+    @classmethod
+    def setup_classes(cls):
+        class Book(cls.Basic):
+            pass
+
+    def test_deferred_column_mapping(self):
+        # defer 'excerpt' at mapping level instead of query level
+        Book, book = self.classes.Book, self.tables.book
+        mapper(Book, book, properties={'excerpt': deferred(book.c.excerpt)})
+        sess = sessionmaker()()
+
+        b = Book(
+            id=1,
+            title='Essential SQLAlchemy',
+            summary='some summary',
+            excerpt='some excerpt',
+        )
+        sess.add(b)
+        sess.commit()
+
+        b1 = sess.query(Book).first()
+        sess.expire(b1, ['summary'])
+        sess.close()
+
+        def go():
+            b2 = sess.merge(b1, load=False)
+
+            # should not emit load for deferred 'excerpt'
+            eq_(b2.summary, 'some summary')
+            not_in_('excerpt', b2.__dict__)
+
+            # now it should emit load for deferred 'excerpt'
+            eq_(b2.excerpt, 'some excerpt')
+            in_('excerpt', b2.__dict__)
+
+        self.sql_eq_(go, [
+            ("SELECT book.summary AS book_summary "
+             "FROM book WHERE book.id = :param_1",
+             {'param_1': 1}),
+            ("SELECT book.excerpt AS book_excerpt "
+             "FROM book WHERE book.id = :param_1",
+             {'param_1': 1})
+        ])
+
+    def test_deferred_column_query(self):
+        Book, book = self.classes.Book, self.tables.book
+        mapper(Book, book)
+        sess = sessionmaker()()
+
+        b = Book(
+            id=1,
+            title='Essential SQLAlchemy',
+            summary='some summary',
+            excerpt='some excerpt',
+        )
+        sess.add(b)
+        sess.commit()
+
+        # defer 'excerpt' at query level instead of mapping level
+        b1 = sess.query(Book).options(defer(Book.excerpt)).first()
+        sess.expire(b1, ['summary'])
+        sess.close()
+
+        def go():
+            b2 = sess.merge(b1, load=False)
+
+            # should not emit load for deferred 'excerpt'
+            eq_(b2.summary, 'some summary')
+            not_in_('excerpt', b2.__dict__)
+
+            # now it should emit load for deferred 'excerpt'
+            eq_(b2.excerpt, 'some excerpt')
+            in_('excerpt', b2.__dict__)
+
+        self.sql_eq_(go, [
+            ("SELECT book.summary AS book_summary "
+             "FROM book WHERE book.id = :param_1",
+             {'param_1': 1}),
+            ("SELECT book.excerpt AS book_excerpt "
+             "FROM book WHERE book.id = :param_1",
+             {'param_1': 1})
+        ])
+
+
 class MutableMergeTest(fixtures.MappedTest):
     @classmethod
     def define_tables(cls, metadata):