From 35c178c405c44798810ceac540faf8385b4632c4 Mon Sep 17 00:00:00 2001 From: Mike Bayer Date: Fri, 5 Jul 2024 09:30:10 -0400 Subject: [PATCH] alter the collation of string type for collate() Fixed issue where the :func:`_sql.collate` construct, which explicitly sets a collation for a given expression, would maintain collation settings for the underlying type object from the expression, causing SQL expressions to have both collations stated at once when used in further expressions for specific dialects that render explicit type casts, such as that of asyncpg. The :func:`_sql.collate` construct now assigns its own type to explicitly include the new collation, assuming it's a string type. Fixes: #11576 Change-Id: I6fc8904d2bcbc21f11bbca57e4a451ed0edbd879 --- doc/build/changelog/unreleased_20/11576.rst | 11 +++ lib/sqlalchemy/sql/elements.py | 12 ++- lib/sqlalchemy/sql/sqltypes.py | 5 ++ lib/sqlalchemy/sql/type_api.py | 29 +++++-- test/sql/test_types.py | 88 +++++++++++++++++++++ 5 files changed, 137 insertions(+), 8 deletions(-) create mode 100644 doc/build/changelog/unreleased_20/11576.rst diff --git a/doc/build/changelog/unreleased_20/11576.rst b/doc/build/changelog/unreleased_20/11576.rst new file mode 100644 index 0000000000..93cfe3bf03 --- /dev/null +++ b/doc/build/changelog/unreleased_20/11576.rst @@ -0,0 +1,11 @@ +.. change:: + :tags: bug, postgresql + :tickets: 11576 + + Fixed issue where the :func:`_sql.collate` construct, which explicitly sets + a collation for a given expression, would maintain collation settings for + the underlying type object from the expression, causing SQL expressions to + have both collations stated at once when used in further expressions for + specific dialects that render explicit type casts, such as that of asyncpg. + The :func:`_sql.collate` construct now assigns its own type to explicitly + include the new collation, assuming it's a string type. diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index a4841e07f3..56b937726e 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -5140,15 +5140,25 @@ class CollationClause(ColumnElement[str]): ] @classmethod + @util.preload_module("sqlalchemy.sql.sqltypes") def _create_collation_expression( cls, expression: _ColumnExpressionArgument[str], collation: str ) -> BinaryExpression[str]: + + sqltypes = util.preloaded.sql_sqltypes + expr = coercions.expect(roles.ExpressionElementRole[str], expression) + + if expr.type._type_affinity is sqltypes.String: + collate_type = expr.type._with_collation(collation) + else: + collate_type = expr.type + return BinaryExpression( expr, CollationClause(collation), operators.collate, - type_=expr.type, + type_=collate_type, ) def __init__(self, collation): diff --git a/lib/sqlalchemy/sql/sqltypes.py b/lib/sqlalchemy/sql/sqltypes.py index 8e559be0b7..8bd036551c 100644 --- a/lib/sqlalchemy/sql/sqltypes.py +++ b/lib/sqlalchemy/sql/sqltypes.py @@ -218,6 +218,11 @@ class String(Concatenable, TypeEngine[str]): self.length = length self.collation = collation + def _with_collation(self, collation): + new_type = self.copy() + new_type.collation = collation + return new_type + def _resolve_for_literal(self, value): # I was SO PROUD of my regex trick, but we dont need it. # re.search(r"[^\u0000-\u007F]", value) diff --git a/lib/sqlalchemy/sql/type_api.py b/lib/sqlalchemy/sql/type_api.py index 38f96780c2..3367aab64c 100644 --- a/lib/sqlalchemy/sql/type_api.py +++ b/lib/sqlalchemy/sql/type_api.py @@ -755,6 +755,10 @@ class TypeEngine(Visitable, Generic[_T]): return self + def _with_collation(self, collation: str) -> Self: + """set up error handling for the collate expression""" + raise NotImplementedError("this datatype does not support collation") + @util.ro_memoized_property def _type_affinity(self) -> Optional[Type[TypeEngine[_T]]]: """Return a rudimental 'affinity' value expressing the general class @@ -1732,6 +1736,16 @@ class TypeDecorator(SchemaEventTarget, ExternalType, TypeEngine[_T]): {}, ) + def _copy_with_check(self) -> Self: + tt = self.copy() + if not isinstance(tt, self.__class__): + raise AssertionError( + "Type object %s does not properly " + "implement the copy() method, it must " + "return an object of type %s" % (self, self.__class__) + ) + return tt + def _gen_dialect_impl(self, dialect: Dialect) -> TypeEngine[_T]: if dialect.name in self._variant_mapping: adapted = dialect.type_descriptor( @@ -1746,16 +1760,17 @@ class TypeDecorator(SchemaEventTarget, ExternalType, TypeEngine[_T]): # to a copy of this TypeDecorator and return # that. typedesc = self.load_dialect_impl(dialect).dialect_impl(dialect) - tt = self.copy() - if not isinstance(tt, self.__class__): - raise AssertionError( - "Type object %s does not properly " - "implement the copy() method, it must " - "return an object of type %s" % (self, self.__class__) - ) + tt = self._copy_with_check() tt.impl = tt.impl_instance = typedesc return tt + def _with_collation(self, collation: str) -> Self: + tt = self._copy_with_check() + tt.impl = tt.impl_instance = self.impl_instance._with_collation( + collation + ) + return tt + @util.ro_non_memoized_property def _type_affinity(self) -> Optional[Type[TypeEngine[Any]]]: return self.impl_instance._type_affinity diff --git a/test/sql/test_types.py b/test/sql/test_types.py index 36c6a74c27..999919c5f5 100644 --- a/test/sql/test_types.py +++ b/test/sql/test_types.py @@ -19,6 +19,7 @@ from sqlalchemy import Boolean from sqlalchemy import cast from sqlalchemy import CHAR from sqlalchemy import CLOB +from sqlalchemy import collate from sqlalchemy import DATE from sqlalchemy import Date from sqlalchemy import DATETIME @@ -66,9 +67,11 @@ import sqlalchemy.dialects.mysql as mysql import sqlalchemy.dialects.oracle as oracle import sqlalchemy.dialects.postgresql as pg from sqlalchemy.engine import default +from sqlalchemy.engine import interfaces from sqlalchemy.schema import AddConstraint from sqlalchemy.schema import CheckConstraint from sqlalchemy.sql import column +from sqlalchemy.sql import compiler from sqlalchemy.sql import ddl from sqlalchemy.sql import elements from sqlalchemy.sql import null @@ -3365,6 +3368,91 @@ class ExpressionTest( ], ) + @testing.fixture + def renders_bind_cast(self): + class MyText(Text): + render_bind_cast = True + + class MyCompiler(compiler.SQLCompiler): + def render_bind_cast(self, type_, dbapi_type, sqltext): + return f"""{sqltext}->BINDCAST->[{ + self.dialect.type_compiler_instance.process( + dbapi_type, identifier_preparer=self.preparer + ) + }]""" + + class MyDialect(default.DefaultDialect): + bind_typing = interfaces.BindTyping.RENDER_CASTS + colspecs = {Text: MyText} + statement_compiler = MyCompiler + + return MyDialect() + + @testing.combinations( + (lambda c1: c1.like("qpr"), "q LIKE :q_1->BINDCAST->[TEXT]"), + ( + lambda c2: c2.like("qpr"), + 'q LIKE :q_1->BINDCAST->[TEXT COLLATE "xyz"]', + ), + ( + # new behavior, a type with no collation passed into collate() + # now has a new type with that collation, so we get the collate + # on the right side bind-cast. previous to #11576 we'd only + # get TEXT for the bindcast. + lambda c1: collate(c1, "abc").like("qpr"), + '(q COLLATE abc) LIKE :param_1->BINDCAST->[TEXT COLLATE "abc"]', + ), + ( + lambda c2: collate(c2, "abc").like("qpr"), + '(q COLLATE abc) LIKE :param_1->BINDCAST->[TEXT COLLATE "abc"]', + ), + argnames="testcase,expected", + ) + @testing.variation("use_type_decorator", [True, False]) + def test_collate_type_interaction( + self, renders_bind_cast, testcase, expected, use_type_decorator + ): + """test #11576. + + This involves dialects that use the render_bind_cast feature only, + currently asycnpg and psycopg. However, the implementation of the + feature is mostly in Core, so a fixture dialect / compiler is used so + that the test is agnostic of those dialects. + + """ + + if use_type_decorator: + + class MyTextThing(TypeDecorator): + cache_ok = True + impl = Text + + c1 = Column("q", MyTextThing()) + c2 = Column("q", MyTextThing(collation="xyz")) + else: + c1 = Column("q", Text()) + c2 = Column("q", Text(collation="xyz")) + + expr = testing.resolve_lambda(testcase, c1=c1, c2=c2) + if use_type_decorator: + assert isinstance(expr.left.type, MyTextThing) + self.assert_compile(expr, expected, dialect=renders_bind_cast) + + # original types still work, have not been modified + eq_(c1.type.collation, None) + eq_(c2.type.collation, "xyz") + + self.assert_compile( + c1.like("qpr"), + "q LIKE :q_1->BINDCAST->[TEXT]", + dialect=renders_bind_cast, + ) + self.assert_compile( + c2.like("qpr"), + 'q LIKE :q_1->BINDCAST->[TEXT COLLATE "xyz"]', + dialect=renders_bind_cast, + ) + def test_bind_adapt(self, connection): # test an untyped bind gets the left side's type -- 2.47.2