Implement raiseload for deferred columns

Added "raiseload" feature for ORM mapped columns.

As part of this change, the behavior of "deferred" is now more strict;
an attribute that is set up as "deferred" at the mapper level no longer
participates in an "unexpire" operation; that is, when an unexpire loads
all the expired columns of an object which are not themselves in a deferred
group, those which are mapper-level deferred will never be loaded.

Deferral options set at query time should always be reset by an expiration
operation.

Renames deferred_scalar_loader to expired_attribute_loader

Unfortunately we can't have raiseload() do this because it would break
existing wildcard behavior.

Fixes: #4826
Change-Id: I30d9a30236e0b69134e4094fb7c1ad2267f089d1

12 files changed, 156 insertions, 48 deletions

diff --git a/lib/sqlalchemy/orm/__init__.py b/lib/sqlalchemy/orm/__init__.py
index 8bd68b417..e2eb93409 100644
--- a/lib/sqlalchemy/orm/__init__.py
+++ b/lib/sqlalchemy/orm/__init__.py
@@ -154,6 +154,15 @@ def deferred(*columns, **kw):
      :class:`.Column` object, however a collection is supported in order
      to support multiple columns mapped under the same attribute.
 
+    :param raiseload: boolean, if True, indicates an exception should be raised
+     if the load operation is to take place.
+
+     .. versionadded:: 1.4
+
+     .. seealso::
+
+        :ref:`deferred_raiseload`
+
     :param \**kw: additional keyword arguments passed to
      :class:`.ColumnProperty`.
 
diff --git a/lib/sqlalchemy/orm/attributes.py b/lib/sqlalchemy/orm/attributes.py
index 1466f5f47..83069f113 100644
--- a/lib/sqlalchemy/orm/attributes.py
+++ b/lib/sqlalchemy/orm/attributes.py
@@ -468,7 +468,7 @@ class AttributeImpl(object):
         compare_function=None,
         active_history=False,
         parent_token=None,
-        expire_missing=True,
+        load_on_unexpire=True,
         send_modified_events=True,
         accepts_scalar_loader=None,
         **kwargs
@@ -503,10 +503,13 @@ class AttributeImpl(object):
           Allows multiple AttributeImpls to all match a single
           owner attribute.
 
-        :param expire_missing:
-          if False, don't add an "expiry" callable to this attribute
-          during state.expire_attributes(None), if no value is present
-          for this key.
+        :param load_on_unexpire:
+          if False, don't include this attribute in a load-on-expired
+          operation, i.e. the "expired_attribute_loader" process.
+          The attribute can still be in the "expired" list and be
+          considered to be "expired".   Previously, this flag was called
+          "expire_missing" and is only used by a deferred column
+          attribute.
 
         :param send_modified_events:
           if False, the InstanceState._modified_event method will have no
@@ -534,7 +537,7 @@ class AttributeImpl(object):
         if active_history:
             self.dispatch._active_history = True
 
-        self.expire_missing = expire_missing
+        self.load_on_unexpire = load_on_unexpire
         self._modified_token = Event(self, OP_MODIFIED)
 
     __slots__ = (
@@ -546,7 +549,7 @@ class AttributeImpl(object):
         "parent_token",
         "send_modified_events",
         "is_equal",
-        "expire_missing",
+        "load_on_unexpire",
         "_modified_token",
         "accepts_scalar_loader",
     )
@@ -683,6 +686,7 @@ class AttributeImpl(object):
 
                 if (
                     self.accepts_scalar_loader
+                    and self.load_on_unexpire
                     and key in state.expired_attributes
                 ):
                     value = state._load_expired(state, passive)
diff --git a/lib/sqlalchemy/orm/base.py b/lib/sqlalchemy/orm/base.py
index e52b6d8bb..6f8d19293 100644
--- a/lib/sqlalchemy/orm/base.py
+++ b/lib/sqlalchemy/orm/base.py
@@ -206,6 +206,8 @@ _SET_DEFERRED_EXPIRED = util.symbol("SET_DEFERRED_EXPIRED")
 
 _DEFER_FOR_STATE = util.symbol("DEFER_FOR_STATE")
 
+_RAISE_FOR_STATE = util.symbol("RAISE_FOR_STATE")
+
 
 def _assertions(*assertions):
     @util.decorator
diff --git a/lib/sqlalchemy/orm/descriptor_props.py b/lib/sqlalchemy/orm/descriptor_props.py
index dd482bf06..3be5502ce 100644
--- a/lib/sqlalchemy/orm/descriptor_props.py
+++ b/lib/sqlalchemy/orm/descriptor_props.py
@@ -36,7 +36,7 @@ class DescriptorProperty(MapperProperty):
 
         class _ProxyImpl(object):
             accepts_scalar_loader = False
-            expire_missing = True
+            load_on_unexpire = True
             collection = False
 
             @property
diff --git a/lib/sqlalchemy/orm/instrumentation.py b/lib/sqlalchemy/orm/instrumentation.py
index 61184ee0a..ecb8d7857 100644
--- a/lib/sqlalchemy/orm/instrumentation.py
+++ b/lib/sqlalchemy/orm/instrumentation.py
@@ -49,12 +49,31 @@ class ClassManager(dict):
 
     _state_setter = staticmethod(util.attrsetter(STATE_ATTR))
 
-    deferred_scalar_loader = None
+    expired_attribute_loader = None
+    "previously known as deferred_scalar_loader"
 
     original_init = object.__init__
 
     factory = None
 
+    @property
+    @util.deprecated(
+        "1.4",
+        message="The ClassManager.deferred_scalar_loader attribute is now "
+        "named expired_attribute_loader",
+    )
+    def deferred_scalar_loader(self):
+        return self.expired_attribute_loader
+
+    @deferred_scalar_loader.setter
+    @util.deprecated(
+        "1.4",
+        message="The ClassManager.deferred_scalar_loader attribute is now "
+        "named expired_attribute_loader",
+    )
+    def deferred_scalar_loader(self, obj):
+        self.expired_attribute_loader = obj
+
     def __init__(self, class_):
         self.class_ = class_
         self.info = {}
diff --git a/lib/sqlalchemy/orm/interfaces.py b/lib/sqlalchemy/orm/interfaces.py
index 09d1858b9..abb6b14cc 100644
--- a/lib/sqlalchemy/orm/interfaces.py
+++ b/lib/sqlalchemy/orm/interfaces.py
@@ -538,9 +538,10 @@ class StrategizedProperty(MapperProperty):
             return self._strategies[key]
         except KeyError:
             cls = self._strategy_lookup(self, *key)
-            self._strategies[key] = self._strategies[cls] = strategy = cls(
-                self, key
-            )
+            # this previosuly was setting self._strategies[cls], that's
+            # a bad idea; should use strategy key at all times because every
+            # strategy has multiple keys at this point
+            self._strategies[key] = strategy = cls(self, key)
             return strategy
 
     def setup(self, context, query_entity, path, adapter, **kwargs):
diff --git a/lib/sqlalchemy/orm/loading.py b/lib/sqlalchemy/orm/loading.py
index 106ea7985..8de7d5a8b 100644
--- a/lib/sqlalchemy/orm/loading.py
+++ b/lib/sqlalchemy/orm/loading.py
@@ -21,6 +21,7 @@ from . import exc as orm_exc
 from . import path_registry
 from . import strategy_options
 from .base import _DEFER_FOR_STATE
+from .base import _RAISE_FOR_STATE
 from .base import _SET_DEFERRED_EXPIRED
 from .util import _none_set
 from .util import aliased
@@ -384,6 +385,8 @@ def _instance_processor(
                 # searching in the result to see if the column might
                 # be present in some unexpected way.
                 populators["expire"].append((prop.key, False))
+            elif col is _RAISE_FOR_STATE:
+                populators["new"].append((prop.key, prop._raise_column_loader))
             else:
                 getter = None
                 # the "adapter" can be here via different paths,
diff --git a/lib/sqlalchemy/orm/mapper.py b/lib/sqlalchemy/orm/mapper.py
index 0d8d454eb..376ad1923 100644
--- a/lib/sqlalchemy/orm/mapper.py
+++ b/lib/sqlalchemy/orm/mapper.py
@@ -1247,7 +1247,7 @@ class Mapper(InspectionAttr):
         self.class_manager = manager
 
         manager.mapper = self
-        manager.deferred_scalar_loader = util.partial(
+        manager.expired_attribute_loader = util.partial(
             loading.load_scalar_attributes, self
         )
 
diff --git a/lib/sqlalchemy/orm/properties.py b/lib/sqlalchemy/orm/properties.py
index f8bf06926..2e6e105fa 100644
--- a/lib/sqlalchemy/orm/properties.py
+++ b/lib/sqlalchemy/orm/properties.py
@@ -53,6 +53,8 @@ class ColumnProperty(StrategizedProperty):
         "_is_polymorphic_discriminator",
         "_mapped_by_synonym",
         "_deferred_column_loader",
+        "_raise_column_loader",
+        "raiseload",
     )
 
     def __init__(self, *columns, **kwargs):
@@ -115,6 +117,16 @@ class ColumnProperty(StrategizedProperty):
         :param info: Optional data dictionary which will be populated into the
             :attr:`.MapperProperty.info` attribute of this object.
 
+        :param raiseload: if True, indicates the column should raise an error
+            when undeferred, rather than loading the value.  This can be
+            altered at query time by using the :func:`.deferred` option with
+            raiseload=False.
+
+            .. versionadded:: 1.4
+
+            .. seealso::
+
+                :ref:`deferred_raiseload`
 
         """
         super(ColumnProperty, self).__init__()
@@ -129,6 +141,7 @@ class ColumnProperty(StrategizedProperty):
         ]
         self.group = kwargs.pop("group", None)
         self.deferred = kwargs.pop("deferred", False)
+        self.raiseload = kwargs.pop("raiseload", False)
         self.instrument = kwargs.pop("_instrument", True)
         self.comparator_factory = kwargs.pop(
             "comparator_factory", self.__class__.Comparator
@@ -163,6 +176,8 @@ class ColumnProperty(StrategizedProperty):
             ("deferred", self.deferred),
             ("instrument", self.instrument),
         )
+        if self.raiseload:
+            self.strategy_key += (("raiseload", True),)
 
     @util.dependencies("sqlalchemy.orm.state", "sqlalchemy.orm.strategies")
     def _memoized_attr__deferred_column_loader(self, state, strategies):
@@ -172,6 +187,14 @@ class ColumnProperty(StrategizedProperty):
             self.key,
         )
 
+    @util.dependencies("sqlalchemy.orm.state", "sqlalchemy.orm.strategies")
+    def _memoized_attr__raise_column_loader(self, state, strategies):
+        return state.InstanceState._instance_level_callable_processor(
+            self.parent.class_manager,
+            strategies.LoadDeferredColumns(self.key, True),
+            self.key,
+        )
+
     def __clause_element__(self):
         """Allow the ColumnProperty to work in expression before it is turned
         into an instrumented attribute.
diff --git a/lib/sqlalchemy/orm/state.py b/lib/sqlalchemy/orm/state.py
index ead9bf2bb..c57af0784 100644
--- a/lib/sqlalchemy/orm/state.py
+++ b/lib/sqlalchemy/orm/state.py
@@ -593,11 +593,7 @@ class InstanceState(interfaces.InspectionAttrInfo):
             del self.__dict__["parents"]
 
         self.expired_attributes.update(
-            [
-                impl.key
-                for impl in self.manager._loader_impls
-                if impl.expire_missing or impl.key in dict_
-            ]
+            [impl.key for impl in self.manager._loader_impls]
         )
 
         if self.callables:
@@ -671,8 +667,13 @@ class InstanceState(interfaces.InspectionAttrInfo):
             return PASSIVE_NO_RESULT
 
         toload = self.expired_attributes.intersection(self.unmodified)
+        toload = toload.difference(
+            attr
+            for attr in toload
+            if not self.manager[attr].impl.load_on_unexpire
+        )
 
-        self.manager.deferred_scalar_loader(self, toload)
+        self.manager.expired_attribute_loader(self, toload)
 
         # if the loader failed, or this
         # instance state didn't have an identity,
@@ -719,11 +720,7 @@ class InstanceState(interfaces.InspectionAttrInfo):
         was never populated or modified.
 
         """
-        return self.unloaded.intersection(
-            attr
-            for attr in self.manager
-            if self.manager[attr].impl.expire_missing
-        )
+        return self.unloaded
 
     @property
     def _unloaded_non_object(self):
diff --git a/lib/sqlalchemy/orm/strategies.py b/lib/sqlalchemy/orm/strategies.py
index f82fc2c57..59877a521 100644
--- a/lib/sqlalchemy/orm/strategies.py
+++ b/lib/sqlalchemy/orm/strategies.py
@@ -21,6 +21,7 @@ from . import query
 from . import unitofwork
 from . import util as orm_util
 from .base import _DEFER_FOR_STATE
+from .base import _RAISE_FOR_STATE
 from .base import _SET_DEFERRED_EXPIRED
 from .interfaces import LoaderStrategy
 from .interfaces import StrategizedProperty
@@ -287,11 +288,14 @@ class ExpressionColumnLoader(ColumnLoader):
 
 @log.class_logger
 @properties.ColumnProperty.strategy_for(deferred=True, instrument=True)
+@properties.ColumnProperty.strategy_for(
+    deferred=True, instrument=True, raiseload=True
+)
 @properties.ColumnProperty.strategy_for(do_nothing=True)
 class DeferredColumnLoader(LoaderStrategy):
     """Provide loading behavior for a deferred :class:`.ColumnProperty`."""
 
-    __slots__ = "columns", "group"
+    __slots__ = "columns", "group", "raiseload"
 
     def __init__(self, parent, strategy_key):
         super(DeferredColumnLoader, self).__init__(parent, strategy_key)
@@ -299,6 +303,7 @@ class DeferredColumnLoader(LoaderStrategy):
             raise NotImplementedError(
                 "Deferred loading for composite " "types not implemented yet"
             )
+        self.raiseload = self.strategy_opts.get("raiseload", False)
         self.columns = self.parent_property.columns
         self.group = self.parent_property.group
 
@@ -306,14 +311,23 @@ class DeferredColumnLoader(LoaderStrategy):
         self, context, path, loadopt, mapper, result, adapter, populators
     ):
 
-        # this path currently does not check the result
-        # for the column; this is because in most cases we are
-        # working just with the setup_query() directive which does
-        # not support this, and the behavior here should be consistent.
+        # for a DeferredColumnLoader, this method is only used during a
+        # "row processor only" query; see test_deferred.py ->
+        # tests with "rowproc_only" in their name.  As of the 1.0 series,
+        # loading._instance_processor doesn't use a "row processing" function
+        # to populate columns, instead it uses data in the "populators"
+        # dictionary.  Normally, the DeferredColumnLoader.setup_query()
+        # sets up that data in the "memoized_populators" dictionary
+        # and "create_row_processor()" here is never invoked.
         if not self.is_class_level:
-            set_deferred_for_local_state = (
-                self.parent_property._deferred_column_loader
-            )
+            if self.raiseload:
+                set_deferred_for_local_state = (
+                    self.parent_property._raise_column_loader
+                )
+            else:
+                set_deferred_for_local_state = (
+                    self.parent_property._deferred_column_loader
+                )
             populators["new"].append((self.key, set_deferred_for_local_state))
         else:
             populators["expire"].append((self.key, False))
@@ -327,7 +341,7 @@ class DeferredColumnLoader(LoaderStrategy):
             useobject=False,
             compare_function=self.columns[0].type.compare_values,
             callable_=self._load_for_state,
-            expire_missing=False,
+            load_on_unexpire=False,
         )
 
     def setup_query(
@@ -374,8 +388,10 @@ class DeferredColumnLoader(LoaderStrategy):
             )
         elif self.is_class_level:
             memoized_populators[self.parent_property] = _SET_DEFERRED_EXPIRED
-        else:
+        elif not self.raiseload:
             memoized_populators[self.parent_property] = _DEFER_FOR_STATE
+        else:
+            memoized_populators[self.parent_property] = _RAISE_FOR_STATE
 
     def _load_for_state(self, state, passive):
         if not state.key:
@@ -408,6 +424,9 @@ class DeferredColumnLoader(LoaderStrategy):
                 % (orm_util.state_str(state), self.key)
             )
 
+        if self.raiseload:
+            self._invoke_raise_load(state, passive, "raise")
+
         query = session.query(localparent)
         if (
             loading.load_on_ident(
@@ -419,19 +438,33 @@ class DeferredColumnLoader(LoaderStrategy):
 
         return attributes.ATTR_WAS_SET
 
+    def _invoke_raise_load(self, state, passive, lazy):
+        raise sa_exc.InvalidRequestError(
+            "'%s' is not available due to raiseload=True" % (self,)
+        )
+
 
 class LoadDeferredColumns(object):
     """serializable loader object used by DeferredColumnLoader"""
 
-    def __init__(self, key):
+    def __init__(self, key, raiseload=False):
         self.key = key
+        self.raiseload = raiseload
 
     def __call__(self, state, passive=attributes.PASSIVE_OFF):
         key = self.key
 
         localparent = state.manager.mapper
         prop = localparent._props[key]
-        strategy = prop._strategies[DeferredColumnLoader]
+        if self.raiseload:
+            strategy_key = (
+                ("deferred", True),
+                ("instrument", True),
+                ("raiseload", True),
+            )
+        else:
+            strategy_key = (("deferred", True), ("instrument", True))
+        strategy = prop._get_strategy(strategy_key)
         return strategy._load_for_state(state, passive)
 
 
diff --git a/lib/sqlalchemy/orm/strategy_options.py b/lib/sqlalchemy/orm/strategy_options.py
index 00c91dab5..26f47f616 100644
--- a/lib/sqlalchemy/orm/strategy_options.py
+++ b/lib/sqlalchemy/orm/strategy_options.py
@@ -1357,7 +1357,7 @@ def noload(*keys):
 
 @loader_option()
 def raiseload(loadopt, attr, sql_only=False):
-    """Indicate that the given relationship attribute should disallow lazy loads.
+    """Indicate that the given attribute should raise an error if accessed.
 
     A relationship attribute configured with :func:`.orm.raiseload` will
     raise an :exc:`~sqlalchemy.exc.InvalidRequestError` upon access.   The
@@ -1367,15 +1367,19 @@ def raiseload(loadopt, attr, sql_only=False):
     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.
+    :func:`.orm.raiseload` applies to :func:`.relationship` attributes only.
+    In order to apply raise-on-SQL behavior to a column-based attribute,
+    use the :paramref:`.orm.defer.raiseload` parameter on the :func:`.defer`
+    loader option.
+
+    :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 relationship loading.
 
     This function is part of the :class:`.Load` interface and supports
     both method-chained and standalone operation.
 
-    :func:`.orm.raiseload` applies to :func:`.relationship` attributes only.
 
     .. versionadded:: 1.1
 
@@ -1385,6 +1389,8 @@ def raiseload(loadopt, attr, sql_only=False):
 
         :ref:`prevent_lazy_with_raiseload`
 
+        :ref:`deferred_raiseload`
+
     """
 
     return loadopt.set_relationship_strategy(
@@ -1440,7 +1446,7 @@ def defaultload(*keys):
 
 
 @loader_option()
-def defer(loadopt, key):
+def defer(loadopt, key, raiseload=False):
     r"""Indicate that the given column-oriented attribute should be deferred,
     e.g. not loaded until accessed.
 
@@ -1480,6 +1486,16 @@ def defer(loadopt, key):
 
     :param key: Attribute to be deferred.
 
+    :param raiseload: raise :class:`.InvalidRequestError` if the column
+     value is to be loaded from emitting SQL.   Used to prevent unwanted
+     SQL from being emitted.
+
+     .. versionadded:: 1.4
+
+     .. seealso::
+
+        :ref:`deferred_raiseload`
+
     :param \*addl_attrs: This option supports the old 0.8 style
      of specifying a path as a series of attributes, which is now superseded
      by the method-chained style.
@@ -1497,13 +1513,14 @@ def defer(loadopt, key):
         :func:`.orm.undefer`
 
     """
-    return loadopt.set_column_strategy(
-        (key,), {"deferred": True, "instrument": True}
-    )
+    strategy = {"deferred": True, "instrument": True}
+    if raiseload:
+        strategy["raiseload"] = True
+    return loadopt.set_column_strategy((key,), strategy)
 
 
 @defer._add_unbound_fn
-def defer(key, *addl_attrs):
+def defer(key, *addl_attrs, **kw):
     if addl_attrs:
         util.warn_deprecated(
             "The *addl_attrs on orm.defer is deprecated.  Please use "
@@ -1511,7 +1528,7 @@ def defer(key, *addl_attrs):
             "indicate a path."
         )
     return _UnboundLoad._from_keys(
-        _UnboundLoad.defer, (key,) + addl_attrs, False, {}
+        _UnboundLoad.defer, (key,) + addl_attrs, False, kw
     )