From 4fdcac30accb54725f05895cb5240af0bb98cb64 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Thu, 23 Mar 2023 21:50:47 -0700 Subject: [PATCH v9 1/5] Fix error inconsistency in older ICU versions. To support older ICU versions, we rely on icu_set_collation_attributes() to do error checking that is handled directly by ucol_open() in newer ICU versions. Commit 3b50275b12 introduced a slight inconsistency, where the error report includes the fixed-up locale string, rather than the locale string passed to pg_ucol_open(). Refactor slightly so that pg_ucol_open() handles the errors from both ucol_open() and icu_set_collation_attributes(), making it easier to see any differences between the error reports. It also makes pg_ucol_open() responsible for closing the UCollator on error, which seems like the right place. Discussion: https://postgr.es/m/04182066-7655-344a-b8b7-040b1b2490fb%40enterprisedb.com Reviewed-by: Peter Eisentraut --- src/backend/utils/adt/pg_locale.c | 60 +++++++++++++++++-------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index 386768ee76..3db27b34ba 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -147,7 +147,8 @@ static size_t uchar_length(UConverter *converter, static int32_t uchar_convert(UConverter *converter, UChar *dest, int32_t destlen, const char *str, int32_t srclen); -static void icu_set_collation_attributes(UCollator *collator, const char *loc); +static void icu_set_collation_attributes(UCollator *collator, const char *loc, + UErrorCode *status); #endif /* @@ -2503,6 +2504,7 @@ pg_ucol_open(const char *loc_str) { UCollator *collator; UErrorCode status; + const char *orig_str = loc_str; char *fixed_str = NULL; /* @@ -2551,11 +2553,27 @@ pg_ucol_open(const char *loc_str) collator = ucol_open(loc_str, &status); if (U_FAILURE(status)) ereport(ERROR, + /* use original string for error report */ (errmsg("could not open collator for locale \"%s\": %s", - loc_str, u_errorName(status)))); + orig_str, u_errorName(status)))); if (U_ICU_VERSION_MAJOR_NUM < 54) - icu_set_collation_attributes(collator, loc_str); + { + status = U_ZERO_ERROR; + icu_set_collation_attributes(collator, loc_str, &status); + + /* + * Pretend the error came from ucol_open(), for consistent error + * message across ICU versions. + */ + if (U_FAILURE(status)) + { + ucol_close(collator); + ereport(ERROR, + (errmsg("could not open collator for locale \"%s\": %s", + orig_str, u_errorName(status)))); + } + } if (fixed_str != NULL) pfree(fixed_str); @@ -2705,9 +2723,9 @@ icu_from_uchar(char **result, const UChar *buff_uchar, int32_t len_uchar) */ pg_attribute_unused() static void -icu_set_collation_attributes(UCollator *collator, const char *loc) +icu_set_collation_attributes(UCollator *collator, const char *loc, + UErrorCode *status) { - UErrorCode status; int32_t len; char *icu_locale_id; char *lower_str; @@ -2720,15 +2738,15 @@ icu_set_collation_attributes(UCollator *collator, const char *loc) * locale ID, e.g. "und@colcaselevel=yes;colstrength=primary", by * uloc_canonicalize(). */ - status = U_ZERO_ERROR; - len = uloc_canonicalize(loc, NULL, 0, &status); + *status = U_ZERO_ERROR; + len = uloc_canonicalize(loc, NULL, 0, status); icu_locale_id = palloc(len + 1); - status = U_ZERO_ERROR; - len = uloc_canonicalize(loc, icu_locale_id, len + 1, &status); - if (U_FAILURE(status)) + *status = U_ZERO_ERROR; + len = uloc_canonicalize(loc, icu_locale_id, len + 1, status); + if (U_FAILURE(*status)) ereport(ERROR, (errmsg("canonicalization failed for locale string \"%s\": %s", - loc, u_errorName(status)))); + loc, u_errorName(*status)))); lower_str = asc_tolower(icu_locale_id, strlen(icu_locale_id)); @@ -2750,7 +2768,7 @@ icu_set_collation_attributes(UCollator *collator, const char *loc) UColAttribute uattr; UColAttributeValue uvalue; - status = U_ZERO_ERROR; + *status = U_ZERO_ERROR; *e = '\0'; name = token; @@ -2800,22 +2818,12 @@ icu_set_collation_attributes(UCollator *collator, const char *loc) else if (strcmp(value, "upper") == 0) uvalue = UCOL_UPPER_FIRST; else - status = U_ILLEGAL_ARGUMENT_ERROR; - - if (status == U_ZERO_ERROR) - ucol_setAttribute(collator, uattr, uvalue, &status); - - /* - * Pretend the error came from ucol_open(), for consistent error - * message across ICU versions. - */ - if (U_FAILURE(status)) { - ucol_close(collator); - ereport(ERROR, - (errmsg("could not open collator for locale \"%s\": %s", - loc, u_errorName(status)))); + *status = U_ILLEGAL_ARGUMENT_ERROR; + break; } + + ucol_setAttribute(collator, uattr, uvalue, status); } } -- 2.34.1