From f97256570f45c33abf695a189ab11b25e6cd7985 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 1 Aug 2017 13:51:05 -0400 Subject: [PATCH] Allow creation of C/POSIX collations without depending on libc behavior. Most of our collations code has special handling for the locale names "C" and "POSIX", allowing those collations to be used whether or not the system libraries think those locale names are valid, or indeed whether said libraries even have any locale support. But we missed handling things that way in CREATE COLLATION. This meant you couldn't clone the C/POSIX collations, nor explicitly define a new collation using those locale names, unless the libraries allow it. That's pretty pointless, as well as being a violation of pg_newlocale_from_collation's API specification. The practical effect of this change is quite limited: it allows creating such collations even on platforms that don't HAVE_LOCALE_T, and it allows making "POSIX" collation objects on Windows, which before this would only let you make "C" collation objects. Hence, even though this is a bug fix IMO, it doesn't seem worth the trouble to back-patch. In passing, suppress the DROP CASCADE detail messages at the end of the collation regression test. I'm surprised we've never been bit by message ordering issues there. Per report from Murtuza Zabuawala. Discussion: https://postgr.es/m/CAKKotZS-wcDcofXDCH=sidiuajE+nqHn2CGjLLX78anyDmi3gQ@mail.gmail.com --- src/backend/commands/collationcmds.c | 12 +++++++---- src/test/regress/expected/collate.out | 29 ++++++++++++--------------- src/test/regress/sql/collate.sql | 12 +++++++++++ 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index b6c14c920d..d19a384f9c 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -214,11 +214,15 @@ DefineCollation(ParseState *pstate, List *names, List *parameters, bool if_not_e if (!OidIsValid(newoid)) return InvalidObjectAddress; - ObjectAddressSet(address, CollationRelationId, newoid); - - /* check that the locales can be loaded */ + /* + * Check that the locales can be loaded. NB: pg_newlocale_from_collation + * is only supposed to be called on non-C-equivalent locales. + */ CommandCounterIncrement(); - (void) pg_newlocale_from_collation(newoid); + if (!lc_collate_is_c(newoid) || !lc_ctype_is_c(newoid)) + (void) pg_newlocale_from_collation(newoid); + + ObjectAddressSet(address, CollationRelationId, newoid); return address; } diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index d667ae1714..b0025c0a87 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -622,6 +622,17 @@ EXPLAIN (COSTS OFF) -> Seq Scan on collate_test10 (3 rows) +-- CREATE/DROP COLLATION +CREATE COLLATION mycoll1 FROM "C"; +CREATE COLLATION mycoll2 ( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" ); +CREATE COLLATION mycoll3 FROM "default"; -- intentionally unsupported +ERROR: collation "default" cannot be copied +DROP COLLATION mycoll1; +CREATE TABLE collate_test23 (f1 text collate mycoll2); +DROP COLLATION mycoll2; -- fail +ERROR: cannot drop collation mycoll2 because other objects depend on it +DETAIL: table collate_test23 column f1 depends on collation mycoll2 +HINT: Use DROP ... CASCADE to drop the dependent objects too. -- 9.1 bug with useless COLLATE in an expression subject to length coercion CREATE TEMP TABLE vctable (f1 varchar(25)); INSERT INTO vctable VALUES ('foo' COLLATE "C"); @@ -650,20 +661,6 @@ SELECT collation for ((SELECT b FROM collate_test1 LIMIT 1)); -- trying to run any platform-specific collation tests later, so we -- must get rid of them. -- +\set VERBOSITY terse DROP SCHEMA collate_tests CASCADE; -NOTICE: drop cascades to 15 other objects -DETAIL: drop cascades to table collate_test1 -drop cascades to table collate_test_like -drop cascades to table collate_test2 -drop cascades to type testdomain_p -drop cascades to table collate_test4 -drop cascades to table collate_test5 -drop cascades to table collate_test10 -drop cascades to view collview1 -drop cascades to view collview2 -drop cascades to view collview3 -drop cascades to type testdomain -drop cascades to function dup(anyelement) -drop cascades to table collate_test20 -drop cascades to table collate_test21 -drop cascades to table collate_test22 +NOTICE: drop cascades to 17 other objects diff --git a/src/test/regress/sql/collate.sql b/src/test/regress/sql/collate.sql index e8ca0856e3..698f577490 100644 --- a/src/test/regress/sql/collate.sql +++ b/src/test/regress/sql/collate.sql @@ -229,6 +229,17 @@ EXPLAIN (COSTS OFF) SELECT * FROM collate_test10 ORDER BY x DESC, y COLLATE "C" ASC NULLS FIRST; +-- CREATE/DROP COLLATION + +CREATE COLLATION mycoll1 FROM "C"; +CREATE COLLATION mycoll2 ( LC_COLLATE = "POSIX", LC_CTYPE = "POSIX" ); +CREATE COLLATION mycoll3 FROM "default"; -- intentionally unsupported + +DROP COLLATION mycoll1; +CREATE TABLE collate_test23 (f1 text collate mycoll2); +DROP COLLATION mycoll2; -- fail + + -- 9.1 bug with useless COLLATE in an expression subject to length coercion CREATE TEMP TABLE vctable (f1 varchar(25)); @@ -246,4 +257,5 @@ SELECT collation for ((SELECT b FROM collate_test1 LIMIT 1)); -- trying to run any platform-specific collation tests later, so we -- must get rid of them. -- +\set VERBOSITY terse DROP SCHEMA collate_tests CASCADE;