Warn when caching is disabled / document

This patch adds new warnings for all elements that
don't indicate their caching behavior, including user-defined
ClauseElement subclasses and third party dialects.
it additionally adds new documentation to discuss an apparent
performance degradation in 1.4 when caching is disabled as a
result in the significant expense incurred by ORM
lazy loaders, which in 1.3 used BakedQuery so were actually
cached.

As a result of adding the warnings, a fair degree of
lesser used SQL expression objects identified that they did not
define caching behavior so would have been producing
``[no key]``, including PostgreSQL constructs ``hstore``
and ``array``.  These have been amended to use inherit
cache where appropriate.  "on conflict" constructs in
PostgreSQL, MySQL, SQLite still explicitly don't generate
a cache key at this time.

The change also adds a test for all constructs via
assert_compile() to assert they will not generate cache
warnings.

Fixes: #7394
Change-Id: I85958affbb99bfad0f5efa21bc8f2a95e7e46981

1 files changed, 98 insertions, 3 deletions

diff --git a/lib/sqlalchemy/sql/traversals.py b/lib/sqlalchemy/sql/traversals.py
index 914a78dae..d58b5c2bb 100644
--- a/lib/sqlalchemy/sql/traversals.py
+++ b/lib/sqlalchemy/sql/traversals.py
@@ -49,7 +49,50 @@ def _preconfigure_traversals(target_hierarchy):
 
 
 class HasCacheKey:
+    """Mixin for objects which can produce a cache key.
+
+    .. seealso::
+
+        :class:`.CacheKey`
+
+        :ref:`sql_caching`
+
+    """
+
     _cache_key_traversal = NO_CACHE
+
+    _is_has_cache_key = True
+
+    _hierarchy_supports_caching = True
+    """private attribute which may be set to False to prevent the
+    inherit_cache warning from being emitted for a hierarchy of subclasses.
+
+    Currently applies to the DDLElement hierarchy which does not implement
+    caching.
+
+    """
+
+    inherit_cache = None
+    """Indicate if this :class:`.HasCacheKey` instance should make use of the
+    cache key generation scheme used by its immediate superclass.
+
+    The attribute defaults to ``None``, which indicates that a construct has
+    not yet taken into account whether or not its appropriate for it to
+    participate in caching; this is functionally equivalent to setting the
+    value to ``False``, except that a warning is also emitted.
+
+    This flag can be set to ``True`` on a particular class, if the SQL that
+    corresponds to the object does not change based on attributes which
+    are local to this class, and not its superclass.
+
+    .. seealso::
+
+        :ref:`compilerext_caching` - General guideslines for setting the
+        :attr:`.HasCacheKey.inherit_cache` attribute for third-party or user
+        defined SQL constructs.
+
+    """
+
     __slots__ = ()
 
     @classmethod
@@ -60,7 +103,8 @@ class HasCacheKey:
         so should only be called once per class.
 
         """
-        inherit = cls.__dict__.get("inherit_cache", False)
+        inherit_cache = cls.__dict__.get("inherit_cache", None)
+        inherit = bool(inherit_cache)
 
         if inherit:
             _cache_key_traversal = getattr(cls, "_cache_key_traversal", None)
@@ -89,6 +133,23 @@ class HasCacheKey:
                 )
                 if _cache_key_traversal is None:
                     cls._generated_cache_key_traversal = NO_CACHE
+                    if (
+                        inherit_cache is None
+                        and cls._hierarchy_supports_caching
+                    ):
+                        util.warn(
+                            "Class %s will not make use of SQL compilation "
+                            "caching as it does not set the 'inherit_cache' "
+                            "attribute to ``True``.  This can have "
+                            "significant performance implications including "
+                            "some performance degradations in comparison to "
+                            "prior SQLAlchemy versions.  Set this attribute "
+                            "to True if this object can make use of the cache "
+                            "key generated by the superclass.  Alternatively, "
+                            "this attribute may be set to False which will "
+                            "disable this warning." % (cls.__name__),
+                            code="cprf",
+                        )
                     return NO_CACHE
 
             return _cache_key_traversal_visitor.generate_dispatch(
@@ -273,6 +334,15 @@ class MemoizedHasCacheKey(HasCacheKey, HasMemoized):
 
 
 class CacheKey(namedtuple("CacheKey", ["key", "bindparams"])):
+    """The key used to identify a SQL statement construct in the
+    SQL compilation cache.
+
+    .. seealso::
+
+        :ref:`sql_caching`
+
+    """
+
     def __hash__(self):
         """CacheKey itself is not hashable - hash the .key portion"""
 
@@ -480,7 +550,19 @@ class _CacheKey(ExtendedInternalTraversal):
             tuple(elem._gen_cache_key(anon_map, bindparams) for elem in obj),
         )
 
-    visit_executable_options = visit_has_cache_key_list
+    def visit_executable_options(
+        self, attrname, obj, parent, anon_map, bindparams
+    ):
+        if not obj:
+            return ()
+        return (
+            attrname,
+            tuple(
+                elem._gen_cache_key(anon_map, bindparams)
+                for elem in obj
+                if elem._is_has_cache_key
+            ),
+        )
 
     def visit_inspectable_list(
         self, attrname, obj, parent, anon_map, bindparams
@@ -1086,7 +1168,20 @@ class TraversalComparatorStrategy(InternalTraversal, util.MemoizedSlots):
             ):
                 return COMPARE_FAILED
 
-    visit_executable_options = visit_has_cache_key_list
+    def visit_executable_options(
+        self, attrname, left_parent, left, right_parent, right, **kw
+    ):
+        for l, r in zip_longest(left, right, fillvalue=None):
+            if (
+                l._gen_cache_key(self.anon_map[0], [])
+                if l._is_has_cache_key
+                else l
+            ) != (
+                r._gen_cache_key(self.anon_map[1], [])
+                if r._is_has_cache_key
+                else r
+            ):
+                return COMPARE_FAILED
 
     def visit_clauseelement(
         self, attrname, left_parent, left, right_parent, right, **kw