From 7b019f3d49cb2e4340e4c2029e9841af0153b2e6 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 7 Aug 2009 21:14:32 +0000 Subject: [PATCH] - renamed PASSIVE_NORESULT to PASSIVE_NO_RESULT - renamed PASSIVE_NO_CALLABLES to PASSIVE_NO_FETCH - passive now propagates all the way through lazy callables, all the way into query._get(), so that many-to-one lazy load can load the instance via the local session but not trigger any SQL if not available, fixes [ticket:1298] without messing up consistency of tests added in r6201 - many-to-one also handles returning PASSIVE_NO_RESULT for the "old" value thus eliminating the need for the previous value even if the new value is None - query._get() uses identity_map.get(), which has been changed to no longer raise KeyError, thus providing mythical time savings that didn't seem to make any difference in how fast the unit tests ran. --- 06CHANGES | 4 ++- lib/sqlalchemy/orm/attributes.py | 57 +++++++++++++----------------- lib/sqlalchemy/orm/identity.py | 11 ++++-- lib/sqlalchemy/orm/mapper.py | 4 +-- lib/sqlalchemy/orm/query.py | 13 ++++--- lib/sqlalchemy/orm/state.py | 12 ++++--- lib/sqlalchemy/orm/strategies.py | 23 ++++++++---- test/orm/test_attributes.py | 39 ++++++++++---------- test/orm/test_backref_mutations.py | 47 ++++++++++++++++++++++-- test/orm/test_dynamic.py | 2 +- test/orm/test_extendedattr.py | 6 ++-- 11 files changed, 140 insertions(+), 78 deletions(-) diff --git a/06CHANGES b/06CHANGES index 4c7f9ca693..476867ad08 100644 --- a/06CHANGES +++ b/06CHANGES @@ -9,7 +9,9 @@ code structure. - the "dont_load=True" flag on Session.merge() is deprecated and is now "load=False". - + - many-to-one relations now fire off a lazyload in fewer cases, including + in most cases will not fetch the "old" value when a new one is replaced. + - sql - returning() support is native to insert(), update(), delete(). Implementations of varying levels of functionality exist for Postgresql, Firebird, MSSQL and diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py index f6947dbc11..3ca7d83119 100644 --- a/lib/sqlalchemy/orm/attributes.py +++ b/lib/sqlalchemy/orm/attributes.py @@ -28,7 +28,7 @@ _entity_info = None identity_equal = None state = None -PASSIVE_NORESULT = util.symbol('PASSIVE_NORESULT') +PASSIVE_NO_RESULT = util.symbol('PASSIVE_NO_RESULT') ATTR_WAS_SET = util.symbol('ATTR_WAS_SET') NO_VALUE = util.symbol('NO_VALUE') NEVER_SET = util.symbol('NEVER_SET') @@ -43,7 +43,7 @@ PASSIVE_NO_INITIALIZE = True #util.symbol('PASSIVE_NO_INITIALIZE') # don't fire off any callables, but if no callables present # then initialize to an empty value/collection # this is used by backrefs. -PASSIVE_NO_CALLABLES = util.symbol('PASSIVE_NO_CALLABLES') +PASSIVE_NO_FETCH = util.symbol('PASSIVE_NO_FETCH') # fire callables/initialize as needed PASSIVE_OFF = False #util.symbol('PASSIVE_OFF') @@ -368,14 +368,16 @@ class AttributeImpl(object): # if no history, check for lazy callables, etc. if state.committed_state.get(self.key, NEVER_SET) is NEVER_SET: if passive is PASSIVE_NO_INITIALIZE: - return PASSIVE_NORESULT + return PASSIVE_NO_RESULT callable_ = self._get_callable(state) if callable_ is not None: - if passive is not PASSIVE_OFF: - return PASSIVE_NORESULT - value = callable_() - if value is not ATTR_WAS_SET: + #if passive is not PASSIVE_OFF: + # return PASSIVE_NO_RESULT + value = callable_(passive=passive) + if value is PASSIVE_NO_RESULT: + return value + elif value is not ATTR_WAS_SET: return self.set_committed_value(state, dict_, value) else: if self.key not in dict_: @@ -504,7 +506,7 @@ class MutableScalarAttributeImpl(ScalarAttributeImpl): def get(self, state, dict_, passive=PASSIVE_OFF): if self.key not in state.mutable_dict: ret = ScalarAttributeImpl.get(self, state, dict_, passive=passive) - if ret is not PASSIVE_NORESULT: + if ret is not PASSIVE_NO_RESULT: state.mutable_dict[self.key] = ret return ret else: @@ -557,7 +559,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): return History.from_attribute(self, state, dict_[self.key]) else: current = self.get(state, dict_, passive=passive) - if current is PASSIVE_NORESULT: + if current is PASSIVE_NO_RESULT: return HISTORY_BLANK else: return History.from_attribute(self, state, current) @@ -576,16 +578,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): if self.active_history: old = self.get(state, dict_) else: - # this would be the "laziest" approach, - # however it breaks currently expected backref - # behavior - #old = dict_.get(self.key, None) - # instead, use the "passive" setting, which - # is only going to be PASSIVE_NOCALLABLES if it - # came from a backref - old = self.get(state, dict_, passive=passive) - if old is PASSIVE_NORESULT: - old = None + old = self.get(state, dict_, passive=PASSIVE_NO_FETCH) value = self.fire_replace_event(state, dict_, value, old, initiator) dict_[self.key] = value @@ -603,7 +596,7 @@ class ScalarObjectAttributeImpl(ScalarAttributeImpl): state.modified_event(dict_, self, False, previous) if self.trackparent: - if previous is not value and previous is not None: + if previous is not value and previous not in (None, PASSIVE_NO_RESULT): self.sethasparent(instance_state(previous), False) for ext in self.extensions: @@ -650,7 +643,7 @@ class CollectionAttributeImpl(AttributeImpl): def get_history(self, state, dict_, passive=PASSIVE_OFF): current = self.get(state, dict_, passive=passive) - if current is PASSIVE_NORESULT: + if current is PASSIVE_NO_RESULT: return HISTORY_BLANK else: return History.from_attribute(self, state, current) @@ -705,7 +698,7 @@ class CollectionAttributeImpl(AttributeImpl): return collection = self.get_collection(state, dict_, passive=passive) - if collection is PASSIVE_NORESULT: + if collection is PASSIVE_NO_RESULT: value = self.fire_append_event(state, dict_, value, initiator) state.get_pending(self.key).append(value) else: @@ -716,7 +709,7 @@ class CollectionAttributeImpl(AttributeImpl): return collection = self.get_collection(state, state.dict, passive=passive) - if collection is PASSIVE_NORESULT: + if collection is PASSIVE_NO_RESULT: self.fire_remove_event(state, dict_, value, initiator) state.get_pending(self.key).remove(value) else: @@ -810,7 +803,7 @@ class CollectionAttributeImpl(AttributeImpl): """ if user_data is None: user_data = self.get(state, dict_, passive=passive) - if user_data is PASSIVE_NORESULT: + if user_data is PASSIVE_NO_RESULT: return user_data return getattr(user_data, '_sa_adapter') @@ -832,29 +825,29 @@ class GenericBackrefExtension(interfaces.AttributeExtension): def set(self, state, child, oldchild, initiator): if oldchild is child: return child - if oldchild is not None: + if oldchild not in (None, PASSIVE_NO_RESULT): # 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), instance_dict(oldchild) impl = old_state.get_impl(self.key) try: - impl.remove(old_state, old_dict, state.obj(), initiator, passive=PASSIVE_NO_CALLABLES) + impl.remove(old_state, old_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH) except (ValueError, KeyError, IndexError): pass if child is not None: new_state, new_dict = instance_state(child), instance_dict(child) - new_state.get_impl(self.key).append(new_state, new_dict, state.obj(), initiator, passive=PASSIVE_NO_CALLABLES) + new_state.get_impl(self.key).append(new_state, new_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH) return child def append(self, state, child, initiator): child_state, child_dict = instance_state(child), instance_dict(child) - child_state.get_impl(self.key).append(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_CALLABLES) + child_state.get_impl(self.key).append(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH) return child def remove(self, state, child, initiator): if child is not None: child_state, child_dict = instance_state(child), instance_dict(child) - child_state.get_impl(self.key).remove(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_CALLABLES) + child_state.get_impl(self.key).remove(child_state, child_dict, state.obj(), initiator, passive=PASSIVE_NO_FETCH) class Events(object): @@ -1248,9 +1241,9 @@ class History(tuple): def as_state(self): return History( - [c is not None and instance_state(c) or None for c in self.added], - [c is not None and instance_state(c) or None for c in self.unchanged], - [c is not None and instance_state(c) or None for c in self.deleted], + [c not in (None, PASSIVE_NO_RESULT) and instance_state(c) or None for c in self.added], + [c not in (None, PASSIVE_NO_RESULT) and instance_state(c) or None for c in self.unchanged], + [c not in (None, PASSIVE_NO_RESULT) and instance_state(c) or None for c in self.deleted], ) @classmethod diff --git a/lib/sqlalchemy/orm/identity.py b/lib/sqlalchemy/orm/identity.py index b7d4234f4c..889b65ba7f 100644 --- a/lib/sqlalchemy/orm/identity.py +++ b/lib/sqlalchemy/orm/identity.py @@ -141,10 +141,15 @@ class WeakInstanceDict(IdentityMap): self._manage_removed_state(state) def get(self, key, default=None): - try: - return self[key] - except KeyError: + state = dict.get(self, key, default) + if state is default: + return default + o = state.obj() + if o is None: + o = state._is_really_none() + if o is None: return default + return o # Py2K def items(self): diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index c2c57825e3..177799602f 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1124,12 +1124,12 @@ class Mapper(object): if leftcol.table not in tables: leftval = self._get_committed_state_attr_by_column(state, leftcol, passive=True) - if leftval is attributes.PASSIVE_NORESULT: + if leftval is attributes.PASSIVE_NO_RESULT: raise ColumnsNotAvailable() binary.left = sql.bindparam(None, leftval, type_=binary.right.type) elif rightcol.table not in tables: rightval = self._get_committed_state_attr_by_column(state, rightcol, passive=True) - if rightval is attributes.PASSIVE_NORESULT: + if rightval is attributes.PASSIVE_NO_RESULT: raise ColumnsNotAvailable() binary.right = sql.bindparam(None, rightval, type_=binary.right.type) diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 21137bc28b..f09b594c0c 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -1447,21 +1447,24 @@ class Query(object): break iterate_instances = util.deprecated()(instances) - def _get(self, key=None, ident=None, refresh_state=None, lockmode=None, only_load_props=None): + def _get(self, key=None, ident=None, refresh_state=None, lockmode=None, only_load_props=None, passive=None): lockmode = lockmode or self._lockmode if not self._populate_existing and not refresh_state and not self._mapper_zero().always_refresh and lockmode is None: - try: - instance = self.session.identity_map[key] + instance = self.session.identity_map.get(key) + if instance: state = attributes.instance_state(instance) if state.expired: + if passive is attributes.PASSIVE_NO_FETCH: + return attributes.PASSIVE_NO_RESULT + try: state() except orm_exc.ObjectDeletedError: self.session._remove_newly_deleted(state) return None return instance - except KeyError: - pass + elif passive is attributes.PASSIVE_NO_FETCH: + return attributes.PASSIVE_NO_RESULT if ident is None: if key is not None: diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py index 4d9fa5ade8..f09c597639 100644 --- a/lib/sqlalchemy/orm/state.py +++ b/lib/sqlalchemy/orm/state.py @@ -1,7 +1,7 @@ from sqlalchemy.util import EMPTY_SET import weakref from sqlalchemy import util -from sqlalchemy.orm.attributes import PASSIVE_NORESULT, PASSIVE_OFF, NEVER_SET, NO_VALUE, manager_of_class, ATTR_WAS_SET +from sqlalchemy.orm.attributes import PASSIVE_NO_RESULT, PASSIVE_OFF, NEVER_SET, NO_VALUE, manager_of_class, ATTR_WAS_SET from sqlalchemy.orm import attributes from sqlalchemy.orm import interfaces @@ -108,13 +108,13 @@ class InstanceState(object): attribute. returns None if passive is not PASSIVE_OFF and the getter returns - PASSIVE_NORESULT. + PASSIVE_NO_RESULT. """ impl = self.get_impl(key) dict_ = self.dict x = impl.get(self, dict_, passive=passive) - if x is PASSIVE_NORESULT: + if x is PASSIVE_NO_RESULT: return None elif hasattr(impl, 'get_collection'): return impl.get_collection(self, dict_, x, passive=passive) @@ -176,12 +176,16 @@ class InstanceState(object): self.dict.pop(key, None) self.callables[key] = callable_ - def __call__(self): + def __call__(self, **kw): """__call__ allows the InstanceState to act as a deferred callable for loading expired attributes, which is also serializable (picklable). """ + + if kw.get('passive') is attributes.PASSIVE_NO_FETCH: + return attributes.PASSIVE_NO_RESULT + unmodified = self.unmodified class_manager = self.manager class_manager.deferred_scalar_loader(self, [ diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index e19e8fb31c..c0609dba3a 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -261,10 +261,12 @@ class LoadDeferredColumns(object): def __init__(self, state, key): self.state, self.key = state, key - def __call__(self): + def __call__(self, **kw): + if kw.get('passive') is attributes.PASSIVE_NO_FETCH: + return attributes.PASSIVE_NO_RESULT + state = self.state - localparent = mapper._state_mapper(state) prop = localparent.get_property(self.key) @@ -536,12 +538,14 @@ class LoadLazyAttribute(object): def __setstate__(self, state): self.state, self.key = state - def __call__(self): + def __call__(self, **kw): state = self.state - instance_mapper = mapper._state_mapper(state) prop = instance_mapper.get_property(self.key) strategy = prop._get_strategy(LazyLoader) + + if kw.get('passive') is attributes.PASSIVE_NO_FETCH and not strategy.use_get: + return attributes.PASSIVE_NO_RESULT if strategy._should_log_debug: strategy.logger.debug("loading %s" % mapperutil.state_attribute_str(state, self.key)) @@ -565,14 +569,21 @@ class LoadLazyAttribute(object): ident = [] allnulls = True for primary_key in prop.mapper.primary_key: - val = instance_mapper._get_committed_state_attr_by_column(state, strategy._equated_columns[primary_key]) + val = instance_mapper._get_committed_state_attr_by_column(state, strategy._equated_columns[primary_key], **kw) + if val is attributes.PASSIVE_NO_RESULT: + return val allnulls = allnulls and val is None ident.append(val) + if allnulls: return None + if state.load_options: q = q._conditional_options(*state.load_options) - return q.get(ident) + + key = prop.mapper.identity_key_from_primary_key(ident) + return q._get(key, ident, **kw) + if prop.order_by: q = q.order_by(*util.to_list(prop.order_by)) diff --git a/test/orm/test_attributes.py b/test/orm/test_attributes.py index fa26ec7d7e..b481b06791 100644 --- a/test/orm/test_attributes.py +++ b/test/orm/test_attributes.py @@ -55,7 +55,7 @@ class AttributesTest(_base.ORMTest): attributes.register_attribute(MyTest2, 'a', uselist=False, useobject=False) attributes.register_attribute(MyTest2, 'b', uselist=False, useobject=False) # shouldnt be pickling callables at the class level - def somecallable(*args): + def somecallable(*args, **kw): return None attributes.register_attribute(MyTest, "mt2", uselist = True, trackparent=True, callable_=somecallable, useobject=True) @@ -276,8 +276,8 @@ class AttributesTest(_base.ORMTest): # create objects as if they'd been freshly loaded from the database (without history) b = Blog() p1 = Post() - attributes.instance_state(b).set_callable('posts', lambda:[p1]) - attributes.instance_state(p1).set_callable('blog', lambda:b) + attributes.instance_state(b).set_callable('posts', lambda **kw:[p1]) + attributes.instance_state(p1).set_callable('blog', lambda **kw:b) p1, attributes.instance_state(b).commit_all(attributes.instance_dict(b)) # no orphans (called before the lazy loaders fire off) @@ -304,13 +304,13 @@ class AttributesTest(_base.ORMTest): attributes.register_class(Foo) attributes.register_class(Bar) - def func1(): + def func1(**kw): print "func1" return "this is the foo attr" - def func2(): + def func2(**kw): print "func2" return "this is the bar attr" - def func3(): + def func3(**kw): print "func3" return "this is the shared attr" attributes.register_attribute(Foo, 'element', uselist=False, callable_=lambda o:func1, useobject=True) @@ -377,9 +377,9 @@ class AttributesTest(_base.ORMTest): attributes.register_class(Bar) bar1, bar2, bar3, bar4 = [Bar(id=1), Bar(id=2), Bar(id=3), Bar(id=4)] - def func1(): + def func1(**kw): return "this is func 1" - def func2(): + def func2(**kw): return [bar1, bar2, bar3] attributes.register_attribute(Foo, 'col1', uselist=False, callable_=lambda o:func1, useobject=True) @@ -626,22 +626,25 @@ class PendingBackrefTest(_base.ORMTest): self.name = name __hash__ = None def __eq__(self, other): - return other.name == self.name + return other is not None and other.name == self.name class Blog(object): def __init__(self, name): self.name = name __hash__ = None def __eq__(self, other): - return other.name == self.name + return other is not None and other.name == self.name called = [0] lazy_load = [] def lazy_posts(instance): - def load(): - called[0] += 1 - return lazy_load + def load(**kw): + if kw['passive'] is not attributes.PASSIVE_NO_FETCH: + called[0] += 1 + return lazy_load + else: + return attributes.PASSIVE_NO_RESULT return load attributes.register_class(Post) @@ -1129,7 +1132,7 @@ class HistoryTest(_base.ORMTest): lazy_load = [] def lazyload(instance): - def load(): + def load(**kw): return lazy_load return load @@ -1164,7 +1167,7 @@ class HistoryTest(_base.ORMTest): lazy_load = [] def lazyload(instance): - def load(): + def load(**kw): return lazy_load return load @@ -1204,7 +1207,7 @@ class HistoryTest(_base.ORMTest): lazy_load = None def lazyload(instance): - def load(): + def load(**kw): return lazy_load return load @@ -1242,7 +1245,7 @@ class HistoryTest(_base.ORMTest): lazy_load = None def lazyload(instance): - def load(): + def load(**kw): return lazy_load return load @@ -1282,7 +1285,7 @@ class HistoryTest(_base.ORMTest): lazy_load = None def lazyload(instance): - def load(): + def load(**kw): return lazy_load return load diff --git a/test/orm/test_backref_mutations.py b/test/orm/test_backref_mutations.py index 1ecf0275a9..f0dfa254f5 100644 --- a/test/orm/test_backref_mutations.py +++ b/test/orm/test_backref_mutations.py @@ -131,14 +131,55 @@ class O2MCollectionTest(_fixtures.FixtureTest): # u1.addresses is loaded u1.addresses - # direct set - the fetching of the - # "old" u1 here allows the backref - # to remove it from the addresses collection + # direct set - the "old" is "fetched", + # but only from the local session - not the + # database, due to the PASSIVE_NO_FETCH flag. + # this is a more fine grained behavior introduced + # in 0.6 a1.user = u2 assert a1 not in u1.addresses assert a1 in u2.addresses + @testing.resolve_artifact_names + def test_plain_load_passive(self): + """test that many-to-one set doesn't load the old value.""" + + sess = sessionmaker()() + u1 = User(name='jack') + u2 = User(name='ed') + a1 = Address(email_address='a1') + a1.user = u1 + sess.add_all([u1, u2, a1]) + sess.commit() + + # in this case, a lazyload would + # ordinarily occur except for the + # PASSIVE_NO_FETCH flag. + def go(): + a1.user = u2 + self.assert_sql_count(testing.db, go, 0) + + assert a1 not in u1.addresses + assert a1 in u2.addresses + + @testing.resolve_artifact_names + def test_set_none(self): + sess = sessionmaker()() + u1 = User(name='jack') + a1 = Address(email_address='a1') + a1.user = u1 + sess.add_all([u1, a1]) + sess.commit() + + # works for None too + def go(): + a1.user = None + self.assert_sql_count(testing.db, go, 0) + + assert a1 not in u1.addresses + + @testing.resolve_artifact_names def test_scalar_move_notloaded(self): diff --git a/test/orm/test_dynamic.py b/test/orm/test_dynamic.py index 23a5fc8762..9f5ccf51f6 100644 --- a/test/orm/test_dynamic.py +++ b/test/orm/test_dynamic.py @@ -76,7 +76,7 @@ class DynamicTest(_fixtures.FixtureTest): ad = sess.query(Address).get(1) def go(): ad.user = None - self.assert_sql_count(testing.db, go, 1) + self.assert_sql_count(testing.db, go, 0) sess.flush() u = sess.query(User).get(7) assert ad not in u.addresses diff --git a/test/orm/test_extendedattr.py b/test/orm/test_extendedattr.py index e0c64bf64a..3e3ee85607 100644 --- a/test/orm/test_extendedattr.py +++ b/test/orm/test_extendedattr.py @@ -197,13 +197,13 @@ class UserDefinedExtensionTest(_base.ORMTest): attributes.register_class(Foo) attributes.register_class(Bar) - def func1(): + def func1(**kw): print "func1" return "this is the foo attr" - def func2(): + def func2(**kw): print "func2" return "this is the bar attr" - def func3(): + def func3(**kw): print "func3" return "this is the shared attr" attributes.register_attribute(Foo, 'element', uselist=False, callable_=lambda o:func1, useobject=True) -- 2.47.3