Skip to content

Commit

Permalink
don't cache TypeDecorator by default
Browse files Browse the repository at this point in the history
The :class:`.TypeDecorator` class will now emit a warning when used in SQL
compilation with caching unless the ``.cache_ok`` flag is set to ``True``
or ``False``. ``.cache_ok`` indicates that all the parameters passed to the
object are safe to be used as a cache key, ``False`` means they are not.

Fixes: #6436
Change-Id: Ib1bb7dc4b124e38521d615c2e2e691e4915594fb
  • Loading branch information
zzzeek committed May 6, 2021
1 parent 900d76b commit 6967b45
Show file tree
Hide file tree
Showing 36 changed files with 308 additions and 1 deletion.
10 changes: 10 additions & 0 deletions doc/build/changelog/unreleased_14/6436.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
.. change::
:tags: bug, sql, regression
:tickets: 6436

The :class:`.TypeDecorator` class will now emit a warning when used in SQL
compilation with caching unless the ``.cache_ok`` flag is set to ``True``
or ``False``. A new class-level attribute :attr:`.TypeDecorator.cache_ok`
may be set which will be used as an indication that all the parameters
passed to the object are safe to be used as a cache key if set to ``True``,
``False`` means they are not.
9 changes: 9 additions & 0 deletions doc/build/core/custom_types.rst
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ denormalize::

class TZDateTime(TypeDecorator):
impl = DateTime
cache_ok = True

def process_bind_param(self, value, dialect):
if value is not None:
Expand Down Expand Up @@ -186,6 +187,7 @@ binary in CHAR(16) if desired::

"""
impl = CHAR
cache_ok = True

def load_dialect_impl(self, dialect):
if dialect.name == 'postgresql':
Expand Down Expand Up @@ -233,6 +235,8 @@ to/from JSON. Can be modified to use Python's builtin json encoder::

impl = VARCHAR

cache_ok = True

def process_bind_param(self, value, dialect):
if value is not None:
value = json.dumps(value)
Expand Down Expand Up @@ -306,6 +310,8 @@ method::

impl = VARCHAR

cache_ok = True

def coerce_compared_value(self, op, value):
if op in (operators.like_op, operators.not_like_op):
return String()
Expand Down Expand Up @@ -416,8 +422,11 @@ transparently::
class PGPString(TypeDecorator):
impl = BYTEA

cache_ok = True

def __init__(self, passphrase):
super(PGPString, self).__init__()

self.passphrase = passphrase

def bind_expression(self, bindvalue):
Expand Down
2 changes: 2 additions & 0 deletions lib/sqlalchemy/dialects/mssql/information_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

class CoerceUnicode(TypeDecorator):
impl = Unicode
cache_ok = True

def process_bind_param(self, value, dialect):
if util.py2k and isinstance(value, util.binary_type):
Expand Down Expand Up @@ -211,6 +212,7 @@ class IdentitySqlVariant(TypeDecorator):
correct value as string.
"""
impl = Unicode
cache_ok = True

def column_expression(self, colexpr):
return cast(colexpr, Numeric)
Expand Down
1 change: 1 addition & 0 deletions lib/sqlalchemy/dialects/sqlite/pysqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,7 @@ def regexp(a, b):
class MixedBinary(TypeDecorator):
impl = String
cache_ok = True
def process_result_value(self, value, dialect):
if isinstance(value, str):
Expand Down
2 changes: 2 additions & 0 deletions lib/sqlalchemy/sql/sqltypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1780,6 +1780,7 @@ class PickleType(TypeDecorator):
"""

impl = LargeBinary
cache_ok = True

def __init__(
self, protocol=pickle.HIGHEST_PROTOCOL, pickler=None, comparator=None
Expand Down Expand Up @@ -2027,6 +2028,7 @@ class Interval(Emulated, _AbstractInterval, TypeDecorator):

impl = DateTime
epoch = dt.datetime.utcfromtimestamp(0)
cache_ok = True

def __init__(self, native=True, second_precision=None, day_precision=None):
"""Construct an Interval object.
Expand Down
6 changes: 5 additions & 1 deletion lib/sqlalchemy/sql/traversals.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,11 @@ def _gen_cache_key(self, anon_map, bindparams):
# efficient switch construct

if meth is STATIC_CACHE_KEY:
result += (attrname, obj._static_cache_key)
sck = obj._static_cache_key
if sck is NO_CACHE:
anon_map[NO_CACHE] = True
return None
result += (attrname, sck)
elif meth is ANON_NAME:
elements = util.preloaded.sql_elements
if isinstance(obj, elements._anonymous_label):
Expand Down
73 changes: 73 additions & 0 deletions lib/sqlalchemy/sql/type_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"""


from sqlalchemy.sql.traversals import NO_CACHE
from . import operators
from .base import SchemaEventTarget
from .visitors import Traversible
Expand Down Expand Up @@ -872,6 +873,8 @@ class MyType(types.TypeDecorator):
impl = types.Unicode
cache_ok = True
def process_bind_param(self, value, dialect):
return "PREFIX:" + value
Expand All @@ -887,6 +890,16 @@ def copy(self, **kw):
given; in this case, the ``impl`` variable can reference
``TypeEngine`` as a placeholder.
The :attr:`.TypeDecorator.cache_ok` class-level flag indicates if this
custom :class:`.TypeDecorator` 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:`.TypeDecorator` 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:`.TypeDecorator.cache_ok` for further notes on how this works.
Types that receive a Python type that isn't similar to the ultimate type
used may want to define the :meth:`TypeDecorator.coerce_compared_value`
method. This is used to give the expression system a hint when coercing
Expand Down Expand Up @@ -946,6 +959,8 @@ def coerce_compared_value(self, op, value):
class MyJsonType(TypeDecorator):
impl = postgresql.JSON
cache_ok = True
def coerce_compared_value(self, op, value):
return self.impl.coerce_compared_value(op, value)
Expand Down Expand Up @@ -1002,6 +1017,47 @@ def __init__(self, *args, **kwargs):
"""

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`.
Expand Down Expand Up @@ -1037,6 +1093,21 @@ def comparator_factory(self):
{},
)

@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
Expand Down Expand Up @@ -1465,6 +1536,8 @@ class Variant(TypeDecorator):
"""

cache_ok = True

def __init__(self, base, mapping):
"""Construct a new :class:`.Variant`.
Expand Down
2 changes: 2 additions & 0 deletions lib/sqlalchemy/testing/suite/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ class _DateFixture(_LiteralRoundTripFixture, fixtures.TestBase):
def define_tables(cls, metadata):
class Decorated(TypeDecorator):
impl = cls.datatype
cache_ok = True

Table(
"date_table",
Expand Down Expand Up @@ -477,6 +478,7 @@ class CastTypeDecoratorTest(_LiteralRoundTripFixture, fixtures.TestBase):
def string_as_int(self):
class StringAsInt(TypeDecorator):
impl = String(50)
cache_ok = True

def get_dbapi_type(self, dbapi):
return dbapi.NUMBER
Expand Down
1 change: 1 addition & 0 deletions test/dialect/mssql/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1196,6 +1196,7 @@ def test_text_text_literal_binds(self):

class MyPickleType(types.TypeDecorator):
impl = PickleType
cache_ok = True

def process_bind_param(self, value, dialect):
if value:
Expand Down
1 change: 1 addition & 0 deletions test/dialect/mysql/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,7 @@ def test_cast(self, type_, expected):
def test_cast_type_decorator(self):
class MyInteger(sqltypes.TypeDecorator):
impl = Integer
cache_ok = True

type_ = MyInteger()
t = sql.table("t", sql.column("col"))
Expand Down
1 change: 1 addition & 0 deletions test/dialect/mysql/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ def test_boolean_roundtrip_reflected(

class MyTime(TypeDecorator):
impl = TIMESTAMP
cache_ok = True

@testing.combinations(
(TIMESTAMP,),
Expand Down
1 change: 1 addition & 0 deletions test/dialect/oracle/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,7 @@ def test_for_update_of_w_limit_offset_adaption_partial_col_unpresent(self):
def test_limit_preserves_typing_information(self):
class MyType(TypeDecorator):
impl = Integer
cache_ok = True

stmt = select(type_coerce(column("x"), MyType).label("foo")).limit(1)
dialect = oracle.dialect()
Expand Down
1 change: 1 addition & 0 deletions test/dialect/oracle/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -1189,6 +1189,7 @@ def test_setinputsizes(

class TestTypeDec(TypeDecorator):
impl = NullType()
cache_ok = True

def load_dialect_impl(self, dialect):
if dialect.name == "oracle":
Expand Down
2 changes: 2 additions & 0 deletions test/dialect/postgresql/test_dialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,8 @@ def test_serial_integer(self):
class BITD(TypeDecorator):
impl = Integer

cache_ok = True

def load_dialect_impl(self, dialect):
if dialect.name == "postgresql":
return BigInteger()
Expand Down
2 changes: 2 additions & 0 deletions test/dialect/postgresql/test_on_conflict.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ def define_tables(cls, metadata):
class SpecialType(sqltypes.TypeDecorator):
impl = String

cache_ok = True

def process_bind_param(self, value, dialect):
return value + " processed"

Expand Down
3 changes: 3 additions & 0 deletions test/dialect/postgresql/test_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ def test_schema_reflection(self, metadata, connection):
def test_custom_subclass(self, metadata, connection):
class MyEnum(TypeDecorator):
impl = Enum("oneHI", "twoHI", "threeHI", name="myenum")
cache_ok = True

def process_bind_param(self, value, dialect):
if value is not None:
Expand Down Expand Up @@ -1391,6 +1392,7 @@ class ArrayRoundTripTest(object):
def define_tables(cls, metadata):
class ProcValue(TypeDecorator):
impl = cls.ARRAY(Integer, dimensions=2)
cache_ok = True

def process_bind_param(self, value, dialect):
if value is None:
Expand Down Expand Up @@ -2127,6 +2129,7 @@ def test_array_overlap_exec(self, connection):
class _ArrayOfEnum(TypeDecorator):
# previous workaround for array of enum
impl = postgresql.ARRAY
cache_ok = True

def bind_expression(self, bindvalue):
return sa.cast(bindvalue, self)
Expand Down
2 changes: 2 additions & 0 deletions test/dialect/test_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -1682,6 +1682,7 @@ def test_sqlite_no_autoincrement(self):
def test_sqlite_autoincrement_int_affinity(self):
class MyInteger(sqltypes.TypeDecorator):
impl = Integer
cache_ok = True

table = Table(
"autoinctable",
Expand Down Expand Up @@ -2693,6 +2694,7 @@ def define_tables(cls, metadata):

class SpecialType(sqltypes.TypeDecorator):
impl = String
cache_ok = True

def process_bind_param(self, value, dialect):
return value + " processed"
Expand Down
3 changes: 3 additions & 0 deletions test/engine/test_execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ class NonStandardException(OperationalError):
def test_exception_wrapping_non_dbapi_statement(self):
class MyType(TypeDecorator):
impl = Integer
cache_ok = True

def process_bind_param(self, value, dialect):
raise SomeException("nope")
Expand Down Expand Up @@ -539,6 +540,7 @@ class MyException(Exception, tsa.exc.DontWrapMixin):

class MyType(TypeDecorator):
impl = Integer
cache_ok = True

def process_bind_param(self, value, dialect):
raise MyException("nope")
Expand Down Expand Up @@ -2575,6 +2577,7 @@ def test_exception_event_ad_hoc_context(self):

class MyType(TypeDecorator):
impl = Integer
cache_ok = True

def process_bind_param(self, value, dialect):
raise nope
Expand Down
1 change: 1 addition & 0 deletions test/ext/mypy/files/type_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

class IntToStr(TypeDecorator[int]):
impl = String
cache_ok = True

def process_bind_param(
self,
Expand Down
Loading

0 comments on commit 6967b45

Please sign in to comment.