diff options
author | Diana Clarke <diana.joan.clarke@gmail.com> | 2016-04-05 18:58:21 -0400 |
---|---|---|
committer | Mike Bayer <mike_mp@zzzcomputing.com> | 2016-04-06 13:28:25 -0400 |
commit | bef15a950ec4140479bc244f9ca57b5da7c9bb3f (patch) | |
tree | 79ed72593990bb7ba482fabed575aaa8acbd815b | |
parent | fdb6ab6a1d5d55d900c388e039835c6433032977 (diff) | |
download | sqlalchemy-bef15a950ec4140479bc244f9ca57b5da7c9bb3f.tar.gz |
- 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>
-rw-r--r-- | doc/build/changelog/changelog_11.rst | 8 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/properties.py | 3 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/session.py | 7 | ||||
-rw-r--r-- | lib/sqlalchemy/orm/state.py | 12 | ||||
-rw-r--r-- | test/orm/test_merge.py | 104 |
5 files changed, 128 insertions, 6 deletions
diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index 18d6d1f74..53bd38a98 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -22,6 +22,14 @@ :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 diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index 8197e041f..f3dce7541 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/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 diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index dc5de7ac6..1cf1bdb24 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/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, diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index 1ad09ee83..2704367f9 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/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] diff --git a/test/orm/test_merge.py b/test/orm/test_merge.py index f69b07fe8..8a3419ecc 100644 --- a/test/orm/test_merge.py +++ b/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): |