pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Catalog changes preparing for builtin collation provider.
12+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2024-03-12 32dd2c1eff Fix version check in 002_pg_upgrade.pl.
2024-03-11 bbbf71d9a6 Fix 002_pg_upgrade.pl.
* pgsql: Catalog changes preparing for builtin collation provider.
@ 2024-03-09 22:50 Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Davis @ 2024-03-09 22:50 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Catalog changes preparing for builtin collation provider.
Rename pg_collation.colliculocale to colllocale, and
pg_database.daticulocale to datlocale. These names reflects that the
fields will be useful for the upcoming builtin provider as well, not
just for ICU.
This is purely a rename; no changes to the meaning of the fields.
Discussion: https://postgr.es/m/ff4c2f2f9c8fc7ca27c1c24ae37ecaeaeaff6b53.camel%40j-davis.com
Reviewed-by: Peter Eisentraut
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/f696c0cd5f299f1b51e214efc55a22a782cc175d
Modified Files
--------------
doc/src/sgml/bki.sgml | 2 +-
doc/src/sgml/catalogs.sgml | 26 +++++++---
src/backend/catalog/pg_collation.c | 10 ++--
src/backend/commands/collationcmds.c | 34 ++++++-------
src/backend/commands/dbcommands.c | 68 +++++++++++++-------------
src/backend/utils/adt/pg_locale.c | 4 +-
src/backend/utils/init/postinit.c | 12 ++---
src/bin/initdb/initdb.c | 34 ++++++-------
src/bin/pg_dump/pg_dump.c | 56 +++++++++++----------
src/bin/pg_upgrade/info.c | 31 +++++++-----
src/bin/pg_upgrade/pg_upgrade.c | 34 +++++++++----
src/bin/pg_upgrade/pg_upgrade.h | 2 +-
src/bin/pg_upgrade/t/002_pg_upgrade.pl | 29 +++++++----
src/bin/psql/describe.c | 20 +++++---
src/include/catalog/catversion.h | 2 +-
src/include/catalog/pg_collation.dat | 2 +-
src/include/catalog/pg_collation.h | 4 +-
src/include/catalog/pg_database.dat | 2 +-
src/include/catalog/pg_database.h | 2 +-
src/test/regress/expected/collate.icu.utf8.out | 4 +-
src/test/regress/expected/psql.out | 18 +++----
src/test/regress/sql/collate.icu.utf8.sql | 4 +-
22 files changed, 231 insertions(+), 169 deletions(-)
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
@ 2024-03-10 03:53 ` Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Davis @ 2024-03-10 03:53 UTC (permalink / raw)
To: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Sat, 2024-03-09 at 22:50 +0000, Jeff Davis wrote:
> Catalog changes preparing for builtin collation provider.
This is causing problems on a couple buildfarm members (mantid,
lapwing, snakefly) that all look like this:
# Running: pg_upgrade --no-sync -d
/home/postgres/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/
t_002_pg_upgrade_old_node_data/pgdata -D
/home/postgres/buildroot/HEAD/pgsql.build/src/bin/pg_upgrade/tmp_check/
t_002_pg_upgrade_new_node_data/pgdata -b
/home/postgres/buildroot/HEAD/pgsql.build/tmp_install/home/postgres/bui
ldroot/HEAD/inst/bin -B
/home/postgres/buildroot/HEAD/pgsql.build/tmp_install/home/postgres/bui
ldroot/HEAD/inst/bin -s /tmp/3b07YyaACZ -p 52828 -P 52829 --copy --
check
[01:08:08.992](1.053s) ok 9 - invalid database causes failure status
(got 1 vs expected 1)
[01:08:08.992](0.001s) ok 10 - invalid database causes failure stdout
/(?^:invalid)/
[01:08:08.993](0.001s) not ok 11 - invalid database causes failure
stderr /(?^:)/
[01:08:08.993](0.000s)
[01:08:08.993](0.000s) # Failed test 'invalid database causes failure
stderr /(?^:)/'
[01:08:08.993](0.000s) # at t/002_pg_upgrade.pl line 370.
[01:08:08.994](0.000s) # ''
# doesn't match '(?^:)'
That's a little confusing to me: the '(?^:)' is just qr//, which should
match anything, right?
It's expecting stderr to be empty, and the test failure seems to
indicate it is empty, so again I don't really see the problem.
Am I missing something obvious here?
Quite a few other members are passing and I didn't find an obvious
pattern yet.
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
@ 2024-03-10 05:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-03-10 05:05 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
Jeff Davis <pgsql@j-davis.com> writes:
> On Sat, 2024-03-09 at 22:50 +0000, Jeff Davis wrote:
>> Catalog changes preparing for builtin collation provider.
> This is causing problems on a couple buildfarm members (mantid,
> lapwing, snakefly) that all look like this:
> ...
> That's a little confusing to me: the '(?^:)' is just qr//, which should
> match anything, right?
Yeah. It looks to me like it's somehow Perl-version-related.
The oldest Perl versions in the buildfarm (as of last month,
last time I scraped these results) are
longfin | 2024-02-08 11:16:05 | configure: using perl 5.14.0
lapwing | 2024-02-08 12:40:20 | configure: using perl 5.14.2
arowana | 2024-02-08 04:08:57 | configure: using perl 5.16.3
boa | 2024-02-07 16:10:12 | configure: using perl 5.16.3
buri | 2024-02-07 20:08:23 | configure: using perl 5.16.3
clam | 2024-02-08 15:00:10 | configure: using perl 5.16.3
dhole | 2024-02-08 06:09:13 | configure: using perl 5.16.3
mantid | 2024-02-08 14:07:10 | configure: using perl 5.16.3
parula | 2024-02-08 11:15:02 | configure: using perl 5.16.3
rhinoceros | 2024-02-08 11:52:11 | configure: using perl 5.16.3
siskin | 2024-02-07 20:16:39 | configure: using perl 5.16.3
snakefly | 2024-02-08 11:15:03 | configure: using perl 5.16.3
Of these, lapwing, mantid, and snakefly have failed; arowana
succeeded, but it didn't run the pg_upgrade test; longfin succeeded,
which I'm confused about; and the rest did not run yet. Not sure
what to make of that, but I counsel sleeping on it and seeing what
the remaining animals say.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-03-11 19:26 ` Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 21:08 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 2 replies; 12+ messages in thread
From: Jeff Davis @ 2024-03-11 19:26 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Jeff Davis <jdavis@postgresql.org>; pgsql-committers@lists.postgresql.org
On Sun, 2024-03-10 at 00:05 -0500, Tom Lane wrote:
> Yeah. It looks to me like it's somehow Perl-version-related.
I found it. On perl 5.16.3 on a failing instance:
use strict;
die '"" does not match qr// before substitution' unless '' =~ qr//;
my $foo = ('|a|' =~ s/a//r);
die '"" does not match qr// after substitution' unless '' =~ qr//;
print "$foo\n";
dies with:
"" does not match qr// after substitution at - line 4.
My commit adds a line using the s/// operator.
That's so bizarre that I have to guess that it's a perl bug. I poked
around in the perl docs to see if I'm missing something, but I didn't
find anything indicating that a substitution would have crazy side
effects. Please correct me if I'm wrong; I'm far from a perl expert.
Assuming that it is a perl bug, there are two potential workarounds:
1. Use qr/^$/ for the test rather than qr//.
2. Don't use s/// anywhere. Find another way to transform devel
versions into numbers.
Either one has the potential to leave traps for later. New tests might
either rely on s/// or expect qr// to work. I am inclined toward #1,
because if we use qr/^$/, other tests are likely to copy it and they
will be safe as well.
Though I'm still not sure what's going on with longfin.
Thoughts?
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
@ 2024-03-11 20:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 20:21 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-03-11 20:15 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org
Jeff Davis <pgsql@j-davis.com> writes:
> I found it. On perl 5.16.3 on a failing instance:
> use strict;
> die '"" does not match qr// before substitution' unless '' =~ qr//;
> my $foo = ('|a|' =~ s/a//r);
> die '"" does not match qr// after substitution' unless '' =~ qr//;
> print "$foo\n";
> dies with:
> "" does not match qr// after substitution at - line 4.
Wow.
> That's so bizarre that I have to guess that it's a perl bug.
I agree.
> Assuming that it is a perl bug, there are two potential workarounds:
> 1. Use qr/^$/ for the test rather than qr//.
> 2. Don't use s/// anywhere. Find another way to transform devel
> versions into numbers.
> Either one has the potential to leave traps for later. New tests might
> either rely on s/// or expect qr// to work. I am inclined toward #1,
> because if we use qr/^$/, other tests are likely to copy it and they
> will be safe as well.
I like #1 as well, because qr// imposes zero constraints on what
the command's stderr is. Checking qr/^$/ would express what we
really want to express, namely no stderr output.
> Though I'm still not sure what's going on with longfin.
Me too, not least because your minimal example fails just as above
with longfin's 5.14.0 build. The fact that longfin nonetheless
passes the whole test indicates that something in between those
steps manages to clean up whatever the broken state is.
I also see that none of the other animals running 5.16.3 ever
showed the failure. This could easily indicate some bizarre
platform-specificity in the bug. It could also mean that the
Linux vendors started carrying patches for this bug, but the
three machines that failed are too old to have any such patch.
Digging in perl's revision history might be interesting if we
really felt the need to identify the bug precisely, but I don't.
Let's just go with your #1.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-03-11 20:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 21:17 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-03-11 20:21 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org
I wrote:
> Jeff Davis <pgsql@j-davis.com> writes:
>> That's so bizarre that I have to guess that it's a perl bug.
> I agree.
Oh! No, it's a "feature": man perlop quoth
The empty pattern "//"
If the PATTERN evaluates to the empty string, the last
successfully matched regular expression is used instead. In
this case, only the "g" and "c" flags on the empty pattern are
honored; the other flags are taken from the original pattern.
If no match has previously succeeded, this will (silently) act
instead as a genuine empty pattern (which will always match).
So we actually need to find and nuke all of these, not just the one
that's causing trouble.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 20:21 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-03-11 21:17 ` Jeff Davis <pgsql@j-davis.com>
2024-03-11 22:04 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Davis @ 2024-03-11 21:17 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Mon, 2024-03-11 at 16:21 -0400, Tom Lane wrote:
> Oh! No, it's a "feature": man perlop quoth
Wow.
> So we actually need to find and nuke all of these, not just the one
> that's causing trouble.
For now I committed the one change to fix the buildfarm.
I am still confused on a couple of points here, such as: why does my
example work fine on newer versions of perl?
But I agree: if the empty pattern is magical, we should get rid of it,
even if we don't understand the exact conditions under which it behaves
magically.
Reagrds,
Jeff Davis
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 20:21 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 21:17 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
@ 2024-03-11 22:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-12 00:10 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
0 siblings, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-03-11 22:04 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org
Jeff Davis <pgsql@j-davis.com> writes:
> I am still confused on a couple of points here, such as: why does my
> example work fine on newer versions of perl?
I'm not sure about that either. The discussions of this that I found
on-line say that it's not something the Perl gurus want to get rid of,
but it sure seems like they must have tightened the behavior to some
extent.
Another question is why it didn't fail on all the animals with similar
Perl vintage. My guess about that is that there's some configuration
difference causing the Perl script to take slightly different code
paths, but it's just a guess.
> But I agree: if the empty pattern is magical, we should get rid of it,
> even if we don't understand the exact conditions under which it behaves
> magically.
Yeah. I was dismayed to find that there's no perlcritic check for
this, because it sure seems like the kind of thing you don't want
to invoke by accident.
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 20:21 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 21:17 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 22:04 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-03-12 00:10 ` Jeff Davis <pgsql@j-davis.com>
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Davis @ 2024-03-12 00:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Mon, 2024-03-11 at 18:04 -0400, Tom Lane wrote:
> Another question is why it didn't fail on all the animals with
> similar
> Perl vintage. My guess about that is that there's some configuration
> difference causing the Perl script to take slightly different code
> paths, but it's just a guess.
Sounds plausible.
>
> Yeah. I was dismayed to find that there's no perlcritic check for
> this, because it sure seems like the kind of thing you don't want
> to invoke by accident.
Additionally, the delimiters can many characters, which makes simple
grepping harder.
I found a few cases -- attached a patch. Something is better than
nothing. I'm not sure that I got the /.*/ cases right, but they don't
seem to be expecting any output, so /.*/ seemed like the right pattern.
Regards,
Jeff Davis
Attachments:
v1-0001-perl-avoid-empty-regex-patterns.patch (text/x-patch, 4.6K) download
From be5aa677e37180a8c1b0faebcceab5506b1c8130 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Mon, 11 Mar 2024 16:44:56 -0700
Subject: [PATCH v1] perl: avoid empty regex patterns
Empty patterns have special behavior that uses the last successful
pattern match. This behavior can be surprising, so remove empty
patterns and instead match against exactly what is intended
(e.g. /^$/ or /.*/).
Unfortunately there's not an easy way to check for this in an
automated way, so it's likely that some cases have been missed and
will be missed in the future. This commit just cleans up known
instances.
Discussion: https://postgr.es/m/1548559.1710188512@sss.pgh.pa.us
---
src/bin/pg_upgrade/t/003_logical_slots.pl | 4 ++--
src/bin/pg_upgrade/t/004_subscription.pl | 2 +-
src/bin/psql/t/001_basic.pl | 8 ++++----
src/bin/psql/t/010_tab_completion.pl | 2 +-
src/test/recovery/t/037_invalid_database.pl | 2 +-
5 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/bin/pg_upgrade/t/003_logical_slots.pl b/src/bin/pg_upgrade/t/003_logical_slots.pl
index 83d71c3084..256dfd53b1 100644
--- a/src/bin/pg_upgrade/t/003_logical_slots.pl
+++ b/src/bin/pg_upgrade/t/003_logical_slots.pl
@@ -78,7 +78,7 @@ command_checks_all(
[
qr/max_replication_slots \(1\) must be greater than or equal to the number of logical replication slots \(2\) on the old cluster/
],
- [qr//],
+ [qr/^$/],
'run of pg_upgrade where the new cluster has insufficient max_replication_slots'
);
ok( -d $newpub->data_dir . "/pg_upgrade_output.d",
@@ -118,7 +118,7 @@ command_checks_all(
[
qr/Your installation contains logical replication slots that can't be upgraded./
],
- [qr//],
+ [qr/^$/],
'run of pg_upgrade of old cluster with slots having unconsumed WAL records'
);
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl b/src/bin/pg_upgrade/t/004_subscription.pl
index df5d6dffbc..c8ee2390d1 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -68,7 +68,7 @@ command_checks_all(
[
qr/max_replication_slots \(0\) must be greater than or equal to the number of subscriptions \(1\) on the old cluster/
],
- [qr//],
+ [qr/^$/],
'run of pg_upgrade where the new cluster has insufficient max_replication_slots'
);
diff --git a/src/bin/psql/t/001_basic.pl b/src/bin/psql/t/001_basic.pl
index 9f0b6cf8ca..ce875ce316 100644
--- a/src/bin/psql/t/001_basic.pl
+++ b/src/bin/psql/t/001_basic.pl
@@ -412,23 +412,23 @@ my $perlbin = $^X;
$perlbin =~ s!\\!/!g if $PostgreSQL::Test::Utils::windows_os;
my $pipe_cmd = "$perlbin -pe '' >$g_file";
-psql_like($node, "SELECT 'one' \\g | $pipe_cmd", qr//, "one command \\g");
+psql_like($node, "SELECT 'one' \\g | $pipe_cmd", qr/.*/, "one command \\g");
my $c1 = slurp_file($g_file);
like($c1, qr/one/);
-psql_like($node, "SELECT 'two' \\; SELECT 'three' \\g | $pipe_cmd", qr//, "two commands \\g");
+psql_like($node, "SELECT 'two' \\; SELECT 'three' \\g | $pipe_cmd", qr/.*/, "two commands \\g");
my $c2 = slurp_file($g_file);
like($c2, qr/two.*three/s);
-psql_like($node, "\\set SHOW_ALL_RESULTS 0\nSELECT 'four' \\; SELECT 'five' \\g | $pipe_cmd", qr//,
+psql_like($node, "\\set SHOW_ALL_RESULTS 0\nSELECT 'four' \\; SELECT 'five' \\g | $pipe_cmd", qr/.*/,
"two commands \\g with only last result");
my $c3 = slurp_file($g_file);
like($c3, qr/five/);
unlike($c3, qr/four/);
psql_like($node, "copy (values ('foo'),('bar')) to stdout \\g | $pipe_cmd",
- qr//,
+ qr/.*/,
"copy output passed to \\g pipe");
my $c4 = slurp_file($g_file);
like($c4, qr/foo.*bar/s);
diff --git a/src/bin/psql/t/010_tab_completion.pl b/src/bin/psql/t/010_tab_completion.pl
index b6575b075e..d3a95ac066 100644
--- a/src/bin/psql/t/010_tab_completion.pl
+++ b/src/bin/psql/t/010_tab_completion.pl
@@ -414,7 +414,7 @@ check_completion(
clear_query();
# check no-completions code path
-check_completion("blarg \t\t", qr//, "check completion failure path");
+check_completion("blarg \t\t", qr/.*/, "check completion failure path");
clear_query();
diff --git a/src/test/recovery/t/037_invalid_database.pl b/src/test/recovery/t/037_invalid_database.pl
index 32b7d8af57..684cdca928 100644
--- a/src/test/recovery/t/037_invalid_database.pl
+++ b/src/test/recovery/t/037_invalid_database.pl
@@ -98,7 +98,7 @@ ok( $bgpsql->query_safe(
"blocked DROP DATABASE completion");
# Try to drop. This will wait due to the still held lock.
-$bgpsql->query_until(qr//, "DROP DATABASE regression_invalid_interrupt;\n");
+$bgpsql->query_until(qr/.*/, "DROP DATABASE regression_invalid_interrupt;\n");
# Ensure we're waiting for the lock
$node->poll_query_until('postgres',
--
2.34.1
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
@ 2024-03-11 21:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 22:51 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
1 sibling, 1 reply; 12+ messages in thread
From: Tom Lane @ 2024-03-11 21:08 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org
Jeff Davis <pgsql@j-davis.com> writes:
> My commit adds a line using the s/// operator.
Oh, independently of the empty-pattern problem: what you added is
+# Numeric major version of old cluster, ignoring "devel" suffix.
+# Needed for testing upgrades from development version to itself.
+my $old_major_version = int($oldnode->pg_version =~ s/devel//rg);
Surely this will break the moment we enter beta. Rather than
trying to fix that directly, what we should be doing is realizing
that you've reinvented -- badly -- the facilities provided by
the PostgreSQL::Version package. The code you changed,
if ($oldnode->pg_version >= 15 && $ENV{with_icu} eq 'yes')
was perfectly correct as it stood, because pg_version is a
PostgreSQL::Version object. Why did you feel a need to
editorialize on that?
regards, tom lane
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 21:08 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
@ 2024-03-11 22:51 ` Jeff Davis <pgsql@j-davis.com>
2024-03-12 20:16 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 12+ messages in thread
From: Jeff Davis @ 2024-03-11 22:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Mon, 2024-03-11 at 17:08 -0400, Tom Lane wrote:
> Jeff Davis <pgsql@j-davis.com> writes:
>
> was perfectly correct as it stood, because pg_version is a
> PostgreSQL::Version object. Why did you feel a need to
> editorialize on that?
The goal was to do a version check for 17 that's inclusive of
development versions.
Patch attached, following the example in AdjustUpgrade.pm. It feels a
bit inconsistent to sometimes use $oldnode->pg_version and sometimes
use $old_major_version, but it's certainly better than what I had done
in f696c0cd5f.
Regards,
Jeff Davis
Attachments:
v1-0001-Another-fix-to-002_pg_upgrade.pl.patch (text/x-patch, 1.6K) download
From ad3ba667553d8c2862b74939676244340631d192 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Mon, 11 Mar 2024 15:32:41 -0700
Subject: [PATCH v1] Another fix to 002_pg_upgrade.pl.
Properly use PostgreSQL::Version, rather than the hacks introduced by
f696c0cd5f.
Discussion: https://postgr.es/m/1553991.1710191312@sss.pgh.pa.us
Reported-by: Tom Lane
---
src/bin/pg_upgrade/t/002_pg_upgrade.pl | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 5ef78691cb..18335070c8 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -92,9 +92,9 @@ my $oldnode =
PostgreSQL::Test::Cluster->new('old_node',
install_path => $ENV{oldinstall});
-# Numeric major version of old cluster, ignoring "devel" suffix.
-# Needed for testing upgrades from development version to itself.
-my $old_major_version = int($oldnode->pg_version =~ s/devel//rg);
+# Needed in some cases for testing upgrades from a development or beta
+# version to itself.
+my $old_major_version = PostgreSQL::Version->new($oldnode->pg_version->major);
my %node_params = ();
@@ -118,7 +118,7 @@ my $original_locale = "C";
my $original_datlocale = "";
my $provider_field = "'c' AS datlocprovider";
my $old_datlocale_field = "NULL AS datlocale";
-if ($old_major_version >= 15 && $ENV{with_icu} eq 'yes')
+if ($oldnode->pg_version >= 15 && $ENV{with_icu} eq 'yes')
{
$provider_field = "datlocprovider";
if ($old_major_version >= 17)
--
2.34.1
* Re: pgsql: Catalog changes preparing for builtin collation provider.
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
2024-03-11 21:08 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 22:51 ` Re: pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <pgsql@j-davis.com>
@ 2024-03-12 20:16 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 12+ messages in thread
From: Tom Lane @ 2024-03-12 20:16 UTC (permalink / raw)
To: Jeff Davis <pgsql@j-davis.com>; +Cc: pgsql-committers@lists.postgresql.org
Jeff Davis <pgsql@j-davis.com> writes:
> Patch attached, following the example in AdjustUpgrade.pm. It feels a
> bit inconsistent to sometimes use $oldnode->pg_version and sometimes
> use $old_major_version, but it's certainly better than what I had done
> in f696c0cd5f.
You're still doing it the hard way. I suggest the attached.
regards, tom lane
Attachments:
simpler-version-check.patch (text/x-diff, 1.1K) download
diff --git a/src/bin/pg_upgrade/t/002_pg_upgrade.pl b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
index 5ef78691cb..34a459496e 100644
--- a/src/bin/pg_upgrade/t/002_pg_upgrade.pl
+++ b/src/bin/pg_upgrade/t/002_pg_upgrade.pl
@@ -92,10 +92,6 @@ my $oldnode =
PostgreSQL::Test::Cluster->new('old_node',
install_path => $ENV{oldinstall});
-# Numeric major version of old cluster, ignoring "devel" suffix.
-# Needed for testing upgrades from development version to itself.
-my $old_major_version = int($oldnode->pg_version =~ s/devel//rg);
-
my %node_params = ();
# To increase coverage of non-standard segment size and group access without
@@ -118,10 +114,10 @@ my $original_locale = "C";
my $original_datlocale = "";
my $provider_field = "'c' AS datlocprovider";
my $old_datlocale_field = "NULL AS datlocale";
-if ($old_major_version >= 15 && $ENV{with_icu} eq 'yes')
+if ($oldnode->pg_version >= 15 && $ENV{with_icu} eq 'yes')
{
$provider_field = "datlocprovider";
- if ($old_major_version >= 17)
+ if ($oldnode->pg_version >= '17devel')
{
$old_datlocale_field = "datlocale";
}
end of thread, other threads:[~2024-03-12 20:16 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-09 22:50 pgsql: Catalog changes preparing for builtin collation provider. Jeff Davis <jdavis@postgresql.org>
2024-03-10 03:53 ` Jeff Davis <pgsql@j-davis.com>
2024-03-10 05:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 19:26 ` Jeff Davis <pgsql@j-davis.com>
2024-03-11 20:15 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 20:21 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 21:17 ` Jeff Davis <pgsql@j-davis.com>
2024-03-11 22:04 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-12 00:10 ` Jeff Davis <pgsql@j-davis.com>
2024-03-11 21:08 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-03-11 22:51 ` Jeff Davis <pgsql@j-davis.com>
2024-03-12 20:16 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox