broken master regress tests

On Fri, Aug 25, 2023 at 3:54 PM Pavel Stehule <pavel.stehule@gmail.com> wrote:

today build is broken on my Fedora 39

Has commit 252dcb32 somehow upset some kind of bleeding edge btrfs
filesystem? That's a wild guess and I can't really imagine how but
apparently your database files are totally messed up...

Hi

pá 25. 8. 2023 v 7:38 odesílatel Thomas Munro <thomas.munro@gmail.com>
napsal:

On Fri, Aug 25, 2023 at 3:54 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

today build is broken on my Fedora 39

Has commit 252dcb32 somehow upset some kind of bleeding edge btrfs
filesystem? That's a wild guess and I can't really imagine how but
apparently your database files are totally messed up...

I use only ext4

[pavel@nemesis ~]$ mount | grep home
/dev/mapper/luks-feb21fdf-c7aa-4373-b25e-fb26d4b28216 on /home type ext4
(rw,relatime,seclabel)

but the kernel is fresh

[pavel@nemesis ~]$ uname -a
Linux nemesis 6.5.0-0.rc6.43.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Aug 14
17:17:41 UTC 2023 x86_64 GNU/Linux

pá 25. 8. 2023 v 8:10 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

Hi

pá 25. 8. 2023 v 7:38 odesílatel Thomas Munro <thomas.munro@gmail.com>
napsal:

On Fri, Aug 25, 2023 at 3:54 PM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

today build is broken on my Fedora 39

Has commit 252dcb32 somehow upset some kind of bleeding edge btrfs
filesystem? That's a wild guess and I can't really imagine how but
apparently your database files are totally messed up...

I use only ext4

[pavel@nemesis ~]$ mount | grep home
/dev/mapper/luks-feb21fdf-c7aa-4373-b25e-fb26d4b28216 on /home type ext4
(rw,relatime,seclabel)

but the kernel is fresh

[pavel@nemesis ~]$ uname -a
Linux nemesis 6.5.0-0.rc6.43.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Mon Aug 14
17:17:41 UTC 2023 x86_64 GNU/Linux

I tested it on another comp with fresh fedora 38 installation with same
result

again I use only ext4 there

On Fri, 25 Aug 2023, 05:54 Pavel Stehule, <pavel.stehule@gmail.com> wrote:

Hi

today build is broken on my Fedora 39

Regards

Pavel

make[2]: Opouští se adresář
„/home/pavel/src/postgresql.master/src/bin/initdb“
make -C pg_amcheck check
make[2]: Vstupuje se do adresáře
„/home/pavel/src/postgresql.master/src/bin/pg_amcheck“
[...]
# ERROR: could not open file "base/16736/16781": Adresář nebo soubor
neexistuje
# '
# doesn't match '(?^:could not open file ".*": No such file or
directory)'

It looks like the error message matcher doesn't account for the localized
version of "No such file or directory", might that be the issue?

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)

pá 25. 8. 2023 v 8:22 odesílatel Matthias van de Meent <
boekewurm+postgres@gmail.com> napsal:

On Fri, 25 Aug 2023, 05:54 Pavel Stehule, <pavel.stehule@gmail.com> wrote:

Hi

today build is broken on my Fedora 39

Regards

Pavel

make[2]: Opouští se adresář
„/home/pavel/src/postgresql.master/src/bin/initdb“
make -C pg_amcheck check
make[2]: Vstupuje se do adresáře
„/home/pavel/src/postgresql.master/src/bin/pg_amcheck“
[...]
# ERROR: could not open file "base/16736/16781": Adresář nebo soubor
neexistuje
# '
# doesn't match '(?^:could not open file ".*": No such file or
directory)'

It looks like the error message matcher doesn't account for the localized
version of "No such file or directory", might that be the issue?

yes

LANG=C maje check-world

fixed this issue

Regards

Pavel

Hi

pá 25. 8. 2023 v 9:12 odesílatel Pavel Stehule <pavel.stehule@gmail.com>
napsal:

pá 25. 8. 2023 v 8:22 odesílatel Matthias van de Meent <
boekewurm+postgres@gmail.com> napsal:

On Fri, 25 Aug 2023, 05:54 Pavel Stehule, <pavel.stehule@gmail.com>
wrote:

Hi

today build is broken on my Fedora 39

Regards

Pavel

make[2]: Opouští se adresář
„/home/pavel/src/postgresql.master/src/bin/initdb“
make -C pg_amcheck check
make[2]: Vstupuje se do adresáře
„/home/pavel/src/postgresql.master/src/bin/pg_amcheck“
[...]
# ERROR: could not open file "base/16736/16781": Adresář nebo
soubor neexistuje
# '
# doesn't match '(?^:could not open file ".*": No such file or
directory)'

It looks like the error message matcher doesn't account for the localized
version of "No such file or directory", might that be the issue?

yes

LANG=C maje check-world

I tried to fix this issue, but there is some strange

regress tests are initialized with

<-->delete $ENV{LANGUAGE};
<-->delete $ENV{LC_ALL};
<-->$ENV{LC_MESSAGES} = 'C';

so the environment should be correct

I checked this setting before

<-->IPC::Run::run($cmd, '>', \$stdout, '2>', \$stderr);

and it looks correct. But the tests fails

Only when I use `LC_MESSAGES=C make check` the tests are ok

My environment has only `LANG=cs_CZ.UTF-8`

So it looks so IPC::Run::run is ignore parent environment

On Sun, Aug 27, 2023 at 3:03 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:

So it looks so IPC::Run::run is ignore parent environment

I guess the new initdb template captures lc_messages in
postgresql.conf, when it runs earlier? I guess if you put
$node->append_conf('postgresql.conf', 'lc_messages=C'); into
src/bin/pg_amcheck/t/003_check.pl then it will work. I'm not sure
what the correct fix should be, ie if the template mechanism should
notice this difference and not use the template, or if tests that
depend on the message locale should explicitly say so with
lc_messages=C or similar (why is this the only one?), or ...

Hi

so 26. 8. 2023 v 23:52 odesílatel Thomas Munro <thomas.munro@gmail.com>
napsal:

On Sun, Aug 27, 2023 at 3:03 AM Pavel Stehule <pavel.stehule@gmail.com>
wrote:

So it looks so IPC::Run::run is ignore parent environment

I guess the new initdb template captures lc_messages in
postgresql.conf, when it runs earlier? I guess if you put
$node->append_conf('postgresql.conf', 'lc_messages=C'); into
src/bin/pg_amcheck/t/003_check.pl then it will work. I'm not sure
what the correct fix should be, ie if the template mechanism should
notice this difference and not use the template, or if tests that
depend on the message locale should explicitly say so with
lc_messages=C or similar (why is this the only one?), or ...

diff --git a/src/bin/pg_amcheck/t/003_check.pl b/src/bin/pg_amcheck/t/
003_check.pl
index d577cffa30..ba7c713adc 100644
--- a/src/bin/pg_amcheck/t/003_check.pl
+++ b/src/bin/pg_amcheck/t/003_check.pl
@@ -122,6 +122,7 @@ sub perform_all_corruptions()
 $node = PostgreSQL::Test::Cluster->new('test');
 $node->init;
 $node->append_conf('postgresql.conf', 'autovacuum=off');
+$node->append_conf('postgresql.conf', 'lc_messages=C');
 $node->start;
 $port = $node->port;

it fixes this issue

Regards

Pavel

On 2023-Aug-27, Thomas Munro wrote:

On Sun, Aug 27, 2023 at 3:03 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:

So it looks so IPC::Run::run is ignore parent environment

I guess the new initdb template captures lc_messages in
postgresql.conf, when it runs earlier? I guess if you put
$node->append_conf('postgresql.conf', 'lc_messages=C'); into
src/bin/pg_amcheck/t/003_check.pl then it will work. I'm not sure
what the correct fix should be, ie if the template mechanism should
notice this difference and not use the template, or if tests that
depend on the message locale should explicitly say so with
lc_messages=C or similar (why is this the only one?), or ...

So I tried this technique, but it gest old pretty fast: apparently
there's a *ton* of tests that depend on the locale. I gave up after
patching the first five files, and noticing that in a second run there
another half a dozen failing tests that hadn't failed the first time
around. (Not sure why this happened.)

So I think injecting --no-locale to the initdb line that creates the
template is a better approach; something like the attached.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/

út 29. 8. 2023 v 17:54 odesílatel Alvaro Herrera <alvherre@alvh.no-ip.org>
napsal:

On 2023-Aug-27, Thomas Munro wrote:

On Sun, Aug 27, 2023 at 3:03 AM Pavel Stehule <pavel.stehule@gmail.com>

wrote:

So it looks so IPC::Run::run is ignore parent environment

I guess the new initdb template captures lc_messages in
postgresql.conf, when it runs earlier? I guess if you put
$node->append_conf('postgresql.conf', 'lc_messages=C'); into
src/bin/pg_amcheck/t/003_check.pl then it will work. I'm not sure
what the correct fix should be, ie if the template mechanism should
notice this difference and not use the template, or if tests that
depend on the message locale should explicitly say so with
lc_messages=C or similar (why is this the only one?), or ...

So I tried this technique, but it gest old pretty fast: apparently
there's a *ton* of tests that depend on the locale. I gave up after
patching the first five files, and noticing that in a second run there
another half a dozen failing tests that hadn't failed the first time
around. (Not sure why this happened.)

So I think injecting --no-locale to the initdb line that creates the
template is a better approach; something like the attached.

ok

thank you for fixing it

Regards

Pavel

Hi,

On 2023-08-29 17:54:24 +0200, Alvaro Herrera wrote:

On 2023-Aug-27, Thomas Munro wrote:

On Sun, Aug 27, 2023 at 3:03 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:

So it looks so IPC::Run::run is ignore parent environment

I guess the new initdb template captures lc_messages in
postgresql.conf, when it runs earlier? I guess if you put
$node->append_conf('postgresql.conf', 'lc_messages=C'); into
src/bin/pg_amcheck/t/003_check.pl then it will work. I'm not sure
what the correct fix should be, ie if the template mechanism should
notice this difference and not use the template, or if tests that
depend on the message locale should explicitly say so with
lc_messages=C or similar (why is this the only one?), or ...

So I tried this technique, but it gest old pretty fast: apparently
there's a *ton* of tests that depend on the locale. I gave up after
patching the first five files, and noticing that in a second run there
another half a dozen failing tests that hadn't failed the first time
around. (Not sure why this happened.)

So I think injecting --no-locale to the initdb line that creates the
template is a better approach; something like the attached.

Makes sense, thanks for taking care of this.

Greetings,

Andres Freund

On Mon, 2023-09-11 at 19:23 -0700, Andres Freund wrote:

So I think injecting --no-locale to the initdb line that creates
the
template is a better approach; something like the attached.

Makes sense, thanks for taking care of this.

After this, it seems "make check" no longer picks up the locale from
the system environment by default.

What is the new way to run the regression tests with an actual locale?
If it's no longer done by default, won't that dramatically reduce the
coverage of non-C locales?

Regards,
Jeff Davis

Hi,

On 2023-10-10 17:08:25 -0700, Jeff Davis wrote:

On Mon, 2023-09-11 at 19:23 -0700, Andres Freund wrote:

So I think injecting --no-locale to the initdb line that creates
the
template is a better approach; something like the attached.

Makes sense, thanks for taking care of this.

After this, it seems "make check" no longer picks up the locale from
the system environment by default.

Yea. I wonder if the better fix would have been to copy setenv("LC_MESSAGES", "C", 1);
to the initdb template creation. That afaict also fixes the issue, with a
smaller blast radius?

Greetings,

Andres Freund

On Tue, 2023-10-10 at 17:54 -0700, Andres Freund wrote:

Yea. I wonder if the better fix would have been to copy
setenv("LC_MESSAGES", "C", 1);
to the initdb template creation. That afaict also fixes the issue,
with a
smaller blast radius?

Sounds good to me. Is there anything else we should do to notice that
tests are unexpectedly skipped, or is this a rare enough problem to not
worry about other cases?

Regards,
Jeff Davis

On Tue, Oct 10, 2023 at 05:54:34PM -0700, Andres Freund wrote:

On 2023-10-10 17:08:25 -0700, Jeff Davis wrote:

After this, it seems "make check" no longer picks up the locale from
the system environment by default.

Yea. I wonder if the better fix would have been to copy setenv("LC_MESSAGES", "C", 1);
to the initdb template creation. That afaict also fixes the issue, with a
smaller blast radius?

+1, that would restore the testing semantics known from v16-. I think the
intent of the template was to optimize without changing semantics, and the
above proposal aligns with that. Currently, for v17 alone, one needs
installcheck or build file hacks to reproduce a locale-specific test failure.

An alternative would be to declare that the tests are supported in one
encoding+locale only, then stop testing others in the buildfarm. Even if we
did that, I'm fairly sure we'd standardize on UTF8, not SQL_ASCII, as the one
testable encoding.

Noah Misch <noah@leadboat.com> writes:

An alternative would be to declare that the tests are supported in one
encoding+locale only, then stop testing others in the buildfarm.

Surely that's not even a thinkably acceptable choice.

regards, tom lane

On Tue, 2023-12-12 at 18:56 -0800, Noah Misch wrote:

Yea. I wonder if the better fix would have been to copy
setenv("LC_MESSAGES", "C", 1);
to the initdb template creation. That afaict also fixes the issue,
with a
smaller blast radius?

+1, that would restore the testing semantics known from v16-.  I
think the
intent of the template was to optimize without changing semantics,
and the
above proposal aligns with that.  Currently, for v17 alone, one needs
installcheck or build file hacks to reproduce a locale-specific test
failure.

Attached.

I just changed --no-locale to --lc-messages=C, which I think solves it
in the right place with minimal blast radius. Andres, did you literally
mean C setenv() somewhere, or is this what you had in mind?

I also noticed that collate.linux.utf8.sql seems to be skipped on my
machine because of the "version() !~ 'linux-gnu'" check, even though
I'm running Ubuntu. Is that test getting run often enough?

And relatedly, is it worth thinking about extending pg_regress to
report skipped tests so it's easier to find these kinds of problems?

Regards,
Jeff Davis

On Wed, 2023-12-20 at 17:48 -0800, Jeff Davis wrote:

Attached.

It appears to increase the coverage. I committed it and I'll see how
the buildfarm reacts.

Regards,
Jeff Davis

Hi

pá 22. 12. 2023 v 0:17 odesílatel Jeff Davis <pgsql@j-davis.com> napsal:

On Wed, 2023-12-20 at 17:48 -0800, Jeff Davis wrote:

Attached.

It appears to increase the coverage. I committed it and I'll see how
the buildfarm reacts.

I tested it locally ( LANG=cs_CZ.UTF-8 ) without problems

Regards

Pavel

Hello Jeff,

22.12.2023 02:17, Jeff Davis wrote:

On Wed, 2023-12-20 at 17:48 -0800, Jeff Davis wrote:

Attached.

It appears to increase the coverage. I committed it and I'll see how
the buildfarm reacts.

Starting from the commit 8793c6005, I observe a failure of test
collate.windows.win1252 on Windows Server 2016:
meson test regress/regress
1/1 postgresql:regress / regress/regress        ERROR 24.47s   exit status 1

regression.diffs contains:
@@ -993,6 +993,8 @@
  -- nondeterministic collations
  -- (not supported with libc provider)
  CREATE COLLATION ctest_det (locale = 'en_US', deterministic = true);
+ERROR:  could not create locale "en_US": No such file or directory
+DETAIL:  The operating system could not find any locale data for the locale name "en_US".
  CREATE COLLATION ctest_nondet (locale = 'en_US', deterministic = false);
  ERROR:  nondeterministic collations not supported with this provider
  -- cleanup

Though
CREATE COLLATION ctest_det (locale = 'English_United States', deterministic = true);
executed successfully on this OS.

AFAICS, before that commit SELECT getdatabaseencoding() in the test
returned SQL_ASCII, hence the test was essentially skipped, but now it
returns WIN1252, so problematic CREATE COLLATION(locale = 'en_US', ...)
is reached.

Best regards,
Alexander

On Thu, 2023-12-28 at 18:00 +0300, Alexander Lakhin wrote:

AFAICS, before that commit SELECT getdatabaseencoding() in the test
returned SQL_ASCII, hence the test was essentially skipped, but now
it
returns WIN1252, so problematic CREATE COLLATION(locale = 'en_US',
...)
is reached.

We do want that test to run though, right?

I suspect that test line never worked reliably. The skip_test check at
the top guarantees that the collation named "en_US" exists, but that
doesn't mean that the OS understands the locale 'en_US'.

Perhaps we can change that line to use a similar trick as what's used
elsewhere in the file:

do $$
BEGIN
EXECUTE 'CREATE COLLATION ctest_det (locale = ' ||
quote_literal((SELECT collcollate FROM pg_collation WHERE
collname = ''en_US'')) || ', deterministic = true);';
END
$$;

The above may need some adjustment, but perhaps you can try it out?
Another option might be to use \gset to assign it to a variable, which
might be more readable, but I think it's better to just follow what the
rest of the file is doing.

Regards,
Jeff Davis

28.12.2023 20:36, Jeff Davis wrote:

We do want that test to run though, right?

Yes, I think so.

I suspect that test line never worked reliably. The skip_test check at
the top guarantees that the collation named "en_US" exists, but that
doesn't mean that the OS understands the locale 'en_US'.

Perhaps we can change that line to use a similar trick as what's used
elsewhere in the file:

do $$
BEGIN
EXECUTE 'CREATE COLLATION ctest_det (locale = ' ||
quote_literal((SELECT collcollate FROM pg_collation WHERE
collname = ''en_US'')) || ', deterministic = true);';
END
$$;

The above may need some adjustment, but perhaps you can try it out?

Yes, this trick resolves the issue, it gives locale 'en-US' on that OS,
which works there. Please see the attached patch.

But looking at the result with the comment above that "do" block, I wonder
whether this successful CREATE COLLATION command is so important to perform
it that tricky way, if we want to demonstrate that nondeterministic
collations not supported.
So in case you decide just to remove this command, please see the second
patch.

Best regards,
Alexander

On Thu, 2023-12-28 at 22:00 +0300, Alexander Lakhin wrote:

But looking at the result with the comment above that "do" block, I
wonder
whether this successful CREATE COLLATION command is so important to
perform
it that tricky way, if we want to demonstrate that nondeterministic
collations not supported.

Thank you, pushed this version. There are other similar commands in the
file, so I think it's fine. It exercises a specific locale that might
be different from datcollate.

Regards,
Jeff Davis