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

10 files changed, 171 insertions, 82 deletions

diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst
index a3cc96f99..6e2fc014c 100644
--- a/doc/build/changelog/changelog_11.rst
+++ b/doc/build/changelog/changelog_11.rst
@@ -22,6 +22,16 @@
     :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 e514f4dbb..7f8ead143 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 6c87ef9ba..9b94af6d1 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 faab70e37..152c21d0b 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 f3dce7541..17c8177dd 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 e8a2992b0..b30a74402 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 202b652b7..108e3496c 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
@@ -366,35 +368,10 @@ class NoLoader(AbstractRelationshipLoader):
 
 
 @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 97d2c0f29..2fb13f3cf 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 4250e363b..2f501eb6c 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 045016bb3..8c4171125 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):