On 6/12/23 17:28, Joe Conway wrote:
On 6/12/23 10:44, Joe Conway wrote:
1/ how do we fix the misbehavior reported due to libperl in existing
stable branches
<snip>
I was mostly trying to concentrate on #1, but 2 & 3 are worthy of
discussion.
Hmm, browsing through the perl source I came across a reference to this
(from https://perldoc.perl.org/perllocale):
---------------
PERL_SKIP_LOCALE_INIT
This environment variable, available starting in Perl v5.20, if set
(to any value), tells Perl to not use the rest of the environment
variables to initialize with. Instead, Perl uses whatever the current
locale settings are. This is particularly useful in embedded
environments, see "Using embedded Perl with POSIX locales" in perlembed.
---------------
Seems we ought to be using that.
Turns out that that does nothing useful as far as I can tell.
So I am back to proposing the attached against pg16beta1, to be
backpatched to pg11.
Since much of the discussion happened on pgsql-bugs, the background
summary for hackers is this:
When plperl is first loaded, the init function eventually works its way
to calling Perl_init_i18nl10n(). In versions of perl >= 5.20, that ends
up at S_emulate_setlocale() which does a series of uselocale() calls.
For reference, RHEL 7 is perl 5.16.3 while RHEL 9 is perl 5.32.1. Older
versions of perl do not have this behavior.
The problem with uselocale() is that it changes the active locale away
from the default global locale. Subsequent uses of setlocale() affect
the global locale, but if that is not the active locale, it does not
control the results of locale dependent functions such as localeconv(),
which is what we depend on in PGLC_localeconv().
The result is illustrated in this example:
8<------------
psql test
psql (16beta1)
Type "help" for help.
test=# show lc_monetary;
lc_monetary
-------------
en_GB.UTF-8
(1 row)
test=# SELECT 12.34::money AS price;
price
--------
£12.34
(1 row)
test=# \q
8<------------
psql test
psql (16beta1)
Type "help" for help.
test=# load 'plperl';
LOAD
test=# show lc_monetary;
lc_monetary
-------------
en_GB.UTF-8
(1 row)
test=# SELECT 12.34::money AS price;
price
--------
$12.34
(1 row)
8<------------
Notice that merely loading plperl makes the currency symbol wrong.
I have proposed a targeted fix that I believe is safe to backpatch --
attached.
IIUC, Tom was +1, but Heikki was looking for a more general solution.
My issue with the more general solution is that it will likely be too
invasive to backpatch, and at the moment at least, there are no other
confirmed bugs related to all of this (even if the current code is more
fragile than we would prefer).
I would like to commit this to all supported branches in the next few
days, unless there are other suggestions or objections.
--
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Ensure the global locale gets used by localeconv()
A loaded library, such as libperl, may call uselocale() underneath us.
This can result in localeconv() grabbing the wrong locale for
numeric and monetary symbols and formatting. Fix that by resetting
to the global locale determined by setlocale(). Backpatch to all
supported versions.
Author: Joe Conway
Reviewed-By: Tom Lane
Reported by: Guido Brugnara
Discussion: https://postgr.es/m/flat/17946-3e84cb577e9551c3%40postgresql.org
Backpatch-through: 11
diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 31e3b16..9dba161 100644
*** a/src/backend/utils/adt/pg_locale.c
--- b/src/backend/utils/adt/pg_locale.c
*************** PGLC_localeconv(void)
*** 505,510 ****
--- 505,517 ----
}
/*
+ * Ensure the global locale will be used by localeconv().
+ * This is necessary, for example, if another loaded library
+ * such as libperl has done uselocale() underneath us.
+ */
+ uselocale(LC_GLOBAL_LOCALE);
+
+ /*
* This is tricky because we really don't want to risk throwing error
* while the locale is set to other than our usual settings. Therefore,
* the process is: collect the usual settings, set locale to special
diff --git a/src/fe_utils/print.c b/src/fe_utils/print.c
index 7af1ccb..5d80e77 100644
*** a/src/fe_utils/print.c
--- b/src/fe_utils/print.c
*************** setDecimalLocale(void)
*** 3628,3633 ****
--- 3628,3639 ----
{
struct lconv *extlconv;
+ /*
+ * Ensure the global locale will be used by localeconv().
+ * This is necessary, for example, if another loaded library
+ * has done uselocale() underneath us.
+ */
+ uselocale(LC_GLOBAL_LOCALE);
extlconv = localeconv();
/* Don't accept an empty decimal_point string */
