]> git.ipfire.org Git - thirdparty/sqlalchemy/sqlalchemy.git/commitdiff
generalize cache_ok to UserDefinedType
authorMike Bayer <mike_mp@zzzcomputing.com>
Wed, 17 Nov 2021 20:08:29 +0000 (15:08 -0500)
committerMike Bayer <mike_mp@zzzcomputing.com>
Wed, 17 Nov 2021 20:55:12 +0000 (15:55 -0500)
Extended the ``cache_ok`` flag 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`.

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, it may return
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.

Fixes: #7319
Change-Id: Ie0b5d4587df87bfe66d2fe7cd4585c3882584575

doc/build/changelog/unreleased_14/7319.rst [new file with mode: 0644]
doc/build/core/custom_types.rst
doc/build/core/type_api.rst
lib/sqlalchemy/sql/type_api.py
lib/sqlalchemy/types.py
test/sql/test_compare.py
test/sql/test_types.py

diff --git a/doc/build/changelog/unreleased_14/7319.rst b/doc/build/changelog/unreleased_14/7319.rst
new file mode 100644 (file)
index 0000000..0c2b19d
--- /dev/null
@@ -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`
index e7564957ed331398c692b97f320cffe339cadd87..3759fce4ba6b3d19c3c1566517d29a5c91792c46 100644 (file)
@@ -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:
 
index 0dd1b492053063c6029f7fb45669ea94e59bb1b9..2586b2b732ada4ab73003698c72e2c31c6958f36 100644 (file)
@@ -18,6 +18,8 @@ Base Type API
 
 .. autoclass:: NullType
 
+.. autoclass:: ExternalType
+    :members:
 
 .. autoclass:: Variant
    :members: with_variant, __init__
index f588512687cbeb7b404956f96c82215d5471a297..46fbefcd3e3f2e81cc047d3fa8b4e116279ab332 100644 (file)
@@ -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
index df8abdc6944f6710e75d725e21a197e4c38fc9d3..9e695f6782b0728a13c8de97d13e46ab5491b1d7 100644 (file)
@@ -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
index 8ec88a760b47d5995ac41b134643fc7e62c6b6ce..5258d09b09c879e0b36067cddb675ff5d3fd99f8 100644 (file)
@@ -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
index a15d163e04144ea5490d8e3a7a927c45803b7f09..c16a975513df8832b4e68b81d264b3eb66b519f0 100644 (file)
@@ -1412,6 +1412,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"
 
@@ -1422,6 +1424,8 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
                 return process
 
         class UTypeTwo(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPETWO"
 
@@ -1470,6 +1474,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"
 
@@ -1480,6 +1486,8 @@ class VariantTest(fixtures.TestBase, AssertsCompiledSQL):
                 return process
 
         class UTypeTwo(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "UTYPETWO"
 
@@ -2854,6 +2862,8 @@ class ExpressionTest(
         global MyCustomType, MyTypeDec
 
         class MyCustomType(types.UserDefinedType):
+            cache_ok = True
+
             def get_col_spec(self):
                 return "INT"