don't cache TypeDecorator by default

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

10 files changed, 171 insertions, 0 deletions

diff --git a/test/sql/test_compare.py b/test/sql/test_compare.py
index 21a349d76..257776c50 100644
--- a/test/sql/test_compare.py
+++ b/test/sql/test_compare.py
@@ -23,6 +23,7 @@ from sqlalchemy import table
 from sqlalchemy import testing
 from sqlalchemy import text
 from sqlalchemy import tuple_
+from sqlalchemy import TypeDecorator
 from sqlalchemy import union
 from sqlalchemy import union_all
 from sqlalchemy import util
@@ -74,6 +75,7 @@ from sqlalchemy.testing import is_false
 from sqlalchemy.testing import is_not
 from sqlalchemy.testing import is_true
 from sqlalchemy.testing import ne_
+from sqlalchemy.testing.assertions import expect_warnings
 from sqlalchemy.testing.util import random_choices
 from sqlalchemy.types import ARRAY
 from sqlalchemy.types import JSON
@@ -144,6 +146,25 @@ dml.Update.argument_for("sqlite", "foo", None)
 dml.Delete.argument_for("sqlite", "foo", None)
 
 
+class MyType1(TypeDecorator):
+    cache_ok = True
+    impl = String
+
+
+class MyType2(TypeDecorator):
+    cache_ok = True
+    impl = Integer
+
+
+class MyType3(TypeDecorator):
+    impl = Integer
+
+    cache_ok = True
+
+    def __init__(self, arg):
+        self.arg = arg
+
+
 class CoreFixtures(object):
     # lambdas which return a tuple of ColumnElement objects.
     # must return at least two objects that should compare differently.
@@ -684,6 +705,20 @@ class CoreFixtures(object):
         lambda: (table_a, table_b),
     ]
 
+    type_cache_key_fixtures = [
+        lambda: (
+            column("q") == column("x"),
+            column("q") == column("y"),
+            column("z") == column("x"),
+            column("z", String(50)) == column("x", String(50)),
+            column("z", String(50)) == column("x", String(30)),
+            column("z", String(50)) == column("x", Integer),
+            column("z", MyType1()) == column("x", MyType2()),
+            column("z", MyType1()) == column("x", MyType3("x")),
+            column("z", MyType1()) == column("x", MyType3("y")),
+        )
+    ]
+
     dont_compare_values_fixtures = [
         lambda: (
             # note the in_(...) all have different column names because
@@ -1126,6 +1161,7 @@ class CacheKeyTest(CacheKeyFixture, CoreFixtures, fixtures.TestBase):
         for fixtures_, compare_values in [
             (self.fixtures, True),
             (self.dont_compare_values_fixtures, False),
+            (self.type_cache_key_fixtures, False),
         ]:
             for fixture in fixtures_:
                 self._run_cache_key_fixture(fixture, compare_values)
@@ -1669,3 +1705,78 @@ class ExecutableFlagsTest(fixtures.TestBase):
             is_true(case.is_select)
         else:
             is_false(case.is_select)
+
+
+class TypesTest(fixtures.TestBase):
+    def test_typedec_no_cache(self):
+        class MyType(TypeDecorator):
+            impl = String
+
+        expr = column("q", MyType()) == 1
+
+        with expect_warnings(
+            r"TypeDecorator MyType\(\) will not produce a cache key"
+        ):
+            is_(expr._generate_cache_key(), None)
+
+    def test_typedec_cache_false(self):
+        class MyType(TypeDecorator):
+            impl = String
+
+            cache_ok = False
+
+        expr = column("q", MyType()) == 1
+
+        is_(expr._generate_cache_key(), None)
+
+    def test_typedec_cache_ok(self):
+        class MyType(TypeDecorator):
+            impl = String
+
+            cache_ok = True
+
+        def go1():
+            expr = column("q", MyType()) == 1
+            return expr
+
+        def go2():
+            expr = column("p", MyType()) == 1
+            return expr
+
+        c1 = go1()._generate_cache_key()[0]
+        c2 = go1()._generate_cache_key()[0]
+        c3 = go2()._generate_cache_key()[0]
+
+        eq_(c1, c2)
+        ne_(c1, c3)
+
+    def test_typedec_cache_ok_params(self):
+        class MyType(TypeDecorator):
+            impl = String
+
+            cache_ok = True
+
+            def __init__(self, p1, p2):
+                self.p1 = p1
+                self._p2 = p2
+
+        def go1():
+            expr = column("q", MyType("x", "y")) == 1
+            return expr
+
+        def go2():
+            expr = column("q", MyType("q", "y")) == 1
+            return expr
+
+        def go3():
+            expr = column("q", MyType("x", "z")) == 1
+            return expr
+
+        c1 = go1()._generate_cache_key()[0]
+        c2 = go1()._generate_cache_key()[0]
+        c3 = go2()._generate_cache_key()[0]
+        c4 = go3()._generate_cache_key()[0]
+
+        eq_(c1, c2)
+        ne_(c1, c3)
+        eq_(c1, c4)
diff --git a/test/sql/test_defaults.py b/test/sql/test_defaults.py
index 007dc157b..ef924e068 100644
--- a/test/sql/test_defaults.py
+++ b/test/sql/test_defaults.py
@@ -407,6 +407,7 @@ class DefaultRoundTripTest(fixtures.TablesTest):
 
         class MyType(TypeDecorator):
             impl = String(50)
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 if value is not None:
@@ -1084,6 +1085,7 @@ class AutoIncrementTest(fixtures.TestBase):
     def test_autoinc_detection_no_affinity(self):
         class MyType(TypeDecorator):
             impl = TypeEngine
+            cache_ok = True
 
         assert MyType()._type_affinity is None
         t = Table("x", MetaData(), Column("id", MyType(), primary_key=True))
@@ -1212,6 +1214,8 @@ class SpecialTypePKTest(fixtures.TestBase):
         class MyInteger(TypeDecorator):
             impl = Integer
 
+            cache_ok = True
+
             def process_bind_param(self, value, dialect):
                 if value is None:
                     return None
diff --git a/test/sql/test_metadata.py b/test/sql/test_metadata.py
index 90da50875..9e0253052 100644
--- a/test/sql/test_metadata.py
+++ b/test/sql/test_metadata.py
@@ -2085,6 +2085,7 @@ class SchemaTypeTest(fixtures.TestBase):
 
     class MyTypeDecAndSchema(TypeDecorator, sqltypes.SchemaType):
         impl = String()
+        cache_ok = True
 
         evt_targets = ()
 
@@ -2114,6 +2115,7 @@ class SchemaTypeTest(fixtures.TestBase):
 
         class MyType(TypeDecorator):
             impl = target_typ
+            cache_ok = True
 
         typ = MyType()
         self._test_before_parent_attach(typ, target_typ)
@@ -2129,6 +2131,7 @@ class SchemaTypeTest(fixtures.TestBase):
     def test_before_parent_attach_typedec_of_schematype(self):
         class MyType(TypeDecorator, sqltypes.SchemaType):
             impl = String
+            cache_ok = True
 
         typ = MyType()
         self._test_before_parent_attach(typ)
@@ -2136,6 +2139,7 @@ class SchemaTypeTest(fixtures.TestBase):
     def test_before_parent_attach_schematype_of_typedec(self):
         class MyType(sqltypes.SchemaType, TypeDecorator):
             impl = String
+            cache_ok = True
 
         typ = MyType()
         self._test_before_parent_attach(typ)
@@ -2243,6 +2247,7 @@ class SchemaTypeTest(fixtures.TestBase):
     def test_to_metadata_copy_decorated(self):
         class MyDecorated(TypeDecorator):
             impl = self.MyType
+            cache_ok = True
 
         m1 = MetaData()
 
diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py
index 8fe802bf3..932d30742 100644
--- a/test/sql/test_operators.py
+++ b/test/sql/test_operators.py
@@ -500,6 +500,7 @@ class TypeDecoratorComparatorTest(_CustomComparatorTests, fixtures.TestBase):
     def _add_override_factory(self):
         class MyInteger(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             class comparator_factory(TypeDecorator.Comparator):
                 def __init__(self, expr):
@@ -520,6 +521,7 @@ class TypeDecoratorTypeDecoratorComparatorTest(
     def _add_override_factory(self):
         class MyIntegerOne(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             class comparator_factory(TypeDecorator.Comparator):
                 def __init__(self, expr):
@@ -533,6 +535,7 @@ class TypeDecoratorTypeDecoratorComparatorTest(
 
         class MyIntegerTwo(TypeDecorator):
             impl = MyIntegerOne
+            cache_ok = True
 
         return MyIntegerTwo
 
@@ -556,6 +559,7 @@ class TypeDecoratorWVariantComparatorTest(
 
         class MyInteger(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             class comparator_factory(TypeDecorator.Comparator):
                 def __init__(self, expr):
@@ -587,6 +591,7 @@ class CustomEmbeddedinTypeDecoratorTest(
 
         class MyDecInteger(TypeDecorator):
             impl = MyInteger
+            cache_ok = True
 
         return MyDecInteger
 
diff --git a/test/sql/test_query.py b/test/sql/test_query.py
index 33245bfbc..a22cf1098 100644
--- a/test/sql/test_query.py
+++ b/test/sql/test_query.py
@@ -353,6 +353,7 @@ class QueryTest(fixtures.TablesTest):
 
         class MyInteger(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return int(value[4:])
@@ -783,6 +784,7 @@ class QueryTest(fixtures.TablesTest):
 
         class NameWithProcess(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return value[3:]
diff --git a/test/sql/test_resultset.py b/test/sql/test_resultset.py
index 2054b3cf1..44422257a 100644
--- a/test/sql/test_resultset.py
+++ b/test/sql/test_resultset.py
@@ -978,18 +978,21 @@ class CursorResultTest(fixtures.TablesTest):
 
         class Goofy1(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_result_value(self, value, dialect):
                 return value + "a"
 
         class Goofy2(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_result_value(self, value, dialect):
                 return value + "b"
 
         class Goofy3(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_result_value(self, value, dialect):
                 return value + "c"
@@ -2527,6 +2530,7 @@ class AlternateCursorResultTest(fixtures.TablesTest):
     def _test_result_processor(self, cls, use_cache):
         class MyType(TypeDecorator):
             impl = String()
+            cache_ok = True
 
             def process_result_value(self, value, dialect):
                 return "HI " + value
diff --git a/test/sql/test_returning.py b/test/sql/test_returning.py
index 62d9ab75e..a0d69e782 100644
--- a/test/sql/test_returning.py
+++ b/test/sql/test_returning.py
@@ -100,6 +100,7 @@ class ReturningTest(fixtures.TablesTest, AssertsExecutionResults):
     def define_tables(cls, metadata):
         class GoofyType(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 if value is None:
@@ -386,6 +387,7 @@ class CompositeStatementTest(fixtures.TestBase):
     def test_select_doesnt_pollute_result(self, connection):
         class MyType(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             def process_result_value(self, value, dialect):
                 raise Exception("I have not been selected")
diff --git a/test/sql/test_selectable.py b/test/sql/test_selectable.py
index b54ef02fd..add07e013 100644
--- a/test/sql/test_selectable.py
+++ b/test/sql/test_selectable.py
@@ -604,6 +604,7 @@ class SelectableTest(
     def test_type_coerce_preserve_subq(self):
         class MyType(TypeDecorator):
             impl = Integer
+            cache_ok = True
 
         stmt = select(type_coerce(column("x"), MyType).label("foo"))
         subq = stmt.subquery()
diff --git a/test/sql/test_type_expressions.py b/test/sql/test_type_expressions.py
index 5f278fb55..c4ed8121e 100644
--- a/test/sql/test_type_expressions.py
+++ b/test/sql/test_type_expressions.py
@@ -37,6 +37,7 @@ class _ExprFixture(object):
     def _type_decorator_outside_fixture(self):
         class MyString(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def bind_expression(self, bindvalue):
                 return func.outside_bind(bindvalue)
@@ -56,6 +57,7 @@ class _ExprFixture(object):
 
         class MyString(TypeDecorator):
             impl = MyInsideString
+            cache_ok = True
 
         return self._test_table(MyString)
 
@@ -69,6 +71,7 @@ class _ExprFixture(object):
 
         class MyString(TypeDecorator):
             impl = String
+            cache_ok = True
 
             # this works because when the compiler calls dialect_impl(),
             # a copy of MyString is created which has just this impl
@@ -427,6 +430,7 @@ class TypeDecRoundTripTest(fixtures.TablesTest, RoundTripTestBase):
     def define_tables(cls, metadata):
         class MyString(TypeDecorator):
             impl = String
+            cache_ok = True
 
             def bind_expression(self, bindvalue):
                 return func.lower(bindvalue)
diff --git a/test/sql/test_types.py b/test/sql/test_types.py
index e63197ae2..9db0fee3b 100644
--- a/test/sql/test_types.py
+++ b/test/sql/test_types.py
@@ -375,6 +375,7 @@ class TypeAffinityTest(fixtures.TestBase):
 
         class MyType(TypeDecorator):
             impl = CHAR
+            cache_ok = True
 
             def load_dialect_impl(self, dialect):
                 if dialect.name == "postgresql":
@@ -504,6 +505,7 @@ class _UserDefinedTypeFixture(object):
 
         class MyDecoratedType(types.TypeDecorator):
             impl = String
+            cache_ok = True
 
             def bind_processor(self, dialect):
                 impl_processor = super(MyDecoratedType, self).bind_processor(
@@ -530,6 +532,7 @@ class _UserDefinedTypeFixture(object):
 
         class MyNewUnicodeType(types.TypeDecorator):
             impl = Unicode
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return "BIND_IN" + value
@@ -542,6 +545,7 @@ class _UserDefinedTypeFixture(object):
 
         class MyNewIntType(types.TypeDecorator):
             impl = Integer
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return value * 10
@@ -561,6 +565,7 @@ class _UserDefinedTypeFixture(object):
 
         class MyUnicodeType(types.TypeDecorator):
             impl = Unicode
+            cache_ok = True
 
             def bind_processor(self, dialect):
                 impl_processor = super(MyUnicodeType, self).bind_processor(
@@ -587,6 +592,7 @@ class _UserDefinedTypeFixture(object):
 
         class MyDecOfDec(types.TypeDecorator):
             impl = MyNewIntType
+            cache_ok = True
 
         Table(
             "users",
@@ -735,6 +741,7 @@ class UserDefinedTest(
     def test_typedecorator_literal_render(self):
         class MyType(types.TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_literal_param(self, value, dialect):
                 return "HI->%s<-THERE" % value
@@ -767,6 +774,7 @@ class UserDefinedTest(
         # value rendering.
         class MyType(types.TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return "HI->%s<-THERE" % value
@@ -796,6 +804,7 @@ class UserDefinedTest(
 
                 class MyType(types.TypeDecorator):
                     impl = impl_
+                    cache_ok = True
 
                 dec_type = MyType(**kw)
 
@@ -813,6 +822,7 @@ class UserDefinedTest(
     def test_user_defined_typedec_impl(self):
         class MyType(types.TypeDecorator):
             impl = Float
+            cache_ok = True
 
             def load_dialect_impl(self, dialect):
                 if dialect.name == "sqlite":
@@ -838,6 +848,7 @@ class UserDefinedTest(
     def test_typedecorator_schematype_constraint(self, typ):
         class B(TypeDecorator):
             impl = typ
+            cache_ok = True
 
         t1 = Table("t1", MetaData(), Column("q", B(create_constraint=True)))
         eq_(
@@ -849,6 +860,8 @@ class UserDefinedTest(
         class MyType(TypeDecorator):
             impl = VARCHAR
 
+            cache_ok = True
+
         eq_(repr(MyType(45)), "MyType(length=45)")
 
     def test_user_defined_typedec_impl_bind(self):
@@ -868,6 +881,7 @@ class UserDefinedTest(
 
         class MyType(types.TypeDecorator):
             impl = TypeOne
+            cache_ok = True
 
             def load_dialect_impl(self, dialect):
                 if dialect.name == "sqlite":
@@ -951,6 +965,7 @@ class TypeCoerceCastTest(fixtures.TablesTest):
     def define_tables(cls, metadata):
         class MyType(types.TypeDecorator):
             impl = String(50)
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return "BIND_IN" + str(value)
@@ -1252,6 +1267,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
     def test_type_decorator_variant_one_roundtrip(self, variant_roundtrip):
         class Foo(TypeDecorator):
             impl = String(50)
+            cache_ok = True
 
         if testing.against("postgresql"):
             data = [5, 6, 10]
@@ -1288,6 +1304,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
 
         class Foo(TypeDecorator):
             impl = variant
+            cache_ok = True
 
         if testing.against("postgresql"):
             data = assert_data = [5, 6, 10]
@@ -1305,6 +1322,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
     def test_type_decorator_variant_three(self, variant_roundtrip):
         class Foo(TypeDecorator):
             impl = String
+            cache_ok = True
 
         if testing.against("postgresql"):
             data = ["five", "six", "ten"]
@@ -1318,6 +1336,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
     def test_type_decorator_compile_variant_one(self):
         class Foo(TypeDecorator):
             impl = String
+            cache_ok = True
 
         self.assert_compile(
             Foo().with_variant(Integer, "sqlite"),
@@ -1356,6 +1375,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
 
         class Foo(TypeDecorator):
             impl = variant
+            cache_ok = True
 
         self.assert_compile(
             Foo().with_variant(Integer, "sqlite"),
@@ -1372,6 +1392,7 @@ class VariantBackendTest(fixtures.TestBase, AssertsCompiledSQL):
     def test_type_decorator_compile_variant_three(self):
         class Foo(TypeDecorator):
             impl = String
+            cache_ok = True
 
         self.assert_compile(
             Integer().with_variant(Foo(), "postgresql"),
@@ -2293,6 +2314,8 @@ class EnumTest(AssertsCompiledSQL, fixtures.TablesTest):
                 self.name = name
 
         class MyEnum(TypeDecorator):
+            cache_ok = True
+
             def __init__(self, values):
                 self.impl = Enum(
                     *[v.name for v in values],
@@ -2431,6 +2454,7 @@ class BinaryTest(fixtures.TablesTest, AssertsExecutionResults):
 
         class MyPickleType(types.TypeDecorator):
             impl = PickleType
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 if value:
@@ -2788,6 +2812,8 @@ class ExpressionTest(
         class MyTypeDec(types.TypeDecorator):
             impl = String
 
+            cache_ok = True
+
             def process_bind_param(self, value, dialect):
                 return "BIND_IN" + str(value)
 
@@ -2797,6 +2823,8 @@ class ExpressionTest(
         class MyDecOfDec(types.TypeDecorator):
             impl = MyTypeDec
 
+            cache_ok = True
+
         Table(
             "test",
             metadata,
@@ -2991,14 +3019,17 @@ class ExpressionTest(
         class CoerceNothing(TypeDecorator):
             coerce_to_is_types = ()
             impl = Integer
+            cache_ok = True
 
         class CoerceBool(TypeDecorator):
             coerce_to_is_types = (bool,)
             impl = Boolean
+            cache_ok = True
 
         class CoerceNone(TypeDecorator):
             coerce_to_is_types = (type(None),)
             impl = Integer
+            cache_ok = True
 
         c1 = column("x", CoerceNothing())
         c2 = column("x", CoerceBool())
@@ -3027,6 +3058,7 @@ class ExpressionTest(
     def test_typedec_righthand_coercion(self, connection):
         class MyTypeDec(types.TypeDecorator):
             impl = String
+            cache_ok = True
 
             def process_bind_param(self, value, dialect):
                 return "BIND_IN" + str(value)
@@ -3531,6 +3563,7 @@ class BooleanTest(
 
         class MyBool(TypeDecorator):
             impl = Boolean(create_constraint=True)
+            cache_ok = True
 
             # future method
             def process_literal_param(self, value, dialect):