Обсуждение: Re: pgsql: psql: add an optional execution-count limit to \watch.
Hello, hackers.
On 18/04/2023 20:34, Tom Lane wrote (on pgsql-committers):
> I shall now retire to a safe distance and watch the buildfarm.
Unfortunately, on fresh perl (5.38.2 verified) and on ru_RU.UTF-8
locale, it breaks basic float comparison: 0 < 0.5 is no longer true.
This is the reproduction on REL_16_STABLE (but it affects master
as well), using fresh Ubuntu 24.04 container.
0. I've used lxc to get a fresh container:
$ lxc launch ubuntu-daily:noble u2404
But I don't think lxc or containerization in general matters in this
case. Also, I think any environment with fresh enough Perl would work,
Ubuntu 24.04 is just an easy example.
(obviously, install necessary dev packages)
1. Generate ru_RU.UTF-8 locale:
a. In /etc/locale.gen, uncomment the line:
# ru_RU.UTF-8 UTF-8
b. Run locale-gen as root. For me, it says:
$ sudo locale-gen
Generating locales (this might take a while)...
en_US.UTF-8... done
ru_RU.UTF-8... done
Generation complete.
2. Apply 0001-demo-of-weird-Perl-setlocale-effect-on-float-numbers.patch
(adding src/test/authentication/t/999_broken.pl)
3. Run the test
LANG=ru_RU.UTF-8 make check -C src/test/authentication
PROVE_TESTS=t/999_broken.pl PROVE_FLAGS=--verbose
The test is, basically:
use PostgreSQL::Test::Utils;
use Test::More tests => 1;
ok(0 < 0.5, "0 < 0.5");
If I comment-out the "use PostgreSQL::Test::Utils" line, the test works.
Otherwise it fails to notice that 0 is less than 0.5.
Alternatively, the test fails if I replace that "use" line with
BEGIN {
use POSIX qw(locale_h);
setlocale(LC_NUMERIC, "");
}
"BEGIN" part is essential: mere use/setlocale is fine.
Also, adding
use locale;
or even
use locale ':numeric';
fixes the test, but I doubt whether it's a good idea to add that to
Utils.pm.
Obviously, one of the reasons is that according to ru_RU.UTF-8 locale
for LC_NUMERIC, fractional part separator is ",", not ".". So one could,
technically, parse "0.5" as "0" and then unparsed ".5" tail. I think it
might even be a Perl bug, because, according to my quick browsing of man
perlfunc (setlocale) and man perllocale, this should not affect the code
outside "use locale", not in such a fundamental way. After all, we're
talking not about strtod etc, but about floating-point numbers in the
source code.
P.S. $ perl --version
This is perl 5, version 38, subversion 2 (v5.38.2) built for
x86_64-linux-gnu-thread-multi
(with 44 registered patches, see perl -V for more detail)
P.P.S. I'm replying to pgsql-hackers, even though part of previous
discussion have been on pgsql-committers. Hopefully, it's OK.
--
Anton Voloshin
Postgres Professional, The Russian Postgres Company
https://postgrespro.ru
Вложения
Anton Voloshin <a.voloshin@postgrespro.ru> writes:
> On 18/04/2023 20:34, Tom Lane wrote (on pgsql-committers):
>>> I shall now retire to a safe distance and watch the buildfarm.
> Unfortunately, on fresh perl (5.38.2 verified) and on ru_RU.UTF-8
> locale, it breaks basic float comparison: 0 < 0.5 is no longer true.
Haven't we worked around that everywhere it matters, in commits such
as 8421f6bce and 605062227? For me, check-world passes under
LANG=ru_RU, even with perl 5.38.2 (where I do confirm that your
test script fails). The buildfarm isn't unhappy either.
> Obviously, one of the reasons is that according to ru_RU.UTF-8 locale
> for LC_NUMERIC, fractional part separator is ",", not ".". So one could,
> technically, parse "0.5" as "0" and then unparsed ".5" tail. I think it
> might even be a Perl bug, because, according to my quick browsing of man
> perlfunc (setlocale) and man perllocale, this should not affect the code
> outside "use locale", not in such a fundamental way. After all, we're
> talking not about strtod etc, but about floating-point numbers in the
> source code.
I agree that it's a Perl bug, mainly because your test case doesn't
fail in Perls as recent as v5.32.1 (released about 3 years ago).
It's impossible to believe that they intentionally broke basic
Perl constant syntax now, after so many years. Particularly in
this way --- what are we supposed to do, write "if (0 < 0,5)"?
That means something else.
regards, tom lane
On 26/04/2024 05:20, Tom Lane wrote: > Haven't we worked around that everywhere it matters, in commits such > as 8421f6bce and 605062227? Yes, needing 8421f6bce and 605062227 was, perhaps, surprising, but reasonable. Unlike breaking floating point constants in the source code. But, I guess, you're right and, since it does look like a Perl bug, we'll have to work around that in all places where we use floating-point constants in Perl code, which are surprisingly few. > For me, check-world passes under > LANG=ru_RU, even with perl 5.38.2 (where I do confirm that your > test script fails). The buildfarm isn't unhappy either. Indeed, check-world seems to run fine on my machine and on the bf as well. Grepping and browsing through, I've only found three spots with \d\.\d directly in Perl code as a float, only one of them needs correction. 1. src/test/perl/PostgreSQL/Test/Kerberos.pm in master src/test/kerberos/t/001_auth.pl in REL_16_STABLE > if ($krb5_version >= 1.15) I guess adding use locale ':numeric' would be easiest workaround here. Alternatively, we could also split version into krb5_major_version and krb5_minor_version while parsing krb5-config --version's output above, but I don't think that's warranted. So I suggest something along the lines of 0001-use-numeric-locale-in-kerberos-test-rel16.patch and *-master.patch (attached, REL_16 and master need this change in different places). I did verify by providing fake 'krb5-config' that before the fix, with LANG=ru_RU.UTF-8 and Perl 5.38.2 and with, say, krb5 "version" 1.13 it would still add the "listen" lines to kdc.conf by mistake (presumably, confusing some versions of kerberos). 2 and 3. contrib/intarray/bench/create_test.pl > if (rand() < 0.7) and > if ($#sect < 0 || rand() < 0.1) PostgreSQL::Test::Utils is not used there, so it's OK, no change needed. I did not find any other float constants in .pl/.pm files in master (I could have missed something). > Particularly in > this way --- what are we supposed to do, write "if (0 < 0,5)"? > That means something else. Yep. I will try to report this to Perl community later. -- Anton Voloshin Postgres Professional, The Russian Postgres Company https://postgrespro.ru
Вложения
On 26/04/2024 17:38, Anton Voloshin wrote: > I will try to report this to Perl community later. Reported under https://github.com/Perl/perl5/issues/22176 Perl 5.36.3 seems to be fine (latest stable release before 5.38.x). 5.38.0 and 5.38.2 are broken. -- Anton Voloshin Postgres Professional, The Russian Postgres Company https://postgrespro.ru
Anton Voloshin <a.voloshin@postgrespro.ru> writes: > On 26/04/2024 17:38, Anton Voloshin wrote: >> I will try to report this to Perl community later. > Reported under https://github.com/Perl/perl5/issues/22176 Thanks for doing that. > Perl 5.36.3 seems to be fine (latest stable release before 5.38.x). > 5.38.0 and 5.38.2 are broken. If the misbehavior is that new, I'm inclined to do nothing about it, figuring that they'll fix it sooner not later. If we were seeing failures in main-line check-world tests then maybe it'd be worth band-aiding those, but AFAICS we're not. regards, tom lane