Skip to content

Commit 6e197cb

Browse files
tglsfdctglsfdc
committed
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.
1 parent 37d6d07 commit 6e197cb

File tree

8 files changed

+105
-18
lines changed

8 files changed

+105
-18
lines changed

‎src/backend/commands/collationcmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ DefineCollation(List *names, List *parameters)
141141

142142
/* check that the locales can be loaded */
143143
CommandCounterIncrement();
144-
pg_newlocale_from_collation(newoid);
144+
(void) pg_newlocale_from_collation(newoid);
145145
}
146146

147147
/*

‎src/backend/commands/indexcmds.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
899899
if (!OidIsValid(attcollation))
900900
ereport(ERROR,
901901
(errcode(ERRCODE_INDETERMINATE_COLLATION),
902-
errmsg("no collation was derived for the index expression"),
902+
errmsg("could not determine which collation to use for index expression"),
903903
errhint("Use the COLLATE clause to set the collation explicitly.")));
904904
}
905905
else

‎src/backend/commands/view.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
139139
if (!OidIsValid(def->collOid))
140140
ereport(ERROR,
141141
(errcode(ERRCODE_INDETERMINATE_COLLATION),
142-
errmsg("no collation was derived for view column \"%s\"",
142+
errmsg("could not determine which collation to use for view column \"%s\"",
143143
def->colname),
144144
errhint("Use the COLLATE clause to set the collation explicitly.")));
145145
}

‎src/backend/utils/adt/formatting.c

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,7 +1503,20 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
15031503
size_t result_size;
15041504

15051505
if (collid != DEFAULT_COLLATION_OID)
1506+
{
1507+
if (!OidIsValid(collid))
1508+
{
1509+
/*
1510+
* This typically means that the parser could not resolve a
1511+
* conflict of implicit collations, so report it that way.
1512+
*/
1513+
ereport(ERROR,
1514+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1515+
errmsg("could not determine which collation to use for lower() function"),
1516+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1517+
}
15061518
mylocale = pg_newlocale_from_collation(collid);
1519+
}
15071520

15081521
/* Overflow paranoia */
15091522
if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t)))
@@ -1540,7 +1553,20 @@ str_tolower(const char *buff, size_t nbytes, Oid collid)
15401553
char *p;
15411554

15421555
if (collid != DEFAULT_COLLATION_OID)
1556+
{
1557+
if (!OidIsValid(collid))
1558+
{
1559+
/*
1560+
* This typically means that the parser could not resolve a
1561+
* conflict of implicit collations, so report it that way.
1562+
*/
1563+
ereport(ERROR,
1564+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1565+
errmsg("could not determine which collation to use for lower() function"),
1566+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1567+
}
15431568
mylocale = pg_newlocale_from_collation(collid);
1569+
}
15441570

15451571
result = pnstrdup(buff, nbytes);
15461572

@@ -1598,7 +1624,20 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
15981624
size_t result_size;
15991625

16001626
if (collid != DEFAULT_COLLATION_OID)
1627+
{
1628+
if (!OidIsValid(collid))
1629+
{
1630+
/*
1631+
* This typically means that the parser could not resolve a
1632+
* conflict of implicit collations, so report it that way.
1633+
*/
1634+
ereport(ERROR,
1635+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1636+
errmsg("could not determine which collation to use for upper() function"),
1637+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1638+
}
16011639
mylocale = pg_newlocale_from_collation(collid);
1640+
}
16021641

16031642
/* Overflow paranoia */
16041643
if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t)))
@@ -1635,7 +1674,20 @@ str_toupper(const char *buff, size_t nbytes, Oid collid)
16351674
char *p;
16361675

16371676
if (collid != DEFAULT_COLLATION_OID)
1677+
{
1678+
if (!OidIsValid(collid))
1679+
{
1680+
/*
1681+
* This typically means that the parser could not resolve a
1682+
* conflict of implicit collations, so report it that way.
1683+
*/
1684+
ereport(ERROR,
1685+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1686+
errmsg("could not determine which collation to use for upper() function"),
1687+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1688+
}
16381689
mylocale = pg_newlocale_from_collation(collid);
1690+
}
16391691

16401692
result = pnstrdup(buff, nbytes);
16411693

@@ -1705,7 +1757,20 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
17051757
size_t result_size;
17061758

17071759
if (collid != DEFAULT_COLLATION_OID)
1760+
{
1761+
if (!OidIsValid(collid))
1762+
{
1763+
/*
1764+
* This typically means that the parser could not resolve a
1765+
* conflict of implicit collations, so report it that way.
1766+
*/
1767+
ereport(ERROR,
1768+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1769+
errmsg("could not determine which collation to use for initcap() function"),
1770+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1771+
}
17081772
mylocale = pg_newlocale_from_collation(collid);
1773+
}
17091774

17101775
/* Overflow paranoia */
17111776
if ((nbytes + 1) > (INT_MAX / sizeof(wchar_t)))
@@ -1754,7 +1819,20 @@ str_initcap(const char *buff, size_t nbytes, Oid collid)
17541819
char *p;
17551820

17561821
if (collid != DEFAULT_COLLATION_OID)
1822+
{
1823+
if (!OidIsValid(collid))
1824+
{
1825+
/*
1826+
* This typically means that the parser could not resolve a
1827+
* conflict of implicit collations, so report it that way.
1828+
*/
1829+
ereport(ERROR,
1830+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1831+
errmsg("could not determine which collation to use for initcap() function"),
1832+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1833+
}
17571834
mylocale = pg_newlocale_from_collation(collid);
1835+
}
17581836

17591837
result = pnstrdup(buff, nbytes);
17601838

‎src/backend/utils/adt/pg_locale.c

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -932,21 +932,13 @@ pg_newlocale_from_collation(Oid collid)
932932
{
933933
collation_cache_entry *cache_entry;
934934

935+
/* Callers must pass a valid OID */
936+
Assert(OidIsValid(collid));
937+
935938
/* Return 0 for "default" collation, just in case caller forgets */
936939
if (collid == DEFAULT_COLLATION_OID)
937940
return (pg_locale_t) 0;
938941

939-
/*
940-
* This is where we'll fail if a collation-aware function is invoked
941-
* and no collation OID is passed. This typically means that the
942-
* parser could not resolve a conflict of implicit collations, so
943-
* report it that way.
944-
*/
945-
if (!OidIsValid(collid))
946-
ereport(ERROR,
947-
(errcode(ERRCODE_INDETERMINATE_COLLATION),
948-
errmsg("locale operation to be invoked, but no collation was derived")));
949-
950942
cache_entry = lookup_collation_cache(collid, false);
951943

952944
if (cache_entry->locale == 0)

‎src/backend/utils/adt/varlena.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,7 +1302,20 @@ varstr_cmp(char *arg1, int len1, char *arg2, int len2, Oid collid)
13021302
pg_locale_t mylocale = 0;
13031303

13041304
if (collid != DEFAULT_COLLATION_OID)
1305+
{
1306+
if (!OidIsValid(collid))
1307+
{
1308+
/*
1309+
* This typically means that the parser could not resolve a
1310+
* conflict of implicit collations, so report it that way.
1311+
*/
1312+
ereport(ERROR,
1313+
(errcode(ERRCODE_INDETERMINATE_COLLATION),
1314+
errmsg("could not determine which collation to use for string comparison"),
1315+
errhint("Use the COLLATE clause to set the collation explicitly.")));
1316+
}
13051317
mylocale = pg_newlocale_from_collation(collid);
1318+
}
13061319

13071320
#ifdef WIN32
13081321
/* Win32 does not have UTF-8, so we need to map to UTF-16 */

‎src/test/regress/expected/collate.linux.utf8.out

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -586,7 +586,8 @@ SELECT a, b FROM collate_test3 EXCEPT SELECT a, b FROM collate_test3 WHERE a < 2
586586
(3 rows)
587587

588588
SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3 ORDER BY 2; -- fail
589-
ERROR: locale operation to be invoked, but no collation was derived
589+
ERROR: could not determine which collation to use for string comparison
590+
HINT: Use the COLLATE clause to set the collation explicitly.
590591
SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test3; -- ok
591592
a | b
592593
---+-----
@@ -629,7 +630,8 @@ ERROR: no collation was derived for column "b" with collatable type text
629630
HINT: Use the COLLATE clause to set the collation explicitly.
630631
-- ideally this would be a parse-time error, but for now it must be run-time:
631632
select x < y from collate_test10; -- fail
632-
ERROR: locale operation to be invoked, but no collation was derived
633+
ERROR: could not determine which collation to use for string comparison
634+
HINT: Use the COLLATE clause to set the collation explicitly.
633635
select x || y from collate_test10; -- ok, because || is not collation aware
634636
?column?
635637
----------

‎src/test/regress/expected/collate.out

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,8 @@ SELECT a, b FROM collate_test2 EXCEPT SELECT a, b FROM collate_test2 WHERE a < 2
392392
(3 rows)
393393

394394
SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test2 ORDER BY 2; -- fail
395-
ERROR: locale operation to be invoked, but no collation was derived
395+
ERROR: could not determine which collation to use for string comparison
396+
HINT: Use the COLLATE clause to set the collation explicitly.
396397
SELECT a, b FROM collate_test1 UNION ALL SELECT a, b FROM collate_test2; -- ok
397398
a | b
398399
---+-----
@@ -435,7 +436,8 @@ ERROR: no collation was derived for column "b" with collatable type text
435436
HINT: Use the COLLATE clause to set the collation explicitly.
436437
-- ideally this would be a parse-time error, but for now it must be run-time:
437438
select x < y from collate_test10; -- fail
438-
ERROR: locale operation to be invoked, but no collation was derived
439+
ERROR: could not determine which collation to use for string comparison
440+
HINT: Use the COLLATE clause to set the collation explicitly.
439441
select x || y from collate_test10; -- ok, because || is not collation aware
440442
?column?
441443
----------

0 commit comments

Comments
 (0)