From 6e197cb2e537880f36828a6c55d0f6df5bf7daa8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 22 Mar 2011 16:55:32 -0400 Subject: [PATCH] Improve reporting of run-time-detected indeterminate-collation errors. pg_newlocale_from_collation does not have enough context to give an error message that's even a little bit useful, so move the responsibility for complaining up to its callers. Also, reword ERRCODE_INDETERMINATE_COLLATION error messages in a less jargony, more message-style-guide-compliant fashion. --- src/backend/commands/collationcmds.c | 2 +- src/backend/commands/indexcmds.c | 2 +- src/backend/commands/view.c | 2 +- src/backend/utils/adt/formatting.c | 78 +++++++++++++++++++ src/backend/utils/adt/pg_locale.c | 14 +--- src/backend/utils/adt/varlena.c | 13 ++++ .../regress/expected/collate.linux.utf8.out | 6 +- src/test/regress/expected/collate.out | 6 +- 8 files changed, 105 insertions(+), 18 deletions(-) diff --git a/src/backend/commands/collationcmds.c b/src/backend/commands/collationcmds.c index 6dafb7223c..2a6938fd04 100644 --- a/src/backend/commands/collationcmds.c +++ b/src/backend/commands/collationcmds.c @@ -141,7 +141,7 @@ DefineCollation(List *names, List *parameters) /* check that the locales can be loaded */ CommandCounterIncrement(); - pg_newlocale_from_collation(newoid); + (void) pg_newlocale_from_collation(newoid); } /* diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c index 163980bbfa..dafa6d5efc 100644 --- a/src/backend/commands/indexcmds.c +++ b/src/backend/commands/indexcmds.c @@ -899,7 +899,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo, if (!OidIsValid(attcollation)) ereport(ERROR, (errcode(ERRCODE_INDETERMINATE_COLLATION), - errmsg("no collation was derived for the index expression"), + errmsg("could not determine which collation to use for index expression"), errhint("Use the COLLATE clause to set the collation explicitly."))); } else diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index 250d02605c..508fb23c9a 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -139,7 +139,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace) if (!OidIsValid(def->collOid)) ereport(ERROR, (errcode(ERRCODE_INDETERMINATE_COLLATION), - errmsg("no collation was derived for view column \"%s\"", + errmsg("could not determine which collation to use for view column \"%s\"", def->colname), errhint("Use the COLLATE clause to set the collation explicitly."))); } diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c index 54783103a2..45e36f92e5 100644 --- a/src/backend/utils/adt/formatting.c +++ b/src/backend/utils/adt/formatting.c @@ -1503,7 +1503,20 @@ str_tolower(const char *buff, size_t nbytes, Oid collid) size_t result_size; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for lower() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } /* Overflow paranoia */ if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t))) @@ -1540,7 +1553,20 @@ str_tolower(const char *buff, size_t nbytes, Oid collid) char *p; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for lower() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } result = pnstrdup(buff, nbytes); @@ -1598,7 +1624,20 @@ str_toupper(const char *buff, size_t nbytes, Oid collid) size_t result_size; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for upper() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } /* Overflow paranoia */ if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t))) @@ -1635,7 +1674,20 @@ str_toupper(const char *buff, size_t nbytes, Oid collid) char *p; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for upper() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } result = pnstrdup(buff, nbytes); @@ -1705,7 +1757,20 @@ str_initcap(const char *buff, size_t nbytes, Oid collid) size_t result_size; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for initcap() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } /* Overflow paranoia */ if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t))) @@ -1754,7 +1819,20 @@ str_initcap(const char *buff, size_t nbytes, Oid collid) char *p; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for initcap() function"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } result = pnstrdup(buff, nbytes); diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index 15d347c4f8..163856d5b1 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -932,21 +932,13 @@ pg_newlocale_from_collation(Oid collid) { collation_cache_entry *cache_entry; + /* Callers must pass a valid OID */ + Assert(OidIsValid(collid)); + /* Return 0 for "default" collation, just in case caller forgets */ if (collid == DEFAULT_COLLATION_OID) return (pg_locale_t) 0; - /* - * This is where we'll fail if a collation-aware function is invoked - * and no collation OID is passed. This typically means that the - * parser could not resolve a conflict of implicit collations, so - * report it that way. - */ - if (!OidIsValid(collid)) - ereport(ERROR, - (errcode(ERRCODE_INDETERMINATE_COLLATION), - errmsg("locale operation to be invoked, but no collation was derived"))); - cache_entry = lookup_collation_cache(collid, false); if (cache_entry->locale == 0) diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 8a7a3cf45b..3587fe4595 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -1302,7 +1302,20 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid) pg_locale_t mylocale = 0; if (collid != DEFAULT_COLLATION_OID) + { + if (!OidIsValid(collid)) + { + /* + * This typically means that the parser could not resolve a + * conflict of implicit collations, so report it that way. + */ + ereport(ERROR, + (errcode(ERRCODE_INDETERMINATE_COLLATION), + errmsg("could not determine which collation to use for string comparison"), + errhint("Use the COLLATE clause to set the collation explicitly."))); + } mylocale = pg_newlocale_from_collation(collid); + } #ifdef WIN32 /* Win32 does not have UTF-8, so we need to map to UTF-16 */ diff --git a/src/test/regress/expected/collate.linux.utf8.out b/src/test/regress/expected/collate.linux.utf8.out index 0ed0b36da7..b03395b35a 100644 --- a/src/test/regress/expected/collate.linux.utf8.out +++ b/src/test/regress/expected/collate.linux.utf8.out @@ -586,7 +586,8 @@ SELECT a, b FROM collate_test3 EXCEPT SELECT a, b FROM collate_test3 WHERE a < 2 (3 rows) SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3 ORDER BY 2; -- fail -ERROR: locale operation to be invoked, but no collation was derived +ERROR: could not determine which collation to use for string comparison +HINT: Use the COLLATE clause to set the collation explicitly. SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3; -- ok a | b ---+----- @@ -629,7 +630,8 @@ ERROR: no collation was derived for column "b" with collatable type text HINT: Use the COLLATE clause to set the collation explicitly. -- ideally this would be a parse-time error, but for now it must be run-time: select x < y from collate_test10; -- fail -ERROR: locale operation to be invoked, but no collation was derived +ERROR: could not determine which collation to use for string comparison +HINT: Use the COLLATE clause to set the collation explicitly. select x || y from collate_test10; -- ok, because || is not collation aware ?column? ---------- diff --git a/src/test/regress/expected/collate.out b/src/test/regress/expected/collate.out index 4536cf2101..0ed9601bf2 100644 --- a/src/test/regress/expected/collate.out +++ b/src/test/regress/expected/collate.out @@ -392,7 +392,8 @@ SELECT a, b FROM collate_test2 EXCEPT SELECT a, b FROM collate_test2 WHERE a < 2 (3 rows) SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test2 ORDER BY 2; -- fail -ERROR: locale operation to be invoked, but no collation was derived +ERROR: could not determine which collation to use for string comparison +HINT: Use the COLLATE clause to set the collation explicitly. SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test2; -- ok a | b ---+----- @@ -435,7 +436,8 @@ ERROR: no collation was derived for column "b" with collatable type text HINT: Use the COLLATE clause to set the collation explicitly. -- ideally this would be a parse-time error, but for now it must be run-time: select x < y from collate_test10; -- fail -ERROR: locale operation to be invoked, but no collation was derived +ERROR: could not determine which collation to use for string comparison +HINT: Use the COLLATE clause to set the collation explicitly. select x || y from collate_test10; -- ok, because || is not collation aware ?column? ----------