From: Mike Bayer Date: Mon, 3 Oct 2016 19:55:04 +0000 (-0400) Subject: Enhance "raise" strategy to include "raise_on_sql" option X-Git-Tag: rel_1_1_0~5^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=95d4cd30420414fcede2662aed87b0f2e5a861d4;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Enhance "raise" strategy to include "raise_on_sql" option The "raise_on_sql" option differentiates from "raise" in that firing a lazy loader is OK as long as it does a simple get from identity map. Whereas "raise" is more useful for the case that objects are to be detached. As part of this, refactors the strategy initiation logic a bit so that a LoaderStrategy itself knows what "key" was used to create it, thus allowing variants of a single strategy based on what the "lazy" argument is. To achieve this we have to also get rid of _get_strategy_by_cls(). Everything here is internal with the one exception of an apparently undocumented, but not underscored, "strategy_class" key on relationship(). Though it's not clear what "strategy_class" accomplishes; at this point the strategy system is extensible using Property.strategy_for(). Fixes: #3812 Change-Id: I812ad878ea5cf764e15f6f71cb39eee78a645d88 --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index a3cc96f992..6e2fc014ce 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,16 @@ .. changelog:: :version: 1.1.0 + .. change:: + :tags: feature, orm + :tickets: 3812 + + Enhanced the new "raise" lazy loader strategy to also include a + "raise_on_sql" variant, available both via :paramref:`.orm.relationship.lazy` + as well as :func:`.orm.raiseload`. This variant only raises if the + lazy load would actually emit SQL, vs. raising if the lazy loader + mechanism is invoked at all. + .. change:: :tags: bug, orm :tickets: 3808 diff --git a/doc/build/changelog/migration_11.rst b/doc/build/changelog/migration_11.rst index e514f4dbbd..7f8ead1436 100644 --- a/doc/build/changelog/migration_11.rst +++ b/doc/build/changelog/migration_11.rst @@ -1019,21 +1019,33 @@ added to the :ref:`mutable_toplevel` extension, to complement the existing .. _change_3512: -New "raise" loader strategy ---------------------------- +New "raise" / "raise_on_sql" loader strategies +---------------------------------------------- To assist with the use case of preventing unwanted lazy loads from occurring -after a series of objects are loaded, the new "lazy='raise'" strategy and +after a series of objects are loaded, the new "lazy='raise'" and +"lazy='raise_on_sql'" strategies and corresponding loader option :func:`.orm.raiseload` may be applied to a relationship attribute which will cause it to raise ``InvalidRequestError`` -when a non-eagerly-loaded attribute is accessed for read:: +when a non-eagerly-loaded attribute is accessed for read. The two variants +test for either a lazy load of any variety, including those that would +only return None or retrieve from the identity map:: + + >>> from sqlalchemy.orm import raiseload + >>> a1 = s.query(A).options(raiseload(A.some_b)).first() + >>> a1.some_b + Traceback (most recent call last): + ... + sqlalchemy.exc.InvalidRequestError: 'A.some_b' is not available due to lazy='raise' + +Or a lazy load only where SQL would be emitted:: >>> from sqlalchemy.orm import raiseload - >>> a1 = s.query(A).options(raiseload(A.bs)).first() - >>> a1.bs + >>> a1 = s.query(A).options(raiseload(A.some_b, sql_only=True)).first() + >>> a1.some_b Traceback (most recent call last): ... - sqlalchemy.exc.InvalidRequestError: 'A.bs' is not available due to lazy='raise' + sqlalchemy.exc.InvalidRequestError: 'A.bs' is not available due to lazy='raise_on_sql' :ticket:`3512` diff --git a/lib/sqlalchemy/orm/descriptor_props.py b/lib/sqlalchemy/orm/descriptor_props.py index 6c87ef9ba9..9b94af6d17 100644 --- a/lib/sqlalchemy/orm/descriptor_props.py +++ b/lib/sqlalchemy/orm/descriptor_props.py @@ -269,7 +269,7 @@ class CompositeProperty(DescriptorProperty): prop.active_history = self.active_history if self.deferred: prop.deferred = self.deferred - prop.strategy_class = prop._strategy_lookup( + prop.strategy_key = ( ("deferred", True), ("instrument", True)) prop.group = self.group diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py index faab70e37a..152c21d0bb 100644 --- a/lib/sqlalchemy/orm/interfaces.py +++ b/lib/sqlalchemy/orm/interfaces.py @@ -489,12 +489,9 @@ class StrategizedProperty(MapperProperty): except KeyError: cls = self._strategy_lookup(*key) self._strategies[key] = self._strategies[ - cls] = strategy = cls(self) + cls] = strategy = cls(self, key) return strategy - def _get_strategy_by_cls(self, cls): - return self._get_strategy(cls._strategy_keys[0]) - def setup( self, context, entity, path, adapter, **kwargs): loader = self._get_context_loader(context, path) @@ -518,7 +515,7 @@ class StrategizedProperty(MapperProperty): def do_init(self): self._strategies = {} - self.strategy = self._get_strategy_by_cls(self.strategy_class) + self.strategy = self._get_strategy(self.strategy_key) def post_instrument_class(self, mapper): if not self.parent.non_primary and \ @@ -603,13 +600,16 @@ class LoaderStrategy(object): """ - __slots__ = 'parent_property', 'is_class_level', 'parent', 'key' + __slots__ = 'parent_property', 'is_class_level', 'parent', 'key', \ + 'strategy_key', 'strategy_opts' - def __init__(self, parent): + def __init__(self, parent, strategy_key): self.parent_property = parent self.is_class_level = False self.parent = self.parent_property.parent self.key = self.parent_property.key + self.strategy_key = strategy_key + self.strategy_opts = dict(strategy_key) def init_class_attribute(self, mapper): pass diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py index f3dce75419..17c8177dd2 100644 --- a/lib/sqlalchemy/orm/properties.py +++ b/lib/sqlalchemy/orm/properties.py @@ -38,7 +38,7 @@ class ColumnProperty(StrategizedProperty): '_orig_columns', 'columns', 'group', 'deferred', 'instrument', 'comparator_factory', 'descriptor', 'extension', 'active_history', 'expire_on_flush', 'info', 'doc', - 'strategy_class', '_creation_order', '_is_polymorphic_discriminator', + 'strategy_key', '_creation_order', '_is_polymorphic_discriminator', '_mapped_by_synonym', '_deferred_column_loader') def __init__(self, *columns, **kwargs): @@ -152,7 +152,7 @@ class ColumnProperty(StrategizedProperty): util.set_creation_order(self) - self.strategy_class = self._strategy_lookup( + self.strategy_key = ( ("deferred", self.deferred), ("instrument", self.instrument) ) diff --git a/lib/sqlalchemy/orm/relationships.py b/lib/sqlalchemy/orm/relationships.py index e8a2992b06..b30a74402e 100644 --- a/lib/sqlalchemy/orm/relationships.py +++ b/lib/sqlalchemy/orm/relationships.py @@ -114,7 +114,7 @@ class RelationshipProperty(StrategizedProperty): cascade_backrefs=True, load_on_pending=False, bake_queries=True, - strategy_class=None, _local_remote_pairs=None, + _local_remote_pairs=None, query_class=None, info=None): """Provide a relationship between two mapped classes. @@ -543,6 +543,20 @@ class RelationshipProperty(StrategizedProperty): * ``raise`` - lazy loading is disallowed; accessing the attribute, if its value were not already loaded via eager loading, will raise an :exc:`~sqlalchemy.exc.InvalidRequestError`. + This strategy can be used when objects are to be detached from + their attached :class:`.Session` after they are loaded. + + .. versionadded:: 1.1 + + * ``raise_on_sql`` - lazy loading that emits SQL is disallowed; + accessing the attribute, if its value were not already loaded via + eager loading, will raise an + :exc:`~sqlalchemy.exc.InvalidRequestError`, **if the lazy load + needs to emit SQL**. If the lazy load can pull the related value + from the identity map or determine that it should be None, the + value is loaded. This strategy can be used when objects will + remain associated with the attached :class:`.Session`, however + additional SELECT statements should be blocked. .. versionadded:: 1.1 @@ -842,10 +856,7 @@ class RelationshipProperty(StrategizedProperty): if info is not None: self.info = info - if strategy_class: - self.strategy_class = strategy_class - else: - self.strategy_class = self._strategy_lookup(("lazy", self.lazy)) + self.strategy_key = (("lazy", self.lazy), ) self._reverse_property = set() diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py index 202b652b72..108e3496c3 100644 --- a/lib/sqlalchemy/orm/strategies.py +++ b/lib/sqlalchemy/orm/strategies.py @@ -120,8 +120,8 @@ class UninstrumentedColumnLoader(LoaderStrategy): """ __slots__ = 'columns', - def __init__(self, parent): - super(UninstrumentedColumnLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(UninstrumentedColumnLoader, self).__init__(parent, strategy_key) self.columns = self.parent_property.columns def setup_query( @@ -145,8 +145,8 @@ class ColumnLoader(LoaderStrategy): __slots__ = 'columns', 'is_composite' - def __init__(self, parent): - super(ColumnLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(ColumnLoader, self).__init__(parent, strategy_key) self.columns = self.parent_property.columns self.is_composite = hasattr(self.parent_property, 'composite_class') @@ -201,8 +201,8 @@ class DeferredColumnLoader(LoaderStrategy): __slots__ = 'columns', 'group' - def __init__(self, parent): - super(DeferredColumnLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(DeferredColumnLoader, self).__init__(parent, strategy_key) if hasattr(self.parent_property, 'composite_class'): raise NotImplementedError("Deferred loading for composite " "types not implemented yet") @@ -257,10 +257,12 @@ class DeferredColumnLoader(LoaderStrategy): only_load_props and self.key in only_load_props ) ): - self.parent_property._get_strategy_by_cls(ColumnLoader).\ - setup_query(context, entity, - path, loadopt, adapter, - column_collection, memoized_populators, **kw) + self.parent_property._get_strategy( + (("deferred", False), ("instrument", True)) + ).setup_query( + context, entity, + path, loadopt, adapter, + column_collection, memoized_populators, **kw) elif self.is_class_level: memoized_populators[self.parent_property] = _SET_DEFERRED_EXPIRED else: @@ -326,8 +328,8 @@ class AbstractRelationshipLoader(LoaderStrategy): __slots__ = 'mapper', 'target', 'uselist' - def __init__(self, parent): - super(AbstractRelationshipLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(AbstractRelationshipLoader, self).__init__(parent, strategy_key) self.mapper = self.parent_property.mapper self.target = self.parent_property.target self.uselist = self.parent_property.uselist @@ -365,36 +367,11 @@ class NoLoader(AbstractRelationshipLoader): populators["new"].append((self.key, invoke_no_load)) -@log.class_logger -@properties.RelationshipProperty.strategy_for(lazy="raise") -class RaiseLoader(NoLoader): - """Provide loading behavior for a :class:`.RelationshipProperty` - with "lazy='raise'". - - """ - - __slots__ = () - - def create_row_processor( - self, context, path, loadopt, mapper, - result, adapter, populators): - - def invoke_raise_load(state, passive): - raise sa_exc.InvalidRequestError( - "'%s' is not available due to lazy='raise'" % self - ) - - set_lazy_callable = InstanceState._instance_level_callable_processor( - mapper.class_manager, - invoke_raise_load, - self.key - ) - populators["new"].append((self.key, set_lazy_callable)) - - @log.class_logger @properties.RelationshipProperty.strategy_for(lazy=True) @properties.RelationshipProperty.strategy_for(lazy="select") +@properties.RelationshipProperty.strategy_for(lazy="raise") +@properties.RelationshipProperty.strategy_for(lazy="raise_on_sql") class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): """Provide loading behavior for a :class:`.RelationshipProperty` with "lazy=True", that is loads when first accessed. @@ -404,10 +381,13 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): __slots__ = ( '_lazywhere', '_rev_lazywhere', 'use_get', '_bind_to_col', '_equated_columns', '_rev_bind_to_col', '_rev_equated_columns', - '_simple_lazy_clause') + '_simple_lazy_clause', '_raise_always', '_raise_on_sql') + + def __init__(self, parent, strategy_key): + super(LazyLoader, self).__init__(parent, strategy_key) + self._raise_always = self.strategy_opts["lazy"] == "raise" + self._raise_on_sql = self.strategy_opts["lazy"] == "raise_on_sql" - def __init__(self, parent): - super(LazyLoader, self).__init__(parent) join_condition = self.parent_property._join_condition self._lazywhere, \ self._bind_to_col, \ @@ -516,7 +496,13 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): return criterion, params + def _invoke_raise_load(self, state, passive, lazy): + raise sa_exc.InvalidRequestError( + "'%s' is not available due to lazy='%s'" % (self, lazy) + ) + def _load_for_state(self, state, passive): + if not state.key and ( ( not self.parent_property.load_on_pending @@ -536,6 +522,9 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): ): return attributes.PASSIVE_NO_RESULT + if self._raise_always: + self._invoke_raise_load(state, passive, "raise") + session = _state_session(state) if not session: raise orm_exc.DetachedInstanceError( @@ -612,6 +601,8 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): q = q._conditional_options(*state.load_options) if self.use_get: + if self._raise_on_sql: + self._invoke_raise_load(state, passive, "raise_on_sql") return loading.load_on_ident(q, ident_key) if self.parent_property.order_by: @@ -636,6 +627,9 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): elif util.has_intersection(orm_util._never_set, params.values()): return None + if self._raise_on_sql: + self._invoke_raise_load(state, passive, "raise_on_sql") + q = q.filter(lazy_clause).params(params) result = q.all() @@ -669,7 +663,7 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): # class-level lazyloader installed. set_lazy_callable = InstanceState._instance_level_callable_processor( mapper.class_manager, - LoadLazyAttribute(key, self._strategy_keys[0]), key) + LoadLazyAttribute(key, self), key) populators["new"].append((self.key, set_lazy_callable)) elif context.populate_existing or mapper.always_refresh: @@ -690,9 +684,9 @@ class LazyLoader(AbstractRelationshipLoader, util.MemoizedSlots): class LoadLazyAttribute(object): """serializable loader object used by LazyLoader""" - def __init__(self, key, strategy_key=(('lazy', 'select'),)): + def __init__(self, key, initiating_strategy): self.key = key - self.strategy_key = strategy_key + self.strategy_key = initiating_strategy.strategy_key def __call__(self, state, passive=attributes.PASSIVE_OFF): key = self.key @@ -709,7 +703,7 @@ class ImmediateLoader(AbstractRelationshipLoader): def init_class_attribute(self, mapper): self.parent_property.\ - _get_strategy_by_cls(LazyLoader).\ + _get_strategy((("lazy", "select"),)).\ init_class_attribute(mapper) def setup_query( @@ -732,13 +726,13 @@ class ImmediateLoader(AbstractRelationshipLoader): class SubqueryLoader(AbstractRelationshipLoader): __slots__ = 'join_depth', - def __init__(self, parent): - super(SubqueryLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(SubqueryLoader, self).__init__(parent, strategy_key) self.join_depth = self.parent_property.join_depth def init_class_attribute(self, mapper): self.parent_property.\ - _get_strategy_by_cls(LazyLoader).\ + _get_strategy((("lazy", "select"),)).\ init_class_attribute(mapper) def setup_query( @@ -1130,13 +1124,13 @@ class JoinedLoader(AbstractRelationshipLoader): __slots__ = 'join_depth', - def __init__(self, parent): - super(JoinedLoader, self).__init__(parent) + def __init__(self, parent, strategy_key): + super(JoinedLoader, self).__init__(parent, strategy_key) self.join_depth = self.parent_property.join_depth def init_class_attribute(self, mapper): self.parent_property.\ - _get_strategy_by_cls(LazyLoader).init_class_attribute(mapper) + _get_strategy((("lazy", "select"),)).init_class_attribute(mapper) def setup_query( self, context, entity, path, loadopt, adapter, @@ -1562,7 +1556,7 @@ class JoinedLoader(AbstractRelationshipLoader): self._create_collection_loader( context, key, _instance, populators) else: - self.parent_property._get_strategy_by_cls(LazyLoader).\ + self.parent_property._get_strategy((("lazy", "select"),)).\ create_row_processor( context, path, loadopt, mapper, result, adapter, populators) diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py index 97d2c0f298..2fb13f3cfa 100644 --- a/lib/sqlalchemy/orm/strategy_options.py +++ b/lib/sqlalchemy/orm/strategy_options.py @@ -879,7 +879,7 @@ def noload(*keys): @loader_option() -def raiseload(loadopt, attr): +def raiseload(loadopt, attr, sql_only=False): """Indicate that the given relationship attribute should disallow lazy loads. A relationship attribute configured with :func:`.orm.raiseload` will @@ -890,6 +890,11 @@ def raiseload(loadopt, attr): to read through SQL logs to ensure lazy loads aren't occurring, this strategy will cause them to raise immediately. + :param sql_only: if True, raise only if the lazy load would emit SQL, + but not if it is only checking the identity map, or determining that + the related value should just be None due to missing keys. When False, + the strategy will raise for all varieties of lazyload. + This function is part of the :class:`.Load` interface and supports both method-chained and standalone operation. @@ -899,12 +904,13 @@ def raiseload(loadopt, attr): """ - return loadopt.set_relationship_strategy(attr, {"lazy": "raise"}) + return loadopt.set_relationship_strategy( + attr, {"lazy": "raise_on_sql" if sql_only else "raise"}) @raiseload._add_unbound_fn -def raiseload(*keys): - return _UnboundLoad._from_keys(_UnboundLoad.raiseload, keys, False, {}) +def raiseload(*keys, **kw): + return _UnboundLoad._from_keys(_UnboundLoad.raiseload, keys, False, kw) @loader_option() diff --git a/test/ext/test_baked.py b/test/ext/test_baked.py index 4250e363b1..2f501eb6c0 100644 --- a/test/ext/test_baked.py +++ b/test/ext/test_baked.py @@ -576,7 +576,7 @@ class ResultTest(BakedTest): Address(id=4, email_address='ed@lala.com'), ]), User(id=9, - addresses=[Address(id=5)], + addresses=[Address(id=5)], orders=[Order(id=2), Order(id=4)]), User(id=10, addresses=[]) ] @@ -728,7 +728,6 @@ class LazyLoaderTest(BakedTest): def test_systemwide_loaders_loadable_via_lazyloader(self): from sqlalchemy.orm import configure_mappers - from sqlalchemy.orm.strategies import LazyLoader baked.bake_lazy_loaders() try: @@ -738,7 +737,7 @@ class LazyLoaderTest(BakedTest): is_( User.addresses.property. - _get_strategy_by_cls(LazyLoader).__class__, + _get_strategy((('lazy', 'select'), )).__class__, BakedLazyLoader ) finally: diff --git a/test/orm/test_mapper.py b/test/orm/test_mapper.py index 045016bb35..8c41711258 100644 --- a/test/orm/test_mapper.py +++ b/test/orm/test_mapper.py @@ -2874,6 +2874,63 @@ class RaiseLoadTest(_fixtures.FixtureTest): self.sql_count_(0, go) + def test_m2o_raise_on_sql_option(self): + Address, addresses, users, User = ( + self.classes.Address, + self.tables.addresses, + self.tables.users, + self.classes.User) + mapper(Address, addresses, properties={ + 'user': relationship(User) + }) + mapper(User, users) + s = Session() + a1 = s.query(Address).filter_by(id=1).options( + sa.orm.raiseload('user', sql_only=True)).first() + + def go(): + assert_raises_message( + sa.exc.InvalidRequestError, + "'Address.user' is not available due to lazy='raise_on_sql'", + lambda: a1.user) + + self.sql_count_(0, go) + + s.close() + + u1 = s.query(User).first() + a1 = s.query(Address).filter_by(id=1).options( + sa.orm.raiseload('user', sql_only=True)).first() + assert 'user' not in a1.__dict__ + is_(a1.user, u1) + + def test_m2o_non_use_get_raise_on_sql_option(self): + Address, addresses, users, User = ( + self.classes.Address, + self.tables.addresses, + self.tables.users, + self.classes.User) + mapper(Address, addresses, properties={ + 'user': relationship( + User, + primaryjoin=sa.and_( + addresses.c.user_id == users.c.id , + users.c.name != None + ) + ) + }) + mapper(User, users) + s = Session() + u1 = s.query(User).first() + a1 = s.query(Address).filter_by(id=1).options( + sa.orm.raiseload('user', sql_only=True)).first() + + def go(): + assert_raises_message( + sa.exc.InvalidRequestError, + "'Address.user' is not available due to lazy='raise_on_sql'", + lambda: a1.user) + class RequirementsTest(fixtures.MappedTest):