On 2022-10-29 14:33, Marina Polyakova wrote:
Hello!
This is the last proposed patch on this subject [1] moved to a
separate thread for Commitfest..
Also added a patch to export with_icu when running src/bin/scripts tests
[1].
The problem can be reproduced as
$ meson setup <source dir> && ninja && meson test --print-errorlogs
--suite setup --suite scripts
[1] https://cirrus-ci.com/task/4825664661487616
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From a6de5922fc533c88c7288051d8797d021ae91dae Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Sat, 29 Oct 2022 14:25:05 +0300
Subject: [PATCH v1 2/2] 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
From dc768bc3b1df0e387ecc3ef636f6233e22c33e59 Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Sat, 29 Oct 2022 16:03:21 +0300
Subject: [PATCH v1 1/2] Export with_icu when running src/bin/scripts tests
with meson
---
src/bin/scripts/meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/bin/scripts/meson.build b/src/bin/scripts/meson.build
index 837562c24e..c9c74d10ac 100644
--- a/src/bin/scripts/meson.build
+++ b/src/bin/scripts/meson.build
@@ -38,6 +38,7 @@ tests += {
'sd': meson.current_source_dir(),
'bd': meson.current_build_dir(),
'tap': {
+ 'env': {'with_icu': icu.found() ? 'yes' : 'no'},
'tests': [
't/010_clusterdb.pl',
't/011_clusterdb_all.pl',
--
2.25.1
