agora inbox for [email protected]
help / color / mirror / Atom feedRe: Cleaning up and speeding up string functions
18+ messages / 10 participants
[nested] [flat]
* Re: Cleaning up and speeding up string functions
@ 2019-06-05 20:54 Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Alvaro Herrera @ 2019-06-05 20:54 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers
On 2019-May-26, David Rowley wrote:
> On Sun, 26 May 2019 at 04:50, Tom Lane <[email protected]> wrote:
> > Here the cost is code space rather than programmer-visible complexity,
> > but I still doubt that it's worth it.
>
> I see on today's master the postgres binary did grow from 8633960
> bytes to 8642504 on my machine using GCC 8.3, so you might be right.
> pg_receivewal grew from 96376 to 96424 bytes.
I suppose one place that could be affected visibly is JSON object
construction (json.c, jsonfuncs.c) that could potentially deal with
millions of stringinfo manipulations, but most of those calls don't
actually use appendStringInfoString with constant values, so it's
probably not worth bothering with.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Cleaning up and speeding up string functions
@ 2019-06-13 05:24 David Rowley <[email protected]>
parent: Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: David Rowley @ 2019-06-13 05:24 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Tom Lane <[email protected]>; pgsql-hackers
On Thu, 6 Jun 2019 at 08:54, Alvaro Herrera <[email protected]> wrote:
>
> On 2019-May-26, David Rowley wrote:
>
> > On Sun, 26 May 2019 at 04:50, Tom Lane <[email protected]> wrote:
>
> > > Here the cost is code space rather than programmer-visible complexity,
> > > but I still doubt that it's worth it.
> >
> > I see on today's master the postgres binary did grow from 8633960
> > bytes to 8642504 on my machine using GCC 8.3, so you might be right.
> > pg_receivewal grew from 96376 to 96424 bytes.
>
> I suppose one place that could be affected visibly is JSON object
> construction (json.c, jsonfuncs.c) that could potentially deal with
> millions of stringinfo manipulations, but most of those calls don't
> actually use appendStringInfoString with constant values, so it's
> probably not worth bothering with.
We could probably get the best of both worlds by using a macro and
__builtin_constant_p() to detect if the string is a const, but I won't
be pushing for that unless I find something to make it worthwhile.
For patch 0004, I think it's likely worth revising so instead of
adding a new function, make use of appendBinaryStringInfo() and pass
in the known length. Likely mostly for the xml.c calls.
--
David Rowley http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-10 04:16 Amul Sul <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Amul Sul @ 2022-01-10 04:16 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, Jan 10, 2022 at 8:25 AM Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2021-11-23 11:43:21 +0530, Amul Sul wrote:
> > Attached patch covers a case where TLI in the filename for a
> > record being read is different from where it belongs to. In other
> > words, it covers following case noted in StartupXLOG():
>
> > Thoughts? Suggestions?
>
> It seems the test isn't quite reliable. It occasionally fails on freebsd,
> macos, linux and always on windows (starting with the new CI stuff, before the
> test wasn't run).
>
> See https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/36/3427
>
Thanks for the note, I can see the same test is failing on my centos
vm too with the latest master head(376ce3e404b). The failing reason is
the "recovery_target_inclusive = off" setting which is unnecessary for
this test, the attached patch removing the same.
Regards,
Amul
Attachments:
[application/x-patch] v2-0001-TAP-test-for-EndOfLogTLI.patch (3.1K, ../../CAAJ_b97D+gyvh-=9VCX7_Cvr9Do-_pRSjrePKmjh4o21Dy9+2w@mail.gmail.com/2-v2-0001-TAP-test-for-EndOfLogTLI.patch)
download | inline diff:
From 88ae9ea5a33c1ecc5b5493dae9c016ef19fbf88f Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Sun, 9 Jan 2022 23:10:07 -0500
Subject: [PATCH v2] TAP test for EndOfLogTLI
---
src/test/recovery/t/003_recovery_targets.pl | 52 ++++++++++++++++++++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 24da78c0bcd..cf72b5d9343 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 9;
+use Test::More tests => 10;
use Time::HiRes qw(usleep);
# Create and test a standby from given backup, with a certain recovery target.
@@ -182,3 +182,53 @@ $logfile = slurp_file($node_standby->logfile());
ok( $logfile =~
qr/FATAL: .* recovery ended before configured recovery target was reached/,
'recovery end before target reached is a fatal error');
+
+# Test to cover a case where that we are looking for WAL record that ought to be
+# in for e.g 000000010000000000000001 we don't find it; instead we find
+# 000000020000000000000003 because of various reasons such as there was a
+# timeline switch in that segment, and we were reading the old WAL from a
+# segment belonging to a higher timeline or our recovery target timeline is 2,
+# or something that has 2 in its history.
+
+# Insert few more data to primary
+$node_primary->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(6001,7000))");
+my $lsn6 = $node_primary->safe_psql('postgres',
+ "SELECT pg_current_wal_lsn()");
+
+# Setup new standby and enable WAL archiving to archive WAL files at the same
+# location as the primary.
+my $archive_cmd = $node_primary->safe_psql('postgres',
+ "SELECT current_setting('archive_command')");
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup(
+ $node_primary, 'my_backup',
+ has_streaming => 1);
+$node_standby->append_conf(
+ 'postgresql.conf', qq(
+archive_mode = on
+archive_command = '$archive_cmd'
+));
+$node_standby->start;
+# Wait until necessary replay has been done on standby
+$node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('write'));
+$node_standby->promote;
+$node_standby->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(7001,8000))");
+# Force archiving of WAL file
+$node_standby->safe_psql('postgres', "SELECT pg_switch_wal()");
+$node_standby->stop;
+
+# Another standby whose recovery target lsn will be in the WAL file has
+# a different TLI than the target LSN belongs to.
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup(
+ $node_primary, 'my_backup',
+ has_restoring => 1);
+$node_standby->append_conf(
+ 'postgresql.conf', qq(recovery_target_lsn = '$lsn6'));
+$node_standby->start;
+my $result = $node_standby->safe_psql('postgres',
+ "SELECT count(*) FROM tab_int");
+is($result, '7000', "check standby content before timeline switch $lsn6");
--
2.18.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-15 06:05 Julien Rouhaud <[email protected]>
parent: Amul Sul <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Julien Rouhaud @ 2022-01-15 06:05 UTC (permalink / raw)
To: Amul Sul <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Mon, Jan 10, 2022 at 09:46:23AM +0530, Amul Sul wrote:
>
> Thanks for the note, I can see the same test is failing on my centos
> vm too with the latest master head(376ce3e404b). The failing reason is
> the "recovery_target_inclusive = off" setting which is unnecessary for
> this test, the attached patch removing the same.
This version still fails on windows according to the cfbot:
https://cirrus-ci.com/task/5882621321281536
[18:14:02.639] c:\cirrus>call perl src/tools/msvc/vcregress.pl recoverycheck
[18:14:56.114]
[18:14:56.122] # Failed test 'check standby content before timeline switch 0/500FB30'
[18:14:56.122] # at t/003_recovery_targets.pl line 234.
[18:14:56.122] # got: '6000'
[18:14:56.122] # expected: '7000'
I'm switching the cf entry to Waiting on Author.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-17 11:37 Amul Sul <[email protected]>
parent: Julien Rouhaud <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Amul Sul @ 2022-01-17 11:37 UTC (permalink / raw)
To: Julien Rouhaud <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, Jan 15, 2022 at 11:35 AM Julien Rouhaud <[email protected]> wrote:
>
> Hi,
>
> On Mon, Jan 10, 2022 at 09:46:23AM +0530, Amul Sul wrote:
> >
> > Thanks for the note, I can see the same test is failing on my centos
> > vm too with the latest master head(376ce3e404b). The failing reason is
> > the "recovery_target_inclusive = off" setting which is unnecessary for
> > this test, the attached patch removing the same.
>
> This version still fails on windows according to the cfbot:
>
> https://cirrus-ci.com/task/5882621321281536
>
> [18:14:02.639] c:\cirrus>call perl src/tools/msvc/vcregress.pl recoverycheck
> [18:14:56.114]
> [18:14:56.122] # Failed test 'check standby content before timeline switch 0/500FB30'
> [18:14:56.122] # at t/003_recovery_targets.pl line 234.
> [18:14:56.122] # got: '6000'
> [18:14:56.122] # expected: '7000'
>
> I'm switching the cf entry to Waiting on Author.
Thanks for the note.
I am not sure what really went wrong but I think the 'standby_9'
server shutdown too early before it gets a chance to archive a
required WAL file. The attached patch tries to shutdown that server
after the required WAL are archived, unfortunately, I couldn't test
that on the window, let see how cfbot reacts to this version.
Regards,
Amul
Attachments:
[application/x-patch] v3-0001-TAP-test-for-EndOfLogTLI.patch (3.4K, ../../CAAJ_b97G9tErQ5CdpDAOeER-eNVW2M2tofYKhyDv1d-9f5w2jQ@mail.gmail.com/2-v3-0001-TAP-test-for-EndOfLogTLI.patch)
download | inline diff:
From 08d19c0ef2f464e8bf722ce13457acf3f9be47e8 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Mon, 17 Jan 2022 06:25:30 -0500
Subject: [PATCH v3] TAP test for EndOfLogTLI
---
src/test/recovery/t/003_recovery_targets.pl | 57 ++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 24da78c0bcd..928799b9490 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 9;
+use Test::More tests => 10;
use Time::HiRes qw(usleep);
# Create and test a standby from given backup, with a certain recovery target.
@@ -182,3 +182,58 @@ $logfile = slurp_file($node_standby->logfile());
ok( $logfile =~
qr/FATAL: .* recovery ended before configured recovery target was reached/,
'recovery end before target reached is a fatal error');
+
+# Test to cover a case where that we are looking for WAL record that ought to be
+# in for e.g 000000010000000000000001 we don't find it; instead we find
+# 000000020000000000000003 because of various reasons such as there was a
+# timeline switch in that segment, and we were reading the old WAL from a
+# segment belonging to a higher timeline or our recovery target timeline is 2,
+# or something that has 2 in its history.
+
+# Insert few more data to primary
+$node_primary->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(6001,7000))");
+my $lsn6 = $node_primary->safe_psql('postgres',
+ "SELECT pg_current_wal_lsn()");
+
+# Setup new standby and enable WAL archiving to archive WAL files at the same
+# location as the primary.
+my $archive_cmd = $node_primary->safe_psql('postgres',
+ "SELECT current_setting('archive_command')");
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup(
+ $node_primary, 'my_backup',
+ has_streaming => 1);
+$node_standby->append_conf(
+ 'postgresql.conf', qq(
+archive_mode = on
+archive_command = '$archive_cmd'
+));
+$node_standby->start;
+# Wait until necessary replay has been done on standby
+$node_primary->wait_for_catchup($node_standby, 'replay',
+ $node_primary->lsn('write'));
+$node_standby->promote;
+$node_standby->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(7001,8000))");
+# Force archiving of WAL file
+my $last_wal = $node_standby->safe_psql('postgres',
+ "SELECT pg_walfile_name(pg_switch_wal())");
+# Wait until this WAL file archive
+my $check_archive = "SELECT last_archived_wal >= '$last_wal' FROM pg_stat_archiver";
+$node_standby->poll_query_until('postgres', $check_archive)
+ or die "Timed out while waiting for $last_wal file archive";
+$node_standby->stop;
+
+# Another standby whose recovery target lsn will be in the WAL file has
+# a different TLI than the target LSN belongs to.
+$node_standby = PostgreSQL::Test::Cluster->new('standby_10');
+$node_standby->init_from_backup(
+ $node_primary, 'my_backup',
+ has_restoring => 1);
+$node_standby->append_conf(
+ 'postgresql.conf', qq(recovery_target_lsn = '$lsn6'));
+$node_standby->start;
+my $result = $node_standby->safe_psql('postgres',
+ "SELECT count(*) FROM tab_int");
+is($result, '7000', "check standby content before timeline switch $lsn6");
--
2.18.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-17 13:33 Julien Rouhaud <[email protected]>
parent: Amul Sul <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Julien Rouhaud @ 2022-01-17 13:33 UTC (permalink / raw)
To: Amul Sul <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Mon, Jan 17, 2022 at 05:07:48PM +0530, Amul Sul wrote:
>
> I am not sure what really went wrong but I think the 'standby_9'
> server shutdown too early before it gets a chance to archive a
> required WAL file. The attached patch tries to shutdown that server
> after the required WAL are archived, unfortunately, I couldn't test
> that on the window, let see how cfbot reacts to this version.
Thanks for the updated patch! Note that thanks to Andres and Thomas work, you
can now easily rely on the exact same CI than the cfbot on your own github
repository, if you need to debug something on a platform you don't have access
to. You can check the documentation at [1] for more detail on how to setup the
CI.
[1] https://github.com/postgres/postgres/blob/master/src/tools/ci/README
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-18 04:25 Julien Rouhaud <[email protected]>
parent: Julien Rouhaud <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Julien Rouhaud @ 2022-01-18 04:25 UTC (permalink / raw)
To: Amul Sul <[email protected]>; +Cc: Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
Hi,
On Mon, Jan 17, 2022 at 09:33:57PM +0800, Julien Rouhaud wrote:
>
> Thanks for the updated patch! Note that thanks to Andres and Thomas work, you
> can now easily rely on the exact same CI than the cfbot on your own github
> repository, if you need to debug something on a platform you don't have access
> to. You can check the documentation at [1] for more detail on how to setup the
> CI.
The cfbot reports that the patch still fails on Windows but also failed on
macos with the same error: https://cirrus-ci.com/task/5655777858813952:
[14:20:43.950] # Failed test 'check standby content before timeline switch 0/500FAF8'
[14:20:43.950] # at t/003_recovery_targets.pl line 239.
[14:20:43.950] # got: '6000'
[14:20:43.950] # expected: '7000'
[14:20:43.950] # Looks like you failed 1 test of 10.
I'm switching the CF entry to Waiting on Author.
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-18 05:01 Kyotaro Horiguchi <[email protected]>
parent: Julien Rouhaud <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Kyotaro Horiguchi @ 2022-01-18 05:01 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]
At Tue, 18 Jan 2022 12:25:15 +0800, Julien Rouhaud <[email protected]> wrote in
> Hi,
>
> On Mon, Jan 17, 2022 at 09:33:57PM +0800, Julien Rouhaud wrote:
> >
> > Thanks for the updated patch! Note that thanks to Andres and Thomas work, you
> > can now easily rely on the exact same CI than the cfbot on your own github
> > repository, if you need to debug something on a platform you don't have access
> > to. You can check the documentation at [1] for more detail on how to setup the
> > CI.
>
> The cfbot reports that the patch still fails on Windows but also failed on
> macos with the same error: https://cirrus-ci.com/task/5655777858813952:
>
> [14:20:43.950] # Failed test 'check standby content before timeline switch 0/500FAF8'
> [14:20:43.950] # at t/003_recovery_targets.pl line 239.
> [14:20:43.950] # got: '6000'
> [14:20:43.950] # expected: '7000'
> [14:20:43.950] # Looks like you failed 1 test of 10.
>
> I'm switching the CF entry to Waiting on Author.
The most significant advantages of the local CI setup are
- CI immediately responds to push.
- You can dirty the code with additional logging aid as much as you
like to see closely what is going on. It makes us hesitant to do
the same on this ML:p
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: TAP test to cover "EndOfLogTLI != replayTLI" case
@ 2022-01-20 06:43 Amul Sul <[email protected]>
parent: Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Amul Sul @ 2022-01-20 06:43 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; Andres Freund <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jan 18, 2022 at 10:31 AM Kyotaro Horiguchi
<[email protected]> wrote:
>
> At Tue, 18 Jan 2022 12:25:15 +0800, Julien Rouhaud <[email protected]> wrote in
> > Hi,
> >
> > On Mon, Jan 17, 2022 at 09:33:57PM +0800, Julien Rouhaud wrote:
> > >
> > > Thanks for the updated patch! Note that thanks to Andres and Thomas work, you
> > > can now easily rely on the exact same CI than the cfbot on your own github
> > > repository, if you need to debug something on a platform you don't have access
> > > to. You can check the documentation at [1] for more detail on how to setup the
> > > CI.
> >
> > The cfbot reports that the patch still fails on Windows but also failed on
> > macos with the same error: https://cirrus-ci.com/task/5655777858813952:
> >
> > [14:20:43.950] # Failed test 'check standby content before timeline switch 0/500FAF8'
> > [14:20:43.950] # at t/003_recovery_targets.pl line 239.
> > [14:20:43.950] # got: '6000'
> > [14:20:43.950] # expected: '7000'
> > [14:20:43.950] # Looks like you failed 1 test of 10.
> >
> > I'm switching the CF entry to Waiting on Author.
>
> The most significant advantages of the local CI setup are
>
> - CI immediately responds to push.
>
> - You can dirty the code with additional logging aid as much as you
> like to see closely what is going on. It makes us hesitant to do
> the same on this ML:p
>
Indeed :)
I found the cause for the test failing on window -- is due to the
custom archive command setting which wasn't setting the correct window
archive directory path.
There is no option to choose a custom wal archive and restore patch in
the TAP test. The attach 0001 patch proposes the same, which enables
you to choose a custom WAL archive and restore directory. The only
concern I have is that $node->info() will print the wrong archive
directory path in that case, do we need to fix that? We might need to
store archive_dir like base_dir, I wasn't sure about that. Thoughts?
Comments?
Regards,
Amul
Attachments:
[application/x-patch] v4-0001-Allow-TAP-tests-to-choose-custom-WAL-archive-and-.patch (1.8K, ../../CAAJ_b94hsL4_aboiyPipfAEMzaW=OVFSd5h7sqCc+R49f3tY1g@mail.gmail.com/2-v4-0001-Allow-TAP-tests-to-choose-custom-WAL-archive-and-.patch)
download | inline diff:
From 4bc968d3508e9f608fd572f40f39e2cb374d53f4 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Wed, 19 Jan 2022 23:22:02 -0500
Subject: [PATCH v4 1/3] Allow TAP tests to choose custom WAL archive and
restore path.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index b7d4c24553..6a562db2ce 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -1033,10 +1033,13 @@ primary_conninfo='$root_connstr'
# Internal routine to enable archive recovery command on a standby node
sub enable_restoring
{
- my ($self, $root_node, $standby) = @_;
- my $path = PostgreSQL::Test::Utils::perl2host($root_node->archive_dir);
+ my ($self, $root_node, $standby, $path) = @_;
my $name = $self->name;
+ # Default archive directory will be used if not specified.
+ $path = $root_node->archive_dir if (!defined($path));
+ $path = PostgreSQL::Test::Utils::perl2host($path);
+
print "### Enabling WAL restore for node \"$name\"\n";
# On Windows, the path specified in the restore command needs to use
@@ -1101,10 +1104,13 @@ sub set_standby_mode
# Internal routine to enable archiving
sub enable_archiving
{
- my ($self) = @_;
- my $path = PostgreSQL::Test::Utils::perl2host($self->archive_dir);
+ my ($self, $path) = @_;
my $name = $self->name;
+ # Default archive directory will be used if not specified.
+ $path = $self->archive_dir if (!defined($path));
+ $path = PostgreSQL::Test::Utils::perl2host($path);
+
print "### Enabling WAL archiving for node \"$name\"\n";
# On Windows, the path specified in the restore command needs to use
--
2.18.0
[application/x-patch] v4-0002-TAP-test-for-EndOfLogTLI.patch (3.1K, ../../CAAJ_b94hsL4_aboiyPipfAEMzaW=OVFSd5h7sqCc+R49f3tY1g@mail.gmail.com/3-v4-0002-TAP-test-for-EndOfLogTLI.patch)
download | inline diff:
From 6c730ee10b9e64d90982aa555283c9094a5fdb87 Mon Sep 17 00:00:00 2001
From: Amul Sul <[email protected]>
Date: Wed, 19 Jan 2022 23:25:07 -0500
Subject: [PATCH v4 2/3] TAP test for EndOfLogTLI
---
src/test/recovery/t/003_recovery_targets.pl | 47 ++++++++++++++++++++-
1 file changed, 46 insertions(+), 1 deletion(-)
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 24da78c0bc..b3f7076540 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -6,7 +6,7 @@ use strict;
use warnings;
use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
-use Test::More tests => 9;
+use Test::More tests => 10;
use Time::HiRes qw(usleep);
# Create and test a standby from given backup, with a certain recovery target.
@@ -182,3 +182,48 @@ $logfile = slurp_file($node_standby->logfile());
ok( $logfile =~
qr/FATAL: .* recovery ended before configured recovery target was reached/,
'recovery end before target reached is a fatal error');
+
+# Test to cover a case where that we are looking for WAL record that ought to be
+# in for e.g 000000010000000000000005 we don't find it; instead we find
+# 000000020000000000000005 because of various reasons such as there was a
+# timeline switch in that segment, and we were reading the old WAL from a
+# segment belonging to a higher timeline or our recovery target timeline is 2,
+# or something that has 2 in its history.
+
+# Insert few more data to primary
+$node_primary->safe_psql('postgres', "SELECT pg_switch_wal()");
+$node_primary->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(6001,7000))");
+my $lsn6 = $node_primary->safe_psql('postgres',
+ "SELECT pg_current_wal_lsn()");
+
+# Setup new standby and enable WAL archiving to archive WAL files at the same
+# location as the primary.
+my $archive_dir = $node_primary->archive_dir;
+$node_standby = PostgreSQL::Test::Cluster->new('standby_9');
+$node_standby->init_from_backup(
+ $node_primary, 'my_backup',
+ has_streaming => 1);
+$node_standby->enable_archiving($archive_dir);
+$node_standby->start;
+# Wait until necessary replay has been done on standby
+$node_primary->wait_for_catchup($node_standby);
+$node_standby->promote;
+$node_standby->safe_psql('postgres',
+ "INSERT INTO tab_int VALUES (generate_series(7001,8000))");
+# Force archiving of WAL file
+my $last_wal = $node_standby->safe_psql('postgres',
+ "SELECT pg_walfile_name(pg_switch_wal())");
+# Wait until this WAL file archive
+my $check_archive = "SELECT last_archived_wal >= '$last_wal' FROM pg_stat_archiver";
+$node_standby->poll_query_until('postgres', $check_archive)
+ or die "Timed out while waiting for $last_wal file archive";
+$node_standby->stop;
+
+# Another standby whose recovery target lsn will be in the WAL file has
+# a different TLI than the target LSN belongs to.
+@recovery_params = (
+ "recovery_target_lsn = '$lsn6'",
+ "recovery_target_inclusive = off");
+test_recovery_standby("target LSN belong to different TLI",
+ 'standby_10', $node_primary, \@recovery_params, "7000", $lsn6);
--
2.18.0
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH] Expand character set for ltree labels
@ 2022-10-04 16:54 Garen Torikian <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Garen Torikian @ 2022-10-04 16:54 UTC (permalink / raw)
To: [email protected]
Dear hackers,
I am submitting a patch to expand the label requirements for ltree.
The current format is restricted to alphanumeric characters, plus _.
Unfortunately, for non-English labels, this set is insufficient. Rather
than figure out how to expand this set to include characters beyond the
ASCII limit, I have instead opted to provide users with some mechanism for
storing encoded UTF-8 characters which is widely used: punycode (
https://en.wikipedia.org/wiki/Punycode).
The punycode range of characters is the exact same set as the existing
ltree range, with the addition of a hyphen (-). Within this system, any
human language can be encoded using just A-Za-z0-9-.
On top of this, I added support for two more characters: # and ;, which are
used for HTML entities. Note that & and % have special significance in the
existing ltree logic; users would have to encode items as #20; (rather than
%20). This seems a fair compromise.
Since the encoding could make a regular slug even longer, I have also
doubled the character limit, from 256 to 512.
Please let me know if I can provide any more information or changes.
Very sincerely,
Garen
Attachments:
[application/octet-stream] 0001-Expand-character-set-for-ltree-labels.patch (15.3K, ../../CAGXsc+-mNg9Gc0rp-ER0sv+zkZSZp2wE9-LX6XcoWSLVz22tZA@mail.gmail.com/3-0001-Expand-character-set-for-ltree-labels.patch)
download | inline diff:
From 0fcef15b15879c73ff3e93b492fe02fe6ea7628f Mon Sep 17 00:00:00 2001
From: "Garen J. Torikian" <[email protected]>
Date: Tue, 4 Oct 2022 12:45:12 -0400
Subject: [PATCH] Expand character set for ltree labels This patch expands the
character set for ltree labels to include four additional characters. These
characters can be used to represent non-alphanumeric characters, via punycode
or HTML encoding.
Furthermore, the label length is doubled to account for potentially
longer strings when saving an encoded format.
---
contrib/ltree/expected/ltree.out | 72 ++++++++++++++++++++++++--------
contrib/ltree/ltree.h | 13 ++++--
contrib/ltree/ltree_io.c | 10 ++---
contrib/ltree/ltxtquery_io.c | 4 +-
contrib/ltree/sql/ltree.sql | 19 ++++++---
doc/src/sgml/ltree.sgml | 6 +--
6 files changed, 87 insertions(+), 37 deletions(-)
diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index c6d8f3ef75..6581c32b28 100644
--- a/contrib/ltree/expected/ltree.out
+++ b/contrib/ltree/expected/ltree.out
@@ -25,6 +25,24 @@ SELECT '1.2'::ltree;
1.2
(1 row)
+SELECT '1.2.#3'::ltree;
+ ltree
+--------
+ 1.2.#3
+(1 row)
+
+SELECT '1.2.-3'::ltree;
+ ltree
+--------
+ 1.2.-3
+(1 row)
+
+SELECT '1.2.;3'::ltree;
+ ltree
+--------
+ 1.2.;3
+(1 row)
+
SELECT '1.2._3'::ltree;
ltree
--------
@@ -45,15 +63,15 @@ ERROR: ltree syntax error
LINE 1: SELECT '1.2.'::ltree;
^
DETAIL: Unexpected end of input.
-SELECT repeat('x', 255)::ltree;
- repeat
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+SELECT repeat('x', 511)::ltree;
+ repeat
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
(1 row)
-SELECT repeat('x', 256)::ltree;
+SELECT repeat('x', 512)::ltree;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 257.
+DETAIL: Label length is 512, must be at most 511, at character 513.
SELECT ltree2text('1.2.3.34.sdf');
ltree2text
--------------
@@ -531,24 +549,24 @@ SELECT '1.2.3|@.4'::lquery;
ERROR: lquery syntax error at character 7
LINE 1: SELECT '1.2.3|@.4'::lquery;
^
-SELECT (repeat('x', 255) || '*@@*')::lquery;
- lquery
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@*
+SELECT (repeat('x', 511) || '*@@*')::lquery;
+ lquery
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@*
(1 row)
-SELECT (repeat('x', 256) || '*@@*')::lquery;
+SELECT (repeat('x', 512) || '*@@*')::lquery;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 257.
-SELECT ('!' || repeat('x', 255))::lquery;
- lquery
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- !xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+DETAIL: Label length is 512, must be at most 511, at character 513.
+SELECT ('!' || repeat('x', 511))::lquery;
+ lquery
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ !xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
(1 row)
-SELECT ('!' || repeat('x', 256))::lquery;
+SELECT ('!' || repeat('x', 512))::lquery;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 258.
+DETAIL: Label length is 512, must be at most 511, at character 514.
SELECT nlevel('1.2.3.4');
nlevel
--------
@@ -1195,6 +1213,24 @@ SELECT 'tree & aw_qw%*'::ltxtquery;
tree & aw_qw%*
(1 row)
+SELECT 'tree & aw#qw%*'::ltxtquery;
+ ltxtquery
+----------------
+ tree & aw#qw%*
+(1 row)
+
+SELECT 'tree & aw-qw%*'::ltxtquery;
+ ltxtquery
+----------------
+ tree & aw-qw%*
+(1 row)
+
+SELECT 'tree & aw;qw%*'::ltxtquery;
+ ltxtquery
+----------------
+ tree & aw;qw%*
+(1 row)
+
SELECT 'ltree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
?column?
----------
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 40aed0ca0c..cd585fa042 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -12,10 +12,10 @@
/*
* We want the maximum length of a label to be encoding-independent, so
- * set it somewhat arbitrarily at 255 characters (not bytes), while using
+ * set it somewhat arbitrarily at 511 characters (not bytes), while using
* uint16 fields to hold the byte length.
*/
-#define LTREE_LABEL_MAX_CHARS 255
+#define LTREE_LABEL_MAX_CHARS 511
/*
* LOWER_NODE used to be defined in the Makefile via the compile flags.
@@ -126,7 +126,14 @@ typedef struct
#define LQUERY_HASNOT 0x01
-#define ISALNUM(x) ( t_isalpha(x) || t_isdigit(x) || ( pg_mblen(x) == 1 && t_iseq((x), '_') ) )
+#define ISPRINT(x) (( pg_mblen(x) == 1 && \
+ (t_iseq((x), '#') || \
+ t_iseq((x), '-') || \
+ t_iseq((x), ';') || \
+ t_iseq((x), '_'))))
+#define ISALNUM(x) ( t_isalpha(x) || t_isdigit(x))
+
+#define ISVALID(x) ( ISALNUM(x) || ISPRINT(x) )
/* full text query */
diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c
index 15115cb29f..b1dd7104e0 100644
--- a/contrib/ltree/ltree_io.c
+++ b/contrib/ltree/ltree_io.c
@@ -74,7 +74,7 @@ parse_ltree(const char *buf)
switch (state)
{
case LTPRS_WAITNAME:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
lptr->start = ptr;
lptr->wlen = 0;
@@ -91,7 +91,7 @@ parse_ltree(const char *buf)
lptr++;
state = LTPRS_WAITNAME;
}
- else if (!ISALNUM(ptr))
+ else if (!ISVALID(ptr))
UNCHAR;
break;
default:
@@ -310,7 +310,7 @@ parse_lquery(const char *buf)
switch (state)
{
case LQPRS_WAITLEVEL:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
GETVAR(curqlevel) = lptr = (nodeitem *) palloc0(sizeof(nodeitem) * (numOR + 1));
lptr->start = ptr;
@@ -333,7 +333,7 @@ parse_lquery(const char *buf)
UNCHAR;
break;
case LQPRS_WAITVAR:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
lptr++;
lptr->start = ptr;
@@ -376,7 +376,7 @@ parse_lquery(const char *buf)
state = LQPRS_WAITLEVEL;
curqlevel = NEXTLEV(curqlevel);
}
- else if (ISALNUM(ptr))
+ else if (ISVALID(ptr))
{
/* disallow more chars after a flag */
if (lptr->flag)
diff --git a/contrib/ltree/ltxtquery_io.c b/contrib/ltree/ltxtquery_io.c
index 3eca5cb8ff..dfe641399e 100644
--- a/contrib/ltree/ltxtquery_io.c
+++ b/contrib/ltree/ltxtquery_io.c
@@ -76,7 +76,7 @@ gettoken_query(QPRS_STATE *state, int32 *val, int32 *lenval, char **strval, uint
(state->buf)++;
return OPEN;
}
- else if (ISALNUM(state->buf))
+ else if (ISVALID(state->buf))
{
state->state = INOPERAND;
*strval = state->buf;
@@ -89,7 +89,7 @@ gettoken_query(QPRS_STATE *state, int32 *val, int32 *lenval, char **strval, uint
errmsg("operand syntax error")));
break;
case INOPERAND:
- if (ISALNUM(state->buf))
+ if (ISVALID(state->buf))
{
if (*flag)
ereport(ERROR,
diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql
index bf733ed17b..612c2072dc 100644
--- a/contrib/ltree/sql/ltree.sql
+++ b/contrib/ltree/sql/ltree.sql
@@ -8,6 +8,9 @@ WHERE opc.oid >= 16384 AND NOT amvalidate(opc.oid);
SELECT ''::ltree;
SELECT '1'::ltree;
SELECT '1.2'::ltree;
+SELECT '1.2.#3'::ltree;
+SELECT '1.2.-3'::ltree;
+SELECT '1.2.;3'::ltree;
SELECT '1.2._3'::ltree;
-- empty labels not allowed
@@ -15,8 +18,8 @@ SELECT '.2.3'::ltree;
SELECT '1..3'::ltree;
SELECT '1.2.'::ltree;
-SELECT repeat('x', 255)::ltree;
-SELECT repeat('x', 256)::ltree;
+SELECT repeat('x', 511)::ltree;
+SELECT repeat('x', 512)::ltree;
SELECT ltree2text('1.2.3.34.sdf');
SELECT text2ltree('1.2.3.34.sdf');
@@ -111,10 +114,10 @@ SELECT '1.!.3'::lquery;
SELECT '1.2.!'::lquery;
SELECT '1.2.3|@.4'::lquery;
-SELECT (repeat('x', 255) || '*@@*')::lquery;
-SELECT (repeat('x', 256) || '*@@*')::lquery;
-SELECT ('!' || repeat('x', 255))::lquery;
-SELECT ('!' || repeat('x', 256))::lquery;
+SELECT (repeat('x', 511) || '*@@*')::lquery;
+SELECT (repeat('x', 512) || '*@@*')::lquery;
+SELECT ('!' || repeat('x', 511))::lquery;
+SELECT ('!' || repeat('x', 512))::lquery;
SELECT nlevel('1.2.3.4');
SELECT nlevel(('1' || repeat('.1', 65534))::ltree);
@@ -233,6 +236,10 @@ SELECT 'QWER_GY'::ltree ~ 'q_t%@*';
--ltxtquery
SELECT '!tree & aWdf@*'::ltxtquery;
SELECT 'tree & aw_qw%*'::ltxtquery;
+SELECT 'tree & aw#qw%*'::ltxtquery;
+SELECT 'tree & aw-qw%*'::ltxtquery;
+SELECT 'tree & aw;qw%*'::ltxtquery;
+
SELECT 'ltree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
SELECT 'tree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
SELECT 'tree.awdfg'::ltree @ '!tree | aWdf@*'::ltxtquery;
diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml
index 508f404ae8..144bccd2c5 100644
--- a/doc/src/sgml/ltree.sgml
+++ b/doc/src/sgml/ltree.sgml
@@ -24,9 +24,9 @@
<para>
A <firstterm>label</firstterm> is a sequence of alphanumeric characters
- and underscores (for example, in C locale the characters
- <literal>A-Za-z0-9_</literal> are allowed).
- Labels must be less than 256 characters long.
+ and a subset of printable characters. In C locale, the characters
+ <literal>A-Za-z0-9#-;_</literal> are allowed.
+ Labels must be less than 512 characters long.
</para>
<para>
--
2.37.3
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-04 22:32 Nathan Bossart <[email protected]>
parent: Garen Torikian <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Nathan Bossart @ 2022-10-04 22:32 UTC (permalink / raw)
To: Garen Torikian <[email protected]>; +Cc: [email protected]
On Tue, Oct 04, 2022 at 12:54:46PM -0400, Garen Torikian wrote:
> The punycode range of characters is the exact same set as the existing
> ltree range, with the addition of a hyphen (-). Within this system, any
> human language can be encoded using just A-Za-z0-9-.
IIUC ASCII characters like '!' and '<' are valid Punycode characters, but
even with your proposal, those wouldn't be allowed.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-04 23:16 Garen Torikian <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Garen Torikian @ 2022-10-04 23:16 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: [email protected]
No, not quite.
Valid Punycode characters are `[A-Za-z0-9-]`. This proposal includes `-`,
as well as `#` and `;` for HTML entities.
I double-checked the RFC to see the valid Punycode characters and the set
above is indeed correct:
https://datatracker.ietf.org/doc/html/draft-ietf-idn-punycode-02#section-5
While it would be nice for ltree labels to support *any* printable
character, it can't because symbols like `!` and `%` already have special
meaning in the querying. This proposal leaves those as is and does not
depend on any existing special character.
On Tue, Oct 4, 2022 at 6:32 PM Nathan Bossart <[email protected]>
wrote:
> On Tue, Oct 04, 2022 at 12:54:46PM -0400, Garen Torikian wrote:
> > The punycode range of characters is the exact same set as the existing
> > ltree range, with the addition of a hyphen (-). Within this system, any
> > human language can be encoded using just A-Za-z0-9-.
>
> IIUC ASCII characters like '!' and '<' are valid Punycode characters, but
> even with your proposal, those wouldn't be allowed.
>
> --
> Nathan Bossart
> Amazon Web Services: https://aws.amazon.com
>
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-05 18:59 Tom Lane <[email protected]>
parent: Garen Torikian <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Tom Lane @ 2022-10-05 18:59 UTC (permalink / raw)
To: Garen Torikian <[email protected]>; +Cc: [email protected]
Garen Torikian <[email protected]> writes:
> I am submitting a patch to expand the label requirements for ltree.
> The current format is restricted to alphanumeric characters, plus _.
> Unfortunately, for non-English labels, this set is insufficient.
Hm? Perhaps the docs are a bit unclear about that, but it's not
restricted to ASCII alphanumerics. AFAICS the code will accept
whatever iswalpha() and iswdigit() will accept in the database's
default locale. There's certainly work that could/should be done
to allow use of not-so-default locales, but that's not specific
to ltree. I'm not sure that doing an application-side encoding
is attractive compared to just using that ability directly.
If you do want to do application-side encoding, I'm unsure why
punycode would be the choice anyway, as opposed to something
that can fit in the existing restrictions.
> On top of this, I added support for two more characters: # and ;, which are
> used for HTML entities.
That seems really pretty random.
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-05 19:34 Garen Torikian <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Garen Torikian @ 2022-10-05 19:34 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
Hi Tom,
> Perhaps the docs are a bit unclear about that, but it's not
> restricted to ASCII alphanumerics. AFAICS the code will accept
> whatever iswalpha() and iswdigit() will accept in the database's
> default locale.
Sorry but I don't think that is correct. Here is the single
definition check of what constitutes a valid character:
https://github.com/postgres/postgres/blob/c3315a7da57be720222b119385ed0f7ad7c15268/contrib/ltree/ltr...
As you can see, there are no `is_*` calls at all. Where in this contrib
package do you see `iswalpha`? Perhaps I missed it.
> That seems really pretty random.
Ok. I am trying to avoid a situation where other users may wish to use
other delimiters other than `-`, due to its commonplace presence in words
(eg., compound ones).
On Wed, Oct 5, 2022 at 2:59 PM Tom Lane <[email protected]> wrote:
> Garen Torikian <[email protected]> writes:
> > I am submitting a patch to expand the label requirements for ltree.
>
> > The current format is restricted to alphanumeric characters, plus _.
> > Unfortunately, for non-English labels, this set is insufficient.
>
> Hm? Perhaps the docs are a bit unclear about that, but it's not
> restricted to ASCII alphanumerics. AFAICS the code will accept
> whatever iswalpha() and iswdigit() will accept in the database's
> default locale. There's certainly work that could/should be done
> to allow use of not-so-default locales, but that's not specific
> to ltree. I'm not sure that doing an application-side encoding
> is attractive compared to just using that ability directly.
>
> If you do want to do application-side encoding, I'm unsure why
> punycode would be the choice anyway, as opposed to something
> that can fit in the existing restrictions.
>
> > On top of this, I added support for two more characters: # and ;, which
> are
> > used for HTML entities.
>
> That seems really pretty random.
>
> regards, tom lane
>
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-05 19:56 Tom Lane <[email protected]>
parent: Garen Torikian <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Tom Lane @ 2022-10-05 19:56 UTC (permalink / raw)
To: Garen Torikian <[email protected]>; +Cc: [email protected]
Garen Torikian <[email protected]> writes:
>> Perhaps the docs are a bit unclear about that, but it's not
>> restricted to ASCII alphanumerics. AFAICS the code will accept
>> whatever iswalpha() and iswdigit() will accept in the database's
>> default locale.
> Sorry but I don't think that is correct. Here is the single
> definition check of what constitutes a valid character:
> https://github.com/postgres/postgres/blob/c3315a7da57be720222b119385ed0f7ad7c15268/contrib/ltree/ltr...
> As you can see, there are no `is_*` calls at all.
Did you chase down what t_isalpha and t_isdigit do?
regards, tom lane
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-10-05 22:05 Garen Torikian <[email protected]>
parent: Tom Lane <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Garen Torikian @ 2022-10-05 22:05 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
After digging into it, you are completely correct. I had to do a bit more
reading to understand the relationships between UTF-8 and wchar, but
ultimately the existing locale support works for my use case.
Therefore I have updated the patch with three much smaller changes:
* Support for `-` in addition to `_`
* Expanding the limit to 512 chars (from the existing 256); again it's not
uncommon for non-English strings to be much longer
* Fixed the documentation to expand on what the ltree label's relationship
to the DB locale is
Thank you,
Garen
On Wed, Oct 5, 2022 at 3:56 PM Tom Lane <[email protected]> wrote:
> Garen Torikian <[email protected]> writes:
> >> Perhaps the docs are a bit unclear about that, but it's not
> >> restricted to ASCII alphanumerics. AFAICS the code will accept
> >> whatever iswalpha() and iswdigit() will accept in the database's
> >> default locale.
>
> > Sorry but I don't think that is correct. Here is the single
> > definition check of what constitutes a valid character:
> >
> https://github.com/postgres/postgres/blob/c3315a7da57be720222b119385ed0f7ad7c15268/contrib/ltree/ltr...
>
> > As you can see, there are no `is_*` calls at all.
>
> Did you chase down what t_isalpha and t_isdigit do?
>
> regards, tom lane
>
Attachments:
[application/octet-stream] 0002-Expand-character-set-for-ltree-labels.patch (14.5K, ../../CAGXsc+_rjeifDg1Xb7O==nkRg+K9DgrKuZUNOPj0zhN4+9vnBA@mail.gmail.com/3-0002-Expand-character-set-for-ltree-labels.patch)
download | inline diff:
commit 3cb57bb601d28b923e0c6c5d0f5a938c9abf68aa
Author: Garen J. Torikian <[email protected]>
Date: Wed Oct 5 17:59:54 2022 -0400
Expand character set for ltree labels
This patch expands the character set for ltree labels to one additional character:
the hyphen.
Furthermore, the label length is doubled to account for longer labels.
diff --git a/contrib/ltree/expected/ltree.out b/contrib/ltree/expected/ltree.out
index c6d8f3ef75..0f558bd7b4 100644
--- a/contrib/ltree/expected/ltree.out
+++ b/contrib/ltree/expected/ltree.out
@@ -25,6 +25,12 @@ SELECT '1.2'::ltree;
1.2
(1 row)
+SELECT '1.2.-3'::ltree;
+ ltree
+--------
+ 1.2.-3
+(1 row)
+
SELECT '1.2._3'::ltree;
ltree
--------
@@ -45,15 +51,15 @@ ERROR: ltree syntax error
LINE 1: SELECT '1.2.'::ltree;
^
DETAIL: Unexpected end of input.
-SELECT repeat('x', 255)::ltree;
- repeat
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+SELECT repeat('x', 511)::ltree;
+ repeat
+---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
(1 row)
-SELECT repeat('x', 256)::ltree;
+SELECT repeat('x', 512)::ltree;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 257.
+DETAIL: Label length is 512, must be at most 511, at character 513.
SELECT ltree2text('1.2.3.34.sdf');
ltree2text
--------------
@@ -531,24 +537,24 @@ SELECT '1.2.3|@.4'::lquery;
ERROR: lquery syntax error at character 7
LINE 1: SELECT '1.2.3|@.4'::lquery;
^
-SELECT (repeat('x', 255) || '*@@*')::lquery;
- lquery
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@*
+SELECT (repeat('x', 511) || '*@@*')::lquery;
+ lquery
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@*
(1 row)
-SELECT (repeat('x', 256) || '*@@*')::lquery;
+SELECT (repeat('x', 512) || '*@@*')::lquery;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 257.
-SELECT ('!' || repeat('x', 255))::lquery;
- lquery
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- !xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+DETAIL: Label length is 512, must be at most 511, at character 513.
+SELECT ('!' || repeat('x', 511))::lquery;
+ lquery
+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
+ !xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
(1 row)
-SELECT ('!' || repeat('x', 256))::lquery;
+SELECT ('!' || repeat('x', 512))::lquery;
ERROR: label string is too long
-DETAIL: Label length is 256, must be at most 255, at character 258.
+DETAIL: Label length is 512, must be at most 511, at character 514.
SELECT nlevel('1.2.3.4');
nlevel
--------
@@ -1195,6 +1201,12 @@ SELECT 'tree & aw_qw%*'::ltxtquery;
tree & aw_qw%*
(1 row)
+SELECT 'tree & aw-qw%*'::ltxtquery;
+ ltxtquery
+----------------
+ tree & aw-qw%*
+(1 row)
+
SELECT 'ltree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
?column?
----------
diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h
index 40aed0ca0c..a9c9f881b0 100644
--- a/contrib/ltree/ltree.h
+++ b/contrib/ltree/ltree.h
@@ -12,10 +12,10 @@
/*
* We want the maximum length of a label to be encoding-independent, so
- * set it somewhat arbitrarily at 255 characters (not bytes), while using
+ * set it somewhat arbitrarily at 511 characters (not bytes), while using
* uint16 fields to hold the byte length.
*/
-#define LTREE_LABEL_MAX_CHARS 255
+#define LTREE_LABEL_MAX_CHARS 511
/*
* LOWER_NODE used to be defined in the Makefile via the compile flags.
@@ -126,7 +126,12 @@ typedef struct
#define LQUERY_HASNOT 0x01
-#define ISALNUM(x) ( t_isalpha(x) || t_isdigit(x) || ( pg_mblen(x) == 1 && t_iseq((x), '_') ) )
+#define ISDASH(x) (( pg_mblen(x) == 1 && \
+ (t_iseq((x), '-') || \
+ t_iseq((x), '_'))))
+#define ISALNUM(x) ( t_isalpha(x) || t_isdigit(x))
+
+#define ISVALID(x) ( ISALNUM(x) || ISDASH(x) )
/* full text query */
diff --git a/contrib/ltree/ltree_io.c b/contrib/ltree/ltree_io.c
index 15115cb29f..b1dd7104e0 100644
--- a/contrib/ltree/ltree_io.c
+++ b/contrib/ltree/ltree_io.c
@@ -74,7 +74,7 @@ parse_ltree(const char *buf)
switch (state)
{
case LTPRS_WAITNAME:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
lptr->start = ptr;
lptr->wlen = 0;
@@ -91,7 +91,7 @@ parse_ltree(const char *buf)
lptr++;
state = LTPRS_WAITNAME;
}
- else if (!ISALNUM(ptr))
+ else if (!ISVALID(ptr))
UNCHAR;
break;
default:
@@ -310,7 +310,7 @@ parse_lquery(const char *buf)
switch (state)
{
case LQPRS_WAITLEVEL:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
GETVAR(curqlevel) = lptr = (nodeitem *) palloc0(sizeof(nodeitem) * (numOR + 1));
lptr->start = ptr;
@@ -333,7 +333,7 @@ parse_lquery(const char *buf)
UNCHAR;
break;
case LQPRS_WAITVAR:
- if (ISALNUM(ptr))
+ if (ISVALID(ptr))
{
lptr++;
lptr->start = ptr;
@@ -376,7 +376,7 @@ parse_lquery(const char *buf)
state = LQPRS_WAITLEVEL;
curqlevel = NEXTLEV(curqlevel);
}
- else if (ISALNUM(ptr))
+ else if (ISVALID(ptr))
{
/* disallow more chars after a flag */
if (lptr->flag)
diff --git a/contrib/ltree/ltxtquery_io.c b/contrib/ltree/ltxtquery_io.c
index 3eca5cb8ff..dfe641399e 100644
--- a/contrib/ltree/ltxtquery_io.c
+++ b/contrib/ltree/ltxtquery_io.c
@@ -76,7 +76,7 @@ gettoken_query(QPRS_STATE *state, int32 *val, int32 *lenval, char **strval, uint
(state->buf)++;
return OPEN;
}
- else if (ISALNUM(state->buf))
+ else if (ISVALID(state->buf))
{
state->state = INOPERAND;
*strval = state->buf;
@@ -89,7 +89,7 @@ gettoken_query(QPRS_STATE *state, int32 *val, int32 *lenval, char **strval, uint
errmsg("operand syntax error")));
break;
case INOPERAND:
- if (ISALNUM(state->buf))
+ if (ISVALID(state->buf))
{
if (*flag)
ereport(ERROR,
diff --git a/contrib/ltree/sql/ltree.sql b/contrib/ltree/sql/ltree.sql
index bf733ed17b..373a7c6569 100644
--- a/contrib/ltree/sql/ltree.sql
+++ b/contrib/ltree/sql/ltree.sql
@@ -8,6 +8,7 @@ WHERE opc.oid >= 16384 AND NOT amvalidate(opc.oid);
SELECT ''::ltree;
SELECT '1'::ltree;
SELECT '1.2'::ltree;
+SELECT '1.2.-3'::ltree;
SELECT '1.2._3'::ltree;
-- empty labels not allowed
@@ -15,8 +16,8 @@ SELECT '.2.3'::ltree;
SELECT '1..3'::ltree;
SELECT '1.2.'::ltree;
-SELECT repeat('x', 255)::ltree;
-SELECT repeat('x', 256)::ltree;
+SELECT repeat('x', 511)::ltree;
+SELECT repeat('x', 512)::ltree;
SELECT ltree2text('1.2.3.34.sdf');
SELECT text2ltree('1.2.3.34.sdf');
@@ -111,10 +112,10 @@ SELECT '1.!.3'::lquery;
SELECT '1.2.!'::lquery;
SELECT '1.2.3|@.4'::lquery;
-SELECT (repeat('x', 255) || '*@@*')::lquery;
-SELECT (repeat('x', 256) || '*@@*')::lquery;
-SELECT ('!' || repeat('x', 255))::lquery;
-SELECT ('!' || repeat('x', 256))::lquery;
+SELECT (repeat('x', 511) || '*@@*')::lquery;
+SELECT (repeat('x', 512) || '*@@*')::lquery;
+SELECT ('!' || repeat('x', 511))::lquery;
+SELECT ('!' || repeat('x', 512))::lquery;
SELECT nlevel('1.2.3.4');
SELECT nlevel(('1' || repeat('.1', 65534))::ltree);
@@ -233,6 +234,8 @@ SELECT 'QWER_GY'::ltree ~ 'q_t%@*';
--ltxtquery
SELECT '!tree & aWdf@*'::ltxtquery;
SELECT 'tree & aw_qw%*'::ltxtquery;
+SELECT 'tree & aw-qw%*'::ltxtquery;
+
SELECT 'ltree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
SELECT 'tree.awdfg'::ltree @ '!tree & aWdf@*'::ltxtquery;
SELECT 'tree.awdfg'::ltree @ '!tree | aWdf@*'::ltxtquery;
diff --git a/doc/src/sgml/ltree.sgml b/doc/src/sgml/ltree.sgml
index 508f404ae8..42f659537f 100644
--- a/doc/src/sgml/ltree.sgml
+++ b/doc/src/sgml/ltree.sgml
@@ -23,10 +23,11 @@
<title>Definitions</title>
<para>
- A <firstterm>label</firstterm> is a sequence of alphanumeric characters
- and underscores (for example, in C locale the characters
- <literal>A-Za-z0-9_</literal> are allowed).
- Labels must be less than 256 characters long.
+ A <firstterm>label</firstterm> is a sequence of alphanumeric characters,
+ underscores, and hyphens. Valid alphanumerics character ranges are dependent on
+ your database locale. For example, in C locale, the characters
+ <literal>A-Za-z0-9-_</literal> are allowed.
+ Labels must be less than 512 characters long.
</para>
<para>
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: [PATCH] Expand character set for ltree labels
@ 2022-11-03 23:37 Ian Lawrence Barwick <[email protected]>
parent: Garen Torikian <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Ian Lawrence Barwick @ 2022-11-03 23:37 UTC (permalink / raw)
To: Garen Torikian <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected]
2022年10月6日(木) 7:05 Garen Torikian <[email protected]>:
>
> After digging into it, you are completely correct. I had to do a bit more reading to understand the relationships between UTF-8 and wchar, but ultimately the existing locale support works for my use case.
>
> Therefore I have updated the patch with three much smaller changes:
>
> * Support for `-` in addition to `_`
> * Expanding the limit to 512 chars (from the existing 256); again it's not uncommon for non-English strings to be much longer
> * Fixed the documentation to expand on what the ltree label's relationship to the DB locale is
>
> Thank you,
> Garen
This entry was marked as "Needs review" in the CommitFest app but cfbot
reports the patch no longer applies.
We've marked it as "Waiting on Author". As CommitFest 2022-11 is
currently underway, this would be an excellent time update the patch.
Once you think the patchset is ready for review again, you (or any
interested party) can move the patch entry forward by visiting
https://commitfest.postgresql.org/40/3929/
and changing the status to "Needs review".
Thanks
Ian Barwick
^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v2 1/2] wip: Log VACUUM information as DEBUG1 when VERBOSE is not used
@ 2023-02-08 02:12 Andres Freund <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Andres Freund @ 2023-02-08 02:12 UTC (permalink / raw)
Otherwise it's very hard to debug some kinds of issues, as one cannot use
VACUUM VERBOSE in regression tests.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/access/heap/vacuumlazy.c | 57 ++++++++++++++++++++++------
1 file changed, 45 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 8f14cf85f38..f6f3bb4c021 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -180,6 +180,7 @@ typedef struct LVRelState
OffsetNumber offnum; /* used only for heap operations */
VacErrPhase phase;
bool verbose; /* VACUUM VERBOSE? */
+ int elevel;
/*
* dead_items stores TIDs whose index tuples are deleted by index
@@ -324,10 +325,33 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
StartPageDirty = VacuumPageDirty;
ErrorContextCallback errcallback;
char **indnames = NULL;
+ int elevel;
+
+ if (params->options & VACOPT_VERBOSE)
+ {
+ verbose = true;
+ elevel = INFO;
+ instrument = true;
+ }
+ else if (IsAutoVacuumWorkerProcess() && params->log_min_duration >= 0)
+ {
+ verbose = true;
+ elevel = LOG;
+ instrument = true;
+ }
+ else if (message_level_is_interesting(DEBUG1))
+ {
+ verbose = true;
+ elevel = DEBUG1;
+ instrument = true;
+ }
+ else
+ {
+ verbose = false;
+ elevel = DEBUG5; /* avoid compiler warning, not reached */
+ instrument = false;
+ }
- verbose = (params->options & VACOPT_VERBOSE) != 0;
- instrument = (verbose || (IsAutoVacuumWorkerProcess() &&
- params->log_min_duration >= 0));
if (instrument)
{
pg_rusage_init(&ru0);
@@ -361,6 +385,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
vacrel->indname = NULL;
vacrel->phase = VACUUM_ERRCB_PHASE_UNKNOWN;
vacrel->verbose = verbose;
+ vacrel->elevel = elevel;
errcallback.callback = vacuum_error_callback;
errcallback.arg = vacrel;
errcallback.previous = error_context_stack;
@@ -475,12 +500,12 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
if (verbose)
{
if (vacrel->aggressive)
- ereport(INFO,
+ ereport(elevel,
(errmsg("aggressively vacuuming \"%s.%s.%s\"",
vacrel->dbname, vacrel->relnamespace,
vacrel->relname)));
else
- ereport(INFO,
+ ereport(elevel,
(errmsg("vacuuming \"%s.%s.%s\"",
vacrel->dbname, vacrel->relnamespace,
vacrel->relname)));
@@ -765,7 +790,7 @@ heap_vacuum_rel(Relation rel, VacuumParams *params,
(unsigned long long) walusage.wal_bytes);
appendStringInfo(&buf, _("system usage: %s"), pg_rusage_show(&ru0));
- ereport(verbose ? INFO : LOG,
+ ereport(elevel,
(errmsg_internal("%s", buf.data)));
pfree(buf.data);
}
@@ -2707,7 +2732,11 @@ lazy_vacuum_one_index(Relation indrel, IndexBulkDeleteResult *istat,
ivinfo.analyze_only = false;
ivinfo.report_progress = false;
ivinfo.estimated_count = true;
- ivinfo.message_level = DEBUG2;
+ /*
+ * FIXME: This shows more information for VACUUM VERBOSE, uses DEBUG1
+ * instead of DEBUG2.
+ */
+ ivinfo.message_level = vacrel->elevel;
ivinfo.num_heap_tuples = reltuples;
ivinfo.strategy = vacrel->bstrategy;
@@ -2755,7 +2784,11 @@ lazy_cleanup_one_index(Relation indrel, IndexBulkDeleteResult *istat,
ivinfo.analyze_only = false;
ivinfo.report_progress = false;
ivinfo.estimated_count = estimated_count;
- ivinfo.message_level = DEBUG2;
+ /*
+ * FIXME: This shows more information for VACUUM VERBOSE, uses DEBUG1
+ * instead of DEBUG2.
+ */
+ ivinfo.message_level = vacrel->elevel;
ivinfo.num_heap_tuples = reltuples;
ivinfo.strategy = vacrel->bstrategy;
@@ -2875,7 +2908,7 @@ lazy_truncate_heap(LVRelState *vacrel)
* We failed to establish the lock in the specified number of
* retries. This means we give up truncating.
*/
- ereport(vacrel->verbose ? INFO : DEBUG2,
+ ereport(vacrel->elevel,
(errmsg("\"%s\": stopping truncate due to conflicting lock request",
vacrel->relname)));
return;
@@ -2945,7 +2978,7 @@ lazy_truncate_heap(LVRelState *vacrel)
vacrel->removed_pages += orig_rel_pages - new_rel_pages;
vacrel->rel_pages = new_rel_pages;
- ereport(vacrel->verbose ? INFO : DEBUG2,
+ ereport(vacrel->elevel,
(errmsg("table \"%s\": truncated %u to %u pages",
vacrel->relname,
orig_rel_pages, new_rel_pages)));
@@ -3007,7 +3040,7 @@ count_nondeletable_pages(LVRelState *vacrel, bool *lock_waiter_detected)
{
if (LockHasWaitersRelation(vacrel->rel, AccessExclusiveLock))
{
- ereport(vacrel->verbose ? INFO : DEBUG2,
+ ereport(vacrel->elevel,
(errmsg("table \"%s\": suspending truncate due to conflicting lock request",
vacrel->relname)));
@@ -3175,7 +3208,7 @@ dead_items_alloc(LVRelState *vacrel, int nworkers)
vacrel->pvs = parallel_vacuum_init(vacrel->rel, vacrel->indrels,
vacrel->nindexes, nworkers,
max_items,
- vacrel->verbose ? INFO : DEBUG2,
+ vacrel->elevel,
vacrel->bstrategy);
/* If parallel mode started, dead_items space is allocated in DSM */
--
2.38.0
--2olgvfw3dklisbsr--
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2023-02-08 02:12 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05 20:54 Re: Cleaning up and speeding up string functions Alvaro Herrera <[email protected]>
2019-06-13 05:24 ` David Rowley <[email protected]>
2022-01-10 04:16 Re: TAP test to cover "EndOfLogTLI != replayTLI" case Amul Sul <[email protected]>
2022-01-15 06:05 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Julien Rouhaud <[email protected]>
2022-01-17 11:37 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Amul Sul <[email protected]>
2022-01-17 13:33 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Julien Rouhaud <[email protected]>
2022-01-18 04:25 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Julien Rouhaud <[email protected]>
2022-01-18 05:01 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Kyotaro Horiguchi <[email protected]>
2022-01-20 06:43 ` Re: TAP test to cover "EndOfLogTLI != replayTLI" case Amul Sul <[email protected]>
2022-10-04 16:54 [PATCH] Expand character set for ltree labels Garen Torikian <[email protected]>
2022-10-04 22:32 ` Re: [PATCH] Expand character set for ltree labels Nathan Bossart <[email protected]>
2022-10-04 23:16 ` Re: [PATCH] Expand character set for ltree labels Garen Torikian <[email protected]>
2022-10-05 18:59 ` Re: [PATCH] Expand character set for ltree labels Tom Lane <[email protected]>
2022-10-05 19:34 ` Re: [PATCH] Expand character set for ltree labels Garen Torikian <[email protected]>
2022-10-05 19:56 ` Re: [PATCH] Expand character set for ltree labels Tom Lane <[email protected]>
2022-10-05 22:05 ` Re: [PATCH] Expand character set for ltree labels Garen Torikian <[email protected]>
2022-11-03 23:37 ` Re: [PATCH] Expand character set for ltree labels Ian Lawrence Barwick <[email protected]>
2023-02-08 02:12 [PATCH v2 1/2] wip: Log VACUUM information as DEBUG1 when VERBOSE is not used Andres Freund <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox