From da761095a7e39463ec6083484c4b1b924cacf387 Mon Sep 17 00:00:00 2001 From: Marina Polyakova Date: Sat, 29 Oct 2022 14:25:05 +0300 Subject: [PATCH v1] Fix order of checking ICU options in initdb and create database First check if ICU is supported in this build. Then check that the selected encoding is supported BY ICU (the encoding can be set from lc_ctype which can be set from an environment variable). Only then check the option for the ICU locale. --- src/backend/commands/dbcommands.c | 6 ++++++ src/backend/utils/adt/pg_locale.c | 10 ++------- src/bin/initdb/initdb.c | 36 ++++++++++++++----------------- src/bin/initdb/t/001_initdb.pl | 21 ++++++++++++++---- src/bin/scripts/t/020_createdb.pl | 31 ++++++++++++++++++++++---- src/include/utils/pg_locale.h | 2 +- 6 files changed, 69 insertions(+), 37 deletions(-) diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 16e422138b..4c1e79fca3 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -1030,6 +1030,11 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (dblocprovider == COLLPROVIDER_ICU) { +#ifndef USE_ICU + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("ICU is not supported in this build"))); +#else if (!(is_encoding_supported_by_icu(encoding))) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -1046,6 +1051,7 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) errmsg("ICU locale must be specified"))); check_icu_locale(dbiculocale); +#endif } else { diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index 2b42d9ccd8..743f11e1d1 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1945,15 +1945,12 @@ icu_set_collation_attributes(UCollator *collator, const char *loc) } } -#endif /* USE_ICU */ - /* * Check if the given locale ID is valid, and ereport(ERROR) if it isn't. */ void check_icu_locale(const char *icu_locale) { -#ifdef USE_ICU UCollator *collator; UErrorCode status; @@ -1967,13 +1964,10 @@ check_icu_locale(const char *icu_locale) if (U_ICU_VERSION_MAJOR_NUM < 54) icu_set_collation_attributes(collator, icu_locale); ucol_close(collator); -#else - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("ICU is not supported in this build"))); -#endif } +#endif /* USE_ICU */ + /* * These functions convert from/to libc's wchar_t, *not* pg_wchar_t. * Therefore we keep them here rather than with the mbutils code. diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index f61a043055..82e6644f89 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -2047,9 +2047,12 @@ check_locale_encoding(const char *locale, int user_enc) * * this should match the similar check in the backend createdb() function */ -static bool +static void check_icu_locale_encoding(int user_enc) { +#ifndef USE_ICU + pg_fatal("ICU is not supported in this build"); +#else if (!(is_encoding_supported_by_icu(user_enc))) { pg_log_error("encoding mismatch"); @@ -2058,9 +2061,17 @@ check_icu_locale_encoding(int user_enc) pg_log_error_hint("Rerun %s and either do not specify an encoding explicitly, " "or choose a matching combination.", progname); - return false; + exit(1); } - return true; + + if (!icu_locale) + pg_fatal("ICU locale must be specified"); + + /* + * In supported builds, the ICU locale ID will be checked by the backend + * during post-bootstrap initialization. + */ +#endif } /* @@ -2113,20 +2124,6 @@ setlocales(void) check_locale_name(LC_CTYPE, lc_messages, &canonname); lc_messages = canonname; #endif - - if (locale_provider == COLLPROVIDER_ICU) - { - if (!icu_locale) - pg_fatal("ICU locale must be specified"); - - /* - * In supported builds, the ICU locale ID will be checked by the - * backend during post-bootstrap initialization. - */ -#ifndef USE_ICU - pg_fatal("ICU is not supported in this build"); -#endif - } } /* @@ -2388,9 +2385,8 @@ setup_locale_encoding(void) !check_locale_encoding(lc_collate, encodingid)) exit(1); /* check_locale_encoding printed the error */ - if (locale_provider == COLLPROVIDER_ICU && - !check_icu_locale_encoding(encodingid)) - exit(1); + if (locale_provider == COLLPROVIDER_ICU) + check_icu_locale_encoding(encodingid); } diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl index 164fc11cbf..884506a1bc 100644 --- a/src/bin/initdb/t/001_initdb.pl +++ b/src/bin/initdb/t/001_initdb.pl @@ -123,28 +123,41 @@ if ($ENV{with_icu} eq 'yes') [ 'initdb', '--no-sync', '--locale-provider=icu', '--encoding=SQL_ASCII', - '--icu-locale=en', "$tempdir/dataX" + "$tempdir/dataX" ], qr/error: encoding mismatch/, 'fails for encoding not supported by ICU'); } else { - command_fails( + command_fails_like( [ 'initdb', '--no-sync', '--locale-provider=icu', "$tempdir/data2" ], + qr/error: ICU is not supported in this build/, 'locale provider ICU fails since no ICU support'); + + command_fails_like( + [ + 'initdb', '--no-sync', + '--locale-provider=icu', '--encoding=SQL_ASCII', + "$tempdir/data2" + ], + qr/error: ICU is not supported in this build/, + 'locale provider ICU with not supported ICU encoding fails since no ICU support' + ); } -command_fails( +command_fails_like( [ 'initdb', '--no-sync', '--locale-provider=xyz', "$tempdir/dataX" ], + qr/error: unrecognized locale provider: xyz/, 'fails for invalid locale provider'); -command_fails( +command_fails_like( [ 'initdb', '--no-sync', '--locale-provider=libc', '--icu-locale=en', "$tempdir/dataX" ], + qr/error: --icu-locale cannot be specified unless locale provider "icu" is chosen/, 'fails for invalid option combination'); done_testing(); diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl index a74bf3b0d8..45b44fce46 100644 --- a/src/bin/scripts/t/020_createdb.pl +++ b/src/bin/scripts/t/020_createdb.pl @@ -30,11 +30,12 @@ if ($ENV{with_icu} eq 'yes') # This fails because template0 uses libc provider and has no ICU # locale set. It would succeed if template0 used the icu # provider. XXX Maybe split into multiple tests? - $node->command_fails( + $node->command_fails_like( [ 'createdb', '-T', 'template0', '-E', 'UTF8', '--locale-provider=icu', 'foobar4' ], + qr/ERROR: ICU locale must be specified/, 'create database with ICU fails without ICU locale specified'); $node->issues_sql_like( @@ -46,12 +47,13 @@ if ($ENV{with_icu} eq 'yes') qr/statement: CREATE DATABASE foobar5 .* LOCALE_PROVIDER icu ICU_LOCALE 'en'/, 'create database with ICU locale specified'); - $node->command_fails( + $node->command_fails_like( [ 'createdb', '-T', 'template0', '-E', 'UTF8', '--locale-provider=icu', '--icu-locale=@colNumeric=lower', 'foobarX' ], + qr/ERROR: could not open collator for locale/, 'fails for invalid ICU locale'); $node->command_fails_like( @@ -78,18 +80,39 @@ if ($ENV{with_icu} eq 'yes') } else { - $node->command_fails( + $node->command_fails_like( [ 'createdb', '-T', 'template0', '--locale-provider=icu', 'foobar4' ], + qr/ERROR: ICU is not supported in this build/, 'create database with ICU fails since no ICU support'); + + $node->command_fails_like( + [ + 'createdb', '-T', + 'template0', '--locale-provider=icu', + '--encoding=SQL_ASCII', 'foobar4' + ], + qr/ERROR: ICU is not supported in this build/, + 'create database with ICU and not supported ICU encoding fails since no ICU support' + ); } $node->command_fails([ 'createdb', 'foobar1' ], 'fails if database already exists'); -$node->command_fails( +$node->command_fails_like( [ 'createdb', '-T', 'template0', '--locale-provider=xyz', 'foobarX' ], + qr/ERROR: unrecognized locale provider: xyz/, 'fails for invalid locale provider'); +$node->command_fails_like( + [ + 'createdb', '-T', + 'template0', '--locale-provider=libc', + '--icu-locale=en', 'foobarX' + ], + qr/ERROR: ICU locale cannot be specified unless locale provider is ICU/, + 'fails for invalid option combination'); + # Check use of templates with shared dependencies copied from the template. my ($ret, $stdout, $stderr) = $node->psql( 'foobar2', diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h index a875942123..6e8af39976 100644 --- a/src/include/utils/pg_locale.h +++ b/src/include/utils/pg_locale.h @@ -104,8 +104,8 @@ extern char *get_collation_actual_version(char collprovider, const char *collcol #ifdef USE_ICU extern int32_t icu_to_uchar(UChar **buff_uchar, const char *buff, size_t nbytes); extern int32_t icu_from_uchar(char **result, const UChar *buff_uchar, int32_t len_uchar); -#endif extern void check_icu_locale(const char *icu_locale); +#endif /* These functions convert from/to libc's wchar_t, *not* pg_wchar_t */ extern size_t wchar2char(char *to, const wchar_t *from, size_t tolen, -- 2.25.1