On 2025/03/21 10:12, Hayato Kuroda (Fujitsu) wrote:
Dear Fujii-san,
Thanks for the patch! It looks good to me.
I'm considering whether to back-patch these changes to older versions.
Since pg_recvlogical --drop-slot worked without --dbname in 9.4
but started failing unintentionally in 9.5, it could be considered a bug.
However, this behavior has existed for a long time without complaints or
bug reports, and there was no clear documentation stating that
--drop-slot should work without --dbname.
Given this, I think that also we could treat it as not a bug and apply
the change only to the master branch. What do you think should we
back-patch it as a bug fix or apply it only to master?
Personally considered, such a long-standing but harmless bug can be regarded as
the specification. So, I vote that this is an enhancement and be applied only to
master.
+1
I've updated the commit messages for both patches and also revised
the code comments in the 0002 patch. The updated patches are attached.
Unless there are any objections, I'm thinking to commit them.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 53360ffb7a908a881a94ff81934dca9a7825d8d1 Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Tue, 18 Mar 2025 10:07:08 +0900
Subject: [PATCH v4] doc: Clarify required options for each action in
pg_recvlogical
Each pg_recvlogical action requires specific options. For example,
--slot, --dbname, and --file must be specified with the --start action.
Previously, the documentation did not clearly outline these requirements.
This commit updates the documentation to explicitly state
the necessary options for each action.
Author: Hayato Kuroda <kuroda.hay...@fujitsu.com>
Co-authored-by: Fujii Masao <masao.fu...@gmail.com>
Reviewed-by: Ashutosh Bapat <ashutosh.bapat....@gmail.com>
Reviewed-by: Vignesh C <vignes...@gmail.com>
Reviewed-by: David G. Johnston <david.g.johns...@gmail.com>
Discussion:
https://postgr.es/m/oscpr01mb14966930b4357bae8c9d68a8af5...@oscpr01mb14966.jpnprd01.prod.outlook.com
---
doc/src/sgml/ref/pg_recvlogical.sgml | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/ref/pg_recvlogical.sgml
b/doc/src/sgml/ref/pg_recvlogical.sgml
index 95eb14b6352..2946bdae1e5 100644
--- a/doc/src/sgml/ref/pg_recvlogical.sgml
+++ b/doc/src/sgml/ref/pg_recvlogical.sgml
@@ -73,6 +73,11 @@ PostgreSQL documentation
by <option>--dbname</option>.
</para>
+ <para>
+ The <option>--slot</option> and <option>--dbname</option> are required
+ for this action.
+ </para>
+
<para>
The <option>--two-phase</option> can be specified with
<option>--create-slot</option> to enable decoding of prepared
transactions.
@@ -87,6 +92,10 @@ PostgreSQL documentation
Drop the replication slot with the name specified
by <option>--slot</option>, then exit.
</para>
+
+ <para>
+ The <option>--slot</option> is required for this action.
+ </para>
</listitem>
</varlistentry>
@@ -101,6 +110,11 @@ PostgreSQL documentation
<option>--no-loop</option> is specified.
</para>
+ <para>
+ The <option>--slot</option> and <option>--dbname</option>,
+ <option>--file</option> are required for this action.
+ </para>
+
<para>
The stream format is determined by the output plugin specified when
the slot was created.
@@ -159,6 +173,9 @@ PostgreSQL documentation
Write received and decoded transaction data into this
file. Use <literal>-</literal> for <systemitem>stdout</systemitem>.
</para>
+ <para>
+ This parameter is required for <option>--start</option>.
+ </para>
</listitem>
</varlistentry>
@@ -265,6 +282,9 @@ PostgreSQL documentation
mode, create the slot with this name. In <option>--drop-slot</option>
mode, delete the slot with this name.
</para>
+ <para>
+ This parameter is required for any of actions.
+ </para>
</listitem>
</varlistentry>
@@ -305,7 +325,11 @@ PostgreSQL documentation
The <replaceable>dbname</replaceable> can be a <link
linkend="libpq-connstring">connection string</link>. If so,
connection string parameters will override any conflicting
- command line options. Defaults to the user name.
+ command line options.
+ </para>
+ <para>
+ This parameter is required for <option>--create-slot</option>
+ and <option>--start</option>.
</para>
</listitem>
</varlistentry>
--
2.48.1
From 44c686ae11cc0de8d0aaf70c23397f94b477a82a Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hay...@fujitsu.com>
Date: Wed, 19 Mar 2025 11:30:16 +0900
Subject: [PATCH v4 2/2] Allow pg_recvlogical --drop-slot to work without
--dbname.
When pg_recvlogical was introduced in 9.4, the --dbname option was not
required for --drop-slot. Without it, pg_recvlogical --drop-slot connected
using a replication connection (not tied to a specific database) and
was able to drop both physical and logical replication slots, similar to
pg_receivewal --drop-slot.
However, commit 0c013e08cfb unintentionally changed this behavior in 9.5,
making pg_recvlogical always check whether it's connected to a specific
database and fail if it's not. This change was expected for --create-slot
and --start, which handle logical replication slots and require a database
connection, but it was unnecessary for --drop-slot, which should work with
any replication connection. As a result, --dbname became a required option
for --drop-slot.
This commit removes that restriction, restoring the original behavior and
allowing pg_recvlogical --drop-slot to work without specifying --dbname.
Although this issue originated from an unintended change, it has existed
for a long time without complaints or bug reports, and the documentation
never explicitly stated that --drop-slot should work without --dbname.
Therefore, the change is not treated as a bug fix and is applied only to
master.
Author: Hayato Kuroda <kuroda.hay...@fujitsu.com>
Reviewed-by: Fujii Masao <masao.fu...@gmail.com>
Discussion:
https://postgr.es/m/b15ecf4f-e5af-4fbb-82c2-a425f453e...@oss.nttdata.com
---
src/bin/pg_basebackup/pg_recvlogical.c | 9 ++++++---
src/bin/pg_basebackup/t/030_pg_recvlogical.pl | 8 ++++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_basebackup/pg_recvlogical.c
b/src/bin/pg_basebackup/pg_recvlogical.c
index b9ea23e1426..a3447753119 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -944,13 +944,16 @@ main(int argc, char **argv)
#endif
/*
- * Run IDENTIFY_SYSTEM to make sure we connected using a database
specific
- * replication connection.
+ * Run IDENTIFY_SYSTEM to check the connection type for each action.
+ * --create-slot and --start actions require a database-specific
+ * replication connection because they handle logical replication slots.
+ * --drop-slot can remove replication slots from any replication
+ * connection without this restriction.
*/
if (!RunIdentifySystem(conn, NULL, NULL, NULL, &db_name))
exit(1);
- if (db_name == NULL)
+ if (!do_drop_slot && db_name == NULL)
pg_fatal("could not establish database-specific replication
connection");
/*
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index a6e10600161..62bbc5a3f98 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -127,4 +127,12 @@ $node->command_ok(
],
'replayed a two-phase transaction');
+$node->command_ok(
+ [
+ 'pg_recvlogical',
+ '--slot' => 'test',
+ '--drop-slot'
+ ],
+ 'drop could work without dbname');
+
done_testing();
--
2.48.1
