Merge "generalize cache_ok to UserDefinedType" into main

7 files changed, 242 insertions, 65 deletions

diff --git a/doc/build/changelog/unreleased_14/7319.rst b/doc/build/changelog/unreleased_14/7319.rst
new file mode 100644
index 000000000..0c2b19d31
--- /dev/null
+++ b/doc/build/changelog/unreleased_14/7319.rst
@@ -0,0 +1,24 @@
+.. change::
+    :tags: bug, types, regression
+    :tickets: 7319
+
+    Extended the :attr:`.TypeDecorator.cache_ok` attribute and corresponding
+    warning message if this flag is not defined, a behavior first established
+    for :class:`.TypeDecorator` as part of :ticket:`6436`, to also take place
+    for :class:`.UserDefinedType`, by generalizing the flag and associated
+    caching logic to a new common base for these two types,
+    :class:`.ExternalType` to create :attr:`.UserDefinedType.cache_ok`.
+
+    The change means any current :class:`.UserDefinedType` will now cause SQL
+    statement caching to no longer take place for statements which make use of
+    the datatype, along with a warning being emitted, unless the class defines
+    the :attr:`.UserDefinedType.cache_ok` flag as True. If the datatype cannot
+    form a deterministic, hashable cache key derived from its arguments,
+    the attribute may be set to False which will continue to keep caching disabled but will suppress the
+    warning. In particular, custom datatypes currently used in packages such as
+    SQLAlchemy-utils will need to implement this flag. The issue was observed
+    as a result of a SQLAlchemy-utils datatype that is not currently cacheable.
+
+    .. seealso::
+
+        :attr:`.ExternalType.cache_ok`
diff --git a/doc/build/core/custom_types.rst b/doc/build/core/custom_types.rst
index e7564957e..3759fce4b 100644
--- a/doc/build/core/custom_types.rst
+++ b/doc/build/core/custom_types.rst
@@ -67,6 +67,7 @@ to and/or from the database is required.
 .. autoclass:: TypeDecorator
    :members:
 
+   .. autoattribute:: cache_ok
 
 TypeDecorator Recipes
 ---------------------
@@ -594,6 +595,7 @@ is needed, use :class:`.TypeDecorator` instead.
 .. autoclass:: UserDefinedType
    :members:
 
+   .. autoattribute:: cache_ok
 
 .. _custom_and_decorated_types_reflection:
 
diff --git a/doc/build/core/type_api.rst b/doc/build/core/type_api.rst
index 0dd1b4920..2586b2b73 100644
--- a/doc/build/core/type_api.rst
+++ b/doc/build/core/type_api.rst
@@ -18,6 +18,8 @@ Base Type API
 
 .. autoclass:: NullType
 
+.. autoclass:: ExternalType
+    :members:
 
 .. autoclass:: Variant
    :members: with_variant, __init__
diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py
index 34f23fb0c..5e929c4a9 100644
--- a/lib/sqlalchemy/sql/type_api.py
+++ b/lib/sqlalchemy/sql/type_api.py
@@ -810,7 +810,180 @@ class VisitableCheckKWArg(util.EnsureKWArgType, TraversibleType):
     pass
 
 
-class UserDefinedType(util.with_metaclass(VisitableCheckKWArg, TypeEngine)):
+class ExternalType(object):
+    """mixin that defines attributes and behaviors specific to third-party
+    datatypes.
+
+    "Third party" refers to datatypes that are defined outside the scope
+    of SQLAlchemy within either end-user application code or within
+    external extensions to SQLAlchemy.
+
+    Subclasses currently include :class:`.TypeDecorator` and
+    :class:`.UserDefinedType`.
+
+    .. versionadded:: 1.4.28
+
+    """
+
+    cache_ok = None
+    """Indicate if statements using this :class:`.ExternalType` are "safe to
+    cache".
+
+    The default value ``None`` will emit a warning and then not allow caching
+    of a statement which includes this type.   Set to ``False`` to disable
+    statements using this type from being cached at all without a warning.
+    When set to ``True``, the object's class and selected elements from its
+    state will be used as part of the cache key.  For example, using a
+    :class:`.TypeDecorator`::
+
+        class MyType(TypeDecorator):
+            impl = String
+
+            cache_ok = True
+
+            def __init__(self, choices):
+                self.choices = tuple(choices)
+                self.internal_only = True
+
+    The cache key for the above type would be equivalent to::
+
+        >>> MyType(["a", "b", "c"])._static_cache_key
+        (<class '__main__.MyType'>, ('choices', ('a', 'b', 'c')))
+
+    The caching scheme will extract attributes from the type that correspond
+    to the names of parameters in the ``__init__()`` method.  Above, the
+    "choices" attribute becomes part of the cache key but "internal_only"
+    does not, because there is no parameter named "internal_only".
+
+    The requirements for cacheable elements is that they are hashable
+    and also that they indicate the same SQL rendered for expressions using
+    this type every time for a given cache value.
+
+    To accommodate for datatypes that refer to unhashable structures such
+    as dictionaries, sets and lists, these objects can be made "cacheable"
+    by assigning hashable structures to the attributes whose names
+    correspond with the names of the arguments.  For example, a datatype
+    which accepts a dictionary of lookup values may publish this as a sorted
+    series of tuples.   Given a previously un-cacheable type as::
+
+        class LookupType(UserDefinedType):
+            '''a custom type that accepts a dictionary as a parameter.
+
+            this is the non-cacheable version, as "self.lookup" is not
+            hashable.
+
+            '''
+
+            def __init__(self, lookup):
+                self.lookup = lookup
+
+            def get_col_spec(self, **kw):
+                return "VARCHAR(255)"
+
+            def bind_processor(self, dialect):
+                # ...  works with "self.lookup" ...
+
+    Where "lookup" is a dictionary.  The type will not be able to generate
+    a cache key::
+
+        >>> type_ = LookupType({"a": 10, "b": 20})
+        >>> type_._static_cache_key
+        <stdin>:1: SAWarning: UserDefinedType LookupType({'a': 10, 'b': 20}) will not
+        produce a cache key because the ``cache_ok`` flag is not set to True.
+        Set this flag to True if this type object's state is safe to use
+        in a cache key, or False to disable this warning.
+        symbol('no_cache')
+
+    If we **did** set up such a cache key, it wouldn't be usable. We would
+    get a tuple structure that contains a dictionary inside of it, which
+    cannot itself be used as a key in a "cache dictionary" such as SQLAlchemy's
+    statement cache, since Python dictionaries aren't hashable::
+
+        >>> # set cache_ok = True
+        >>> type_.cache_ok = True
+
+        >>> # this is the cache key it would generate
+        >>> key = type_._static_cache_key
+        >>> key
+        (<class '__main__.LookupType'>, ('lookup', {'a': 10, 'b': 20}))
+
+        >>> # however this key is not hashable, will fail when used with
+        >>> # SQLAlchemy statement cache
+        >>> some_cache = {key: "some sql value"}
+        Traceback (most recent call last): File "<stdin>", line 1,
+        in <module> TypeError: unhashable type: 'dict'
+
+    The type may be made cacheable by assigning a sorted tuple of tuples
+    to the ".lookup" attribute::
+
+        class LookupType(UserDefinedType):
+            '''a custom type that accepts a dictionary as a parameter.
+
+            The dictionary is stored both as itself in a private variable,
+            and published in a public variable as a sorted tuple of tuples,
+            which is hashable and will also return the same value for any
+            two equivalent dictionaries.  Note it assumes the keys and
+            values of the dictionary are themselves hashable.
+
+            '''
+
+            cache_ok = True
+
+            def __init__(self, lookup):
+                self._lookup = lookup
+
+                # assume keys/values of "lookup" are hashable; otherwise
+                # they would also need to be converted in some way here
+                self.lookup = tuple(
+                    (key, lookup[key]) for key in sorted(lookup)
+                )
+
+            def get_col_spec(self, **kw):
+                return "VARCHAR(255)"
+
+            def bind_processor(self, dialect):
+                # ...  works with "self._lookup" ...
+
+    Where above, the cache key for ``LookupType({"a": 10, "b": 20})`` will be::
+
+        >>> LookupType({"a": 10, "b": 20})._static_cache_key
+        (<class '__main__.LookupType'>, ('lookup', (('a', 10), ('b', 20))))
+
+    .. versionadded:: 1.4.14 - added the ``cache_ok`` flag to allow
+       some configurability of caching for :class:`.TypeDecorator` classes.
+
+    .. versionadded:: 1.4.28 - added the :class:`.ExternalType` mixin which
+       generalizes the ``cache_ok`` flag to both the :class:`.TypeDecorator`
+       and :class:`.UserDefinedType` classes.
+
+    .. seealso::
+
+        :ref:`sql_caching`
+
+    """  # noqa: E501
+
+    @property
+    def _static_cache_key(self):
+        if self.cache_ok is None:
+            subtype_idx = self.__class__.__mro__.index(ExternalType)
+            subtype = self.__class__.__mro__[max(subtype_idx - 1, 0)]
+
+            util.warn(
+                "%s %r will not produce a cache key because "
+                "the ``cache_ok`` flag is not set to True.  "
+                "Set this flag to True if this type object's "
+                "state is safe to use in a cache key, or False to "
+                "disable this warning." % (subtype.__name__, self)
+            )
+        elif self.cache_ok is True:
+            return super(ExternalType, self)._static_cache_key
+
+        return NO_CACHE
+
+
+class UserDefinedType(
+    util.with_metaclass(VisitableCheckKWArg, ExternalType, TypeEngine)
+):
     """Base for user defined types.
 
     This should be the base of new types.  Note that
@@ -820,6 +993,8 @@ class UserDefinedType(util.with_metaclass(VisitableCheckKWArg, TypeEngine)):
       import sqlalchemy.types as types
 
       class MyType(types.UserDefinedType):
+          cache_ok = True
+
           def __init__(self, precision = 8):
               self.precision = precision
 
@@ -855,6 +1030,20 @@ class UserDefinedType(util.with_metaclass(VisitableCheckKWArg, TypeEngine)):
        the ``get_col_spec()`` method via the keyword argument
        ``type_expression``, if it receives ``**kw`` in its signature.
 
+    The :attr:`.UserDefinedType.cache_ok` class-level flag indicates if this
+    custom :class:`.UserDefinedType` is safe to be used as part of a cache key.
+    This flag defaults to ``None`` which will initially generate a warning
+    when the SQL compiler attempts to generate a cache key for a statement
+    that uses this type.  If the :class:`.UserDefinedType` is not guaranteed
+    to produce the same bind/result behavior and SQL generation
+    every time, this flag should be set to ``False``; otherwise if the
+    class produces the same behavior each time, it may be set to ``True``.
+    See :attr:`.UserDefinedType.cache_ok` for further notes on how this works.
+
+    .. versionadded:: 1.4.28 Generalized the :attr:`.ExternalType.cache_ok`
+       flag so that it is available for both :class:`.TypeDecorator` as well
+       as :class:`.UserDefinedType`.
+
     """
 
     __visit_name__ = "user_defined"
@@ -952,7 +1141,7 @@ class NativeForEmulated(object):
         return cls(**kw)
 
 
-class TypeDecorator(SchemaEventTarget, TypeEngine):
+class TypeDecorator(ExternalType, SchemaEventTarget, TypeEngine):
     """Allows the creation of types which add additional functionality
     to an existing type.
 
@@ -1115,47 +1304,6 @@ class TypeDecorator(SchemaEventTarget, TypeEngine):
 
     """
 
-    cache_ok = None
-    """Indicate if statements using this :class:`.TypeDecorator` are "safe to
-    cache".
-
-    The default value ``None`` will emit a warning and then not allow caching
-    of a statement which includes this type.   Set to ``False`` to disable
-    statements using this type from being cached at all without a warning.
-    When set to ``True``, the object's class and selected elements from its
-    state will be used as part of the cache key, e.g.::
-
-        class MyType(TypeDecorator):
-            impl = String
-
-            cache_ok = True
-
-            def __init__(self, choices):
-                self.choices = tuple(choices)
-                self.internal_only = True
-
-    The cache key for the above type would be equivalent to::
-
-        (<class '__main__.MyType'>, ('choices', ('a', 'b', 'c')))
-
-    The caching scheme will extract attributes from the type that correspond
-    to the names of parameters in the ``__init__()`` method.  Above, the
-    "choices" attribute becomes part of the cache key but "internal_only"
-    does not, because there is no parameter named "internal_only".
-
-    The requirements for cacheable elements is that they are hashable
-    and also that they indicate the same SQL rendered for expressions using
-    this type every time for a given cache value.
-
-    .. versionadded:: 1.4.14 - added the ``cache_ok`` flag to allow
-       some configurability of caching for :class:`.TypeDecorator` classes.
-
-    .. seealso::
-
-        :ref:`sql_caching`
-
-    """
-
     class Comparator(TypeEngine.Comparator):
         """A :class:`.TypeEngine.Comparator` that is specific to
         :class:`.TypeDecorator`.
@@ -1191,21 +1339,6 @@ class TypeDecorator(SchemaEventTarget, TypeEngine):
                 {},
             )
 
-    @property
-    def _static_cache_key(self):
-        if self.cache_ok is None:
-            util.warn(
-                "TypeDecorator %r will not produce a cache key because "
-                "the ``cache_ok`` flag is not set to True.  "
-                "Set this flag to True if this type object's "
-                "state is safe to use in a cache key, or False to "
-                "disable this warning." % self
-            )
-        elif self.cache_ok is True:
-            return super(TypeDecorator, self)._static_cache_key
-
-        return NO_CACHE
-
     def _gen_dialect_impl(self, dialect):
         """
         #todo
diff --git a/lib/sqlalchemy/types.py b/lib/sqlalchemy/types.py
index df8abdc69..9e695f678 100644
--- a/lib/sqlalchemy/types.py
+++ b/lib/sqlalchemy/types.py
@@ -13,6 +13,7 @@ __all__ = [
     "TypeEngine",
     "TypeDecorator",
     "UserDefinedType",
+    "ExternalType",
     "INT",
     "CHAR",
     "VARCHAR",
@@ -110,6 +111,7 @@ from .sql.sqltypes import UnicodeText
 from .sql.sqltypes import VARBINARY
 from .sql.sqltypes import VARCHAR
 from .sql.type_api import adapt_type
+from .sql.type_api import ExternalType
 from .sql.type_api import to_instance
 from .sql.type_api import TypeDecorator
 from .sql.type_api import TypeEngine
diff --git a/test/sql/test_compare.py b/test/sql/test_compare.py
index 8ec88a760..5258d09b0 100644
--- a/test/sql/test_compare.py
+++ b/test/sql/test_compare.py
@@ -67,6 +67,7 @@ from sqlalchemy.sql.selectable import LABEL_STYLE_TABLENAME_PLUS_COL
 from sqlalchemy.sql.selectable import Select
 from sqlalchemy.sql.selectable import Selectable
 from sqlalchemy.sql.selectable import SelectStatementGrouping
+from sqlalchemy.sql.type_api import UserDefinedType
 from sqlalchemy.sql.visitors import InternalTraversal
 from sqlalchemy.testing import eq_
 from sqlalchemy.testing import fixtures
@@ -1744,19 +1745,21 @@ class ExecutableFlagsTest(fixtures.TestBase):
 
 
 class TypesTest(fixtures.TestBase):
-    def test_typedec_no_cache(self):
-        class MyType(TypeDecorator):
+    @testing.combinations(TypeDecorator, UserDefinedType)
+    def test_thirdparty_no_cache(self, base):
+        class MyType(base):
             impl = String
 
         expr = column("q", MyType()) == 1
 
         with expect_warnings(
-            r"TypeDecorator MyType\(\) will not produce a cache key"
+            r"%s MyType\(\) will not produce a cache key" % base.__name__
         ):
             is_(expr._generate_cache_key(), None)
 
-    def test_typedec_cache_false(self):
-        class MyType(TypeDecorator):
+    @testing.combinations(TypeDecorator, UserDefinedType)
+    def test_thirdparty_cache_false(self, base):
+        class MyType(base):
             impl = String
 
             cache_ok = False
@@ -1765,8 +1768,9 @@ class TypesTest(fixtures.TestBase):
 
         is_(expr._generate_cache_key(), None)
 
-    def test_typedec_cache_ok(self):
-        class MyType(TypeDecorator):
+    @testing.combinations(TypeDecorator, UserDefinedType)
+    def test_thirdparty_cache_ok(self, base):
+        class MyType(base):
             impl = String
 
             cache_ok = True
diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index 7f850d00b..d57603622 100644
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -1411,6 +1411,8 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
 
     def test_type_decorator_compile_variant_two(self):
         class UTypeOne(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPEONE"
 
@@ -1421,6 +1423,8 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
                 return process
 
         class UTypeTwo(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPETWO"
 
@@ -1469,6 +1473,8 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
 class VariantTest(fixtures.TestBase, AssertsCompiledSQL):
     def setup_test(self):
         class UTypeOne(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPEONE"
 
@@ -1479,6 +1485,8 @@ class VariantTest(fixtures.TestBase, AssertsCompiledSQL):
                 return process
 
         class UTypeTwo(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPETWO"
 
@@ -2853,6 +2861,8 @@ class ExpressionTest(
         global MyCustomType, MyTypeDec
 
         class MyCustomType(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "INT"