public inbox for [email protected]  
help / color / mirror / Atom feed
From: Hayato Kuroda (Fujitsu) <[email protected]>
To: '[email protected]' <[email protected]>
Subject: pg_recvlogical requires -d but not described on the documentation
Date: Fri, 21 Feb 2025 04:25:43 +0000
Message-ID: <OSCPR01MB14966930B4357BAE8C9D68A8AF5C72@OSCPR01MB14966.jpnprd01.prod.outlook.com> (raw)

Dear hackers,

Hi, I found the issue $SUBJECT. According to the doc [1]:

```
-d dbname
--dbname=dbname
The database to connect to. See the description of the actions for what this means in detail.
The dbname can be a connection string. If so, connection string parameters will override any
conflicting command line options. Defaults to the user name.
```

IIUC final statement implies --dbname can be omitted. If it is mandatory, it should be described
in the synopsis part - not written now. However, the command would fail when -d is missed.

```
$ pg_recvlogical -U postgres --start -S test -f -
pg_recvlogical: error: no database specified
pg_recvlogical: hint: Try "pg_recvlogical --help" for more information.
```

ISTM the inconsistency is introduced since the initial commit. I think they should be unified either
1) update the doc or 2) accept when -d is not specified. Personally, I like 2nd approach, pg_recvlogical
can follow the normal connection rule. I.e., 

a) Use PGDATABASE if specified
b) Check 'U' and PGUSER and use it if specified
c) Otherwise use the current OS user name

Attached patch tries to implement the 2nd approach. How do you think?

[1]: https://www.postgresql.org/docs/devel/app-pgrecvlogical.html

Best regards,
Hayato Kuroda
FUJITSU LIMITED



Attachments:

  [application/octet-stream] 0001-pg_recvlogical-accept-if-d-is-not-specified.patch (2.2K, ../OSCPR01MB14966930B4357BAE8C9D68A8AF5C72@OSCPR01MB14966.jpnprd01.prod.outlook.com/2-0001-pg_recvlogical-accept-if-d-is-not-specified.patch)
  download | inline diff:
From d12206817f78721b7739389c23277f959a61c263 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Fri, 21 Feb 2025 13:22:37 +0900
Subject: [PATCH] pg_recvlogical: accept if -d is not specified

---
 src/bin/pg_basebackup/pg_recvlogical.c        | 20 ++++++++++++-------
 src/bin/pg_basebackup/t/030_pg_recvlogical.pl |  4 ++--
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_recvlogical.c b/src/bin/pg_basebackup/pg_recvlogical.c
index b9ea23e142..3fbc2e819e 100644
--- a/src/bin/pg_basebackup/pg_recvlogical.c
+++ b/src/bin/pg_basebackup/pg_recvlogical.c
@@ -20,6 +20,7 @@
 
 #include "common/file_perm.h"
 #include "common/logging.h"
+#include "common/username.h"
 #include "fe_utils/option_utils.h"
 #include "getopt_long.h"
 #include "libpq-fe.h"
@@ -864,6 +865,18 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
+	if (dbname == NULL)
+	{
+		if (getenv("PGDATABASE"))
+			dbname = getenv("PGDATABASE");
+		else if (dbuser)
+			dbname = dbuser;
+		else if (getenv("PGUSER"))
+			dbname = getenv("PGUSER");
+		else
+			dbname = (char *) get_user_name_or_exit(progname);
+	}
+
 	/*
 	 * Required arguments
 	 */
@@ -881,13 +894,6 @@ main(int argc, char **argv)
 		exit(1);
 	}
 
-	if (!do_drop_slot && dbname == NULL)
-	{
-		pg_log_error("no database specified");
-		pg_log_error_hint("Try \"%s --help\" for more information.", progname);
-		exit(1);
-	}
-
 	if (!do_drop_slot && !do_create_slot && !do_start_slot)
 	{
 		pg_log_error("at least one action needs to be specified");
diff --git a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
index a6e1060016..6fccdba63c 100644
--- a/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
+++ b/src/bin/pg_basebackup/t/030_pg_recvlogical.pl
@@ -29,8 +29,8 @@ $node->start;
 
 $node->command_fails(['pg_recvlogical'], 'pg_recvlogical needs a slot name');
 $node->command_fails(
-	[ 'pg_recvlogical', '--slot' => 'test' ],
-	'pg_recvlogical needs a database');
+	[ 'pg_recvlogical', '--slot' => 'test', '--user' => 'postgres' ],
+	'pg_recvlogical needs an action');
 $node->command_fails(
 	[ 'pg_recvlogical', '--slot' => 'test', '--dbname' => 'postgres' ],
 	'pg_recvlogical needs an action');
-- 
2.43.5



view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pg_recvlogical requires -d but not described on the documentation
  In-Reply-To: <OSCPR01MB14966930B4357BAE8C9D68A8AF5C72@OSCPR01MB14966.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox