From: Mike Bayer Date: Thu, 23 Mar 2017 19:11:03 +0000 (-0400) Subject: Treat collation names as identifiers X-Git-Tag: rel_1_2_0b1~136 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0883d8213bcfbeb5e0ae6dd1cbcf70494eb06dac;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Treat collation names as identifiers The expression used for COLLATE as rendered by the column-level :func:`.expression.collate` and :meth:`.ColumnOperators.collate` is now quoted as an identifier when the name is case sensitive, e.g. has uppercase characters. Note that this does not impact type-level collation, which is already quoted. Change-Id: I83d5d9cd1e66a4f20b96303bb84c5f360d5d6a1a Fixes: #3785 --- diff --git a/doc/build/changelog/changelog_12.rst b/doc/build/changelog/changelog_12.rst index 9e0f558994..834dc074de 100644 --- a/doc/build/changelog/changelog_12.rst +++ b/doc/build/changelog/changelog_12.rst @@ -13,6 +13,20 @@ .. changelog:: :version: 1.2.0b1 + .. change:: 3785 + :tags: bug, sql + :tickets: 3785 + + The expression used for COLLATE as rendered by the column-level + :func:`.expression.collate` and :meth:`.ColumnOperators.collate` is now + quoted as an identifier when the name is case sensitive, e.g. has + uppercase characters. Note that this does not impact type-level + collation, which is already quoted. + + .. seealso:: + + :ref:`change_3785` + .. change:: 3229 :tags: feature, orm, ext :tickets: 3229 diff --git a/doc/build/changelog/migration_12.rst b/doc/build/changelog/migration_12.rst index 0fc60e9d98..9aebf6042a 100644 --- a/doc/build/changelog/migration_12.rst +++ b/doc/build/changelog/migration_12.rst @@ -578,6 +578,32 @@ warning. However, it is anticipated that most users will appreciate the :ticket:`3907` +.. _change_3785: + +The column-level COLLATE keyword now quotes the collation name +-------------------------------------------------------------- + +A bug in the :func:`.expression.collate` and :meth:`.ColumnOperators.collate` +functions, used to supply ad-hoc column collations at the statement level, +is fixed, where a case sensitive name would not be quoted:: + + stmt = select([mytable.c.x, mytable.c.y]).\ + order_by(mytable.c.somecolumn.collate("fr_FR")) + +now renders:: + + SELECT mytable.x, mytable.y, + FROM mytable ORDER BY mytable.somecolumn COLLATE "fr_FR" + +Previously, the case sensitive name `"fr_FR"` would not be quoted. Currently, +manual quoting of the "fr_FR" name is **not** detected, so applications that +are manually quoting the identifier should be adjusted. Note that this change +does not impact the use of collations at the type level (e.g. specified +on the datatype like :class:`.String` at the table level), where quoting +is already applied. + +:ticket:`3785` + Dialect Improvements and Changes - PostgreSQL ============================================= diff --git a/lib/sqlalchemy/sql/elements.py b/lib/sqlalchemy/sql/elements.py index a450efaf02..1f81293829 100644 --- a/lib/sqlalchemy/sql/elements.py +++ b/lib/sqlalchemy/sql/elements.py @@ -41,12 +41,18 @@ def collate(expression, collation): mycolumn COLLATE utf8_bin + The collation expression is also quoted if it is a case sensitive + identifer, e.g. contains uppercase characters. + + .. versionchanged:: 1.2 quoting is automatically applied to COLLATE + expressions if they are case sensitive. + """ expr = _literal_as_binds(expression) return BinaryExpression( expr, - _literal_as_text(collation), + ColumnClause(collation), operators.collate, type_=expr.type) diff --git a/lib/sqlalchemy/sql/operators.py b/lib/sqlalchemy/sql/operators.py index 8f697b27eb..49642acdd5 100644 --- a/lib/sqlalchemy/sql/operators.py +++ b/lib/sqlalchemy/sql/operators.py @@ -695,7 +695,13 @@ class ColumnOperators(Operators): def collate(self, collation): """Produce a :func:`~.expression.collate` clause against - the parent object, given the collation string.""" + the parent object, given the collation string. + + .. seealso:: + + :func:`~.expression.collate` + + """ return self.operate(collate, collation) def __radd__(self, other): diff --git a/test/orm/test_query.py b/test/orm/test_query.py index ce119cf502..9924c9547c 100644 --- a/test/orm/test_query.py +++ b/test/orm/test_query.py @@ -1396,9 +1396,9 @@ class OperatorTest(QueryTest, AssertsCompiledSQL): def test_collate(self): User = self.classes.User - self._test(collate(User.id, 'binary'), "users.id COLLATE binary") + self._test(collate(User.id, 'utf8_bin'), "users.id COLLATE utf8_bin") - self._test(User.id.collate('binary'), "users.id COLLATE binary") + self._test(User.id.collate('utf8_bin'), "users.id COLLATE utf8_bin") def test_selfref_between(self): User = self.classes.User diff --git a/test/sql/test_operators.py b/test/sql/test_operators.py index 0e0a8b29c3..7c3ce13898 100644 --- a/test/sql/test_operators.py +++ b/test/sql/test_operators.py @@ -1389,19 +1389,19 @@ class OperatorPrecedenceTest(fixtures.TestBase, testing.AssertsCompiledSQL): def test_operator_precedence_collate_1(self): self.assert_compile( self.table1.c.name == literal('foo').collate('utf-8'), - "mytable.name = (:param_1 COLLATE utf-8)" + 'mytable.name = (:param_1 COLLATE "utf-8")' ) def test_operator_precedence_collate_2(self): self.assert_compile( (self.table1.c.name == literal('foo')).collate('utf-8'), - "mytable.name = :param_1 COLLATE utf-8" + 'mytable.name = :param_1 COLLATE "utf-8"' ) def test_operator_precedence_collate_3(self): self.assert_compile( self.table1.c.name.collate('utf-8') == 'foo', - "(mytable.name COLLATE utf-8) = :param_1" + '(mytable.name COLLATE "utf-8") = :param_1' ) def test_operator_precedence_collate_4(self): @@ -1410,8 +1410,8 @@ class OperatorPrecedenceTest(fixtures.TestBase, testing.AssertsCompiledSQL): (self.table1.c.name == literal('foo')).collate('utf-8'), (self.table2.c.field == literal('bar')).collate('utf-8'), ), - "mytable.name = :param_1 COLLATE utf-8 " - "AND op.field = :param_2 COLLATE utf-8" + 'mytable.name = :param_1 COLLATE "utf-8" ' + 'AND op.field = :param_2 COLLATE "utf-8"' ) def test_operator_precedence_collate_5(self): @@ -1419,7 +1419,7 @@ class OperatorPrecedenceTest(fixtures.TestBase, testing.AssertsCompiledSQL): select([self.table1.c.name]).order_by( self.table1.c.name.collate('utf-8').desc()), "SELECT mytable.name FROM mytable " - "ORDER BY mytable.name COLLATE utf-8 DESC" + 'ORDER BY mytable.name COLLATE "utf-8" DESC' ) def test_operator_precedence_collate_6(self): @@ -1427,7 +1427,7 @@ class OperatorPrecedenceTest(fixtures.TestBase, testing.AssertsCompiledSQL): select([self.table1.c.name]).order_by( self.table1.c.name.collate('utf-8').desc().nullslast()), "SELECT mytable.name FROM mytable " - "ORDER BY mytable.name COLLATE utf-8 DESC NULLS LAST" + 'ORDER BY mytable.name COLLATE "utf-8" DESC NULLS LAST' ) def test_operator_precedence_collate_7(self): @@ -1435,7 +1435,7 @@ class OperatorPrecedenceTest(fixtures.TestBase, testing.AssertsCompiledSQL): select([self.table1.c.name]).order_by( self.table1.c.name.collate('utf-8').asc()), "SELECT mytable.name FROM mytable " - "ORDER BY mytable.name COLLATE utf-8 ASC" + 'ORDER BY mytable.name COLLATE "utf-8" ASC' ) def test_commutative_operators(self): diff --git a/test/sql/test_quote.py b/test/sql/test_quote.py index 94f9d62ac4..a436dde670 100644 --- a/test/sql/test_quote.py +++ b/test/sql/test_quote.py @@ -454,6 +454,23 @@ class QuoteTest(fixtures.TestBase, AssertsCompiledSQL): 'SELECT t1.col1 AS "ShouldQuote" FROM t1 ORDER BY "ShouldQuote"' ) + def test_collate(self): + self.assert_compile( + column('foo').collate('utf8'), + "foo COLLATE utf8" + ) + + self.assert_compile( + column('foo').collate('fr_FR'), + 'foo COLLATE "fr_FR"' + ) + + self.assert_compile( + column('foo').collate('utf8_GERMAN_ci'), + 'foo COLLATE `utf8_GERMAN_ci`', + dialect="mysql" + ) + def test_join(self): # Lower case names, should not quote metadata = MetaData()