From: Mike Bayer Date: Wed, 17 Nov 2021 20:08:29 +0000 (-0500) Subject: generalize cache_ok to UserDefinedType X-Git-Tag: rel_2_0_0b1~643^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4761e6878b127f7d5fb09addaae15426edbb0b73;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git generalize cache_ok to UserDefinedType 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 --- diff --git a/doc/build/changelog/unreleased_14/7319.rst b/doc/build/changelog/unreleased_14/7319.rst new file mode 100644 index 0000000000..0c2b19d314 --- /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 e7564957ed..3759fce4ba 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 0dd1b49205..2586b2b732 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 f588512687..46fbefcd3e 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 + (, ('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 + :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 + (, ('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 "", line 1, + in 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 + (, ('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:: - - (, ('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 df8abdc694..9e695f6782 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 8ec88a760b..5258d09b09 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 a15d163e04..c16a975513 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -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"