From 6c12838f9a7a48fccb774afef87e4300e1c5e529 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 2 Oct 2009 22:23:30 +0000 Subject: [PATCH] - query.options() now only propagate to loaded objects for potential further sub-loads only for options where such behavior is relevant, keeping various unserializable options like those generated by contains_eager() out of individual instance states. [ticket:1553] --- CHANGES | 7 +++++++ lib/sqlalchemy/orm/__init__.py | 2 +- lib/sqlalchemy/orm/interfaces.py | 7 ++++++- lib/sqlalchemy/orm/mapper.py | 10 ++-------- lib/sqlalchemy/orm/query.py | 1 + lib/sqlalchemy/orm/strategies.py | 18 ++++++++---------- test/orm/test_mapper.py | 24 +++++++++++++++++++++++- 7 files changed, 48 insertions(+), 21 deletions(-) diff --git a/CHANGES b/CHANGES index 26b8130d16..c3380c0741 100644 --- a/CHANGES +++ b/CHANGES @@ -397,6 +397,13 @@ CHANGES subclass table to the query separately producing a cartesian product. An example is in the ticket description. [ticket:1543] + + - query.options() now only propagate to loaded objects + for potential further sub-loads only for options where + such behavior is relevant, keeping + various unserializable options like those generated + by contains_eager() out of individual instance states. + [ticket:1553] 0.5.6 ===== diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py index 6a3c086bf6..713ade3e5a 100644 --- a/lib/sqlalchemy/orm/__init__.py +++ b/lib/sqlalchemy/orm/__init__.py @@ -943,7 +943,7 @@ def contains_eager(*keys, **kwargs): if kwargs: raise exceptions.ArgumentError("Invalid kwargs for contains_eager: %r" % kwargs.keys()) - return (strategies.EagerLazyOption(keys, lazy=False, _only_on_lead=True), strategies.LoadEagerFromAliasOption(keys, alias=alias)) + return (strategies.EagerLazyOption(keys, lazy=False, propagate_to_loaders=False), strategies.LoadEagerFromAliasOption(keys, alias=alias)) @sa_util.accepts_a_list_as_starargs(list_deprecation='pending') def defer(*keys): diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index dace1978e4..308f92780a 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -630,6 +630,11 @@ def deserialize_path(path): class MapperOption(object): """Describe a modification to a Query.""" + propagate_to_loaders = False + """if True, indicate this option should be carried along + Query object generated by scalar or object lazy loaders. + """ + def process_query(self, query): pass @@ -643,7 +648,7 @@ class MapperOption(object): class ExtensionOption(MapperOption): """a MapperOption that applies a MapperExtension to a query operation.""" - + def __init__(self, ext): self.ext = ext diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py index 3512c0b6b7..13151f2321 100644 --- a/lib/sqlalchemy/orm/mapper.py +++ b/lib/sqlalchemy/orm/mapper.py @@ -1590,14 +1590,8 @@ class Mapper(object): def populate_state(state, dict_, row, isnew, only_load_props, **flags): if isnew: - if context.options: - state.load_options = context.options - if state.load_options: - state.load_path = context.query._current_path + path - - if isnew: - if context.options: - state.load_options = context.options + if context.propagate_options: + state.load_options = context.propagate_options if state.load_options: state.load_path = context.query._current_path + path diff --git a/lib/sqlalchemy/orm/query.py b/lib/sqlalchemy/orm/query.py index 2ffa677cb7..b1b85cd01c 100644 --- a/lib/sqlalchemy/orm/query.py +++ b/lib/sqlalchemy/orm/query.py @@ -2186,6 +2186,7 @@ class QueryContext(object): self.adapter = None self.options = set(query._with_options) + self.propagate_options = self.options.difference(o for o in self.options if not o.propagate_to_loaders) self.attributes = query._attributes.copy() class AliasOption(interfaces.MapperOption): diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index e9d2c399c0..5192417439 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -306,6 +306,8 @@ class LoadDeferredColumns(object): return attributes.ATTR_WAS_SET class DeferredOption(StrategizedOption): + propagate_to_loaders = True + def __init__(self, key, defer=False): super(DeferredOption, self).__init__(key) self.defer = defer @@ -317,6 +319,8 @@ class DeferredOption(StrategizedOption): return ColumnLoader class UndeferGroupOption(MapperOption): + propagate_to_loaders = True + def __init__(self, group): self.group = group def process_query(self, query): @@ -817,16 +821,13 @@ class EagerLoader(AbstractRelationLoader): log.class_logger(EagerLoader) class EagerLazyOption(StrategizedOption): - def __init__(self, key, lazy=True, chained=False, mapper=None, _only_on_lead=False): + + def __init__(self, key, lazy=True, chained=False, mapper=None, propagate_to_loaders=True): super(EagerLazyOption, self).__init__(key, mapper) self.lazy = lazy self.chained = chained - self._only_on_lead = _only_on_lead + self.propagate_to_loaders = propagate_to_loaders - def process_query_conditionally(self, query): - if not self._only_on_lead: - StrategizedOption.process_query_conditionally(self, query) - def is_chained(self): return not self.lazy and self.chained @@ -839,6 +840,7 @@ class EagerLazyOption(StrategizedOption): return NoLoader class LoadEagerFromAliasOption(PropertyOption): + def __init__(self, key, alias=None): super(LoadEagerFromAliasOption, self).__init__(key) if alias is not None: @@ -846,10 +848,6 @@ class LoadEagerFromAliasOption(PropertyOption): m, alias, is_aliased_class = mapperutil._entity_info(alias) self.alias = alias - def process_query_conditionally(self, query): - # dont run this option on a secondary load - pass - def process_query_property(self, query, paths, mappers): if self.alias is not None: if isinstance(self.alias, basestring): diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 146baebd24..79286fe2a9 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -1119,7 +1119,6 @@ class OptionsTest(_fixtures.FixtureTest): eq_(l, self.static.user_address_result) self.sql_count_(4, go) - @testing.resolve_artifact_names def test_eager_degrade_deep(self): # test with a deeper set of eager loads. when we first load the three @@ -1179,6 +1178,29 @@ class OptionsTest(_fixtures.FixtureTest): eq_(l, self.static.user_address_result) self.sql_count_(4, go) + @testing.resolve_artifact_names + def test_option_propagate(self): + mapper(User, users, properties=dict( + orders = relation(Order) + )) + mapper(Order, orders, properties=dict( + items = relation(Item, secondary=order_items) + )) + mapper(Item, items) + + sess = create_session() + + oalias = aliased(Order) + opt1 = sa.orm.eagerload(User.orders, Order.items) + opt2a, opt2b = sa.orm.contains_eager(User.orders, Order.items, alias=oalias) + u1 = sess.query(User).join((oalias, User.orders)).options(opt1, opt2a, opt2b).first() + ustate = attributes.instance_state(u1) + assert opt1 in ustate.load_options + assert opt2a not in ustate.load_options + assert opt2b not in ustate.load_options + + import pickle + pickle.dumps(u1) class DeepOptionsTest(_fixtures.FixtureTest): @classmethod -- 2.47.2