Re: Allow specifying a dbname in pg_basebackup connection string - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Allow specifying a dbname in pg_basebackup connection string
Date
Msg-id 2879235.1730746652@sss.pgh.pa.us
Whole thread Raw
In response to Re: Allow specifying a dbname in pg_basebackup connection string  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Allow specifying a dbname in pg_basebackup connection string
Re: Allow specifying a dbname in pg_basebackup connection string
List pgsql-hackers
Daniel Gustafsson <daniel@yesql.se> writes:
> +1 on your patch, reading through I can't see anything it misses.  Maybe a
> comment above the change with a note on why dbname is tested for NULL there
> would be good?

I thought Jelte's suggestion of clarifying the first comment was good:

-    /* pg_recvlogical uses dbname only; others use connection_string only. */
+    /*
+     * pg_recvlogical uses dbname only; others use connection_string only.
+     * (Note: both variables will be NULL if there's no command line options.)
+     */
     Assert(dbname == NULL || connection_string == NULL);

Also, I realized that the associated documentation is pretty much a
lie:

        ... any database
        name in the connection string will be ignored
        by <productname>PostgreSQL</productname>.

Maybe that's true for a very narrow interpretation of "PostgreSQL",
but I think most people would consider that pg_basebackup's lookup
of ~/.pgpass entries is covered.  I'm intending to go with the
attached.

            regards, tom lane

diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index 4f99340c1d..c2d721208b 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -807,14 +807,18 @@ PostgreSQL documentation
         will override any conflicting command line options.
        </para>
        <para>
-        The option is called <literal>--dbname</literal> for consistency with other
+        This option is called <literal>--dbname</literal> for consistency with other
         client applications, but because <application>pg_basebackup</application>
         doesn't connect to any particular database in the cluster, any database
-        name in the connection string will be ignored
-        by <productname>PostgreSQL</productname>. Middleware, or proxies, used in
-        connecting to <productname>PostgreSQL</productname> might however
-        utilize the value. The database name specified in connection string can
-        also be used by <link linkend="logicaldecoding-replication-slots-synchronization">
+        name included in the connection string will be ignored by the server.
+        However, a database name supplied that way overrides the default
+        database name (<literal>replication</literal>) for purposes of
+        looking up the replication connection's password
+        in <filename>~/.pgpass</filename>.  Similarly, middleware or proxies
+        used in connecting to <productname>PostgreSQL</productname> might
+        utilize the name for purposes such as connection routing.  The
+        database name can also be used
+        by <link linkend="logicaldecoding-replication-slots-synchronization">
         logical replication slot synchronization</link>.
        </para>
       </listitem>
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 30b3d9a377..9b86482546 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -74,7 +74,10 @@ GetConnection(void)
     PQconninfoOption *conn_opt;
     char       *err_msg = NULL;

-    /* pg_recvlogical uses dbname only; others use connection_string only. */
+    /*
+     * pg_recvlogical uses dbname only; others use connection_string only.
+     * (Note: both variables will be NULL if there's no command line options.)
+     */
     Assert(dbname == NULL || connection_string == NULL);

     /*
@@ -120,12 +123,12 @@ GetConnection(void)
         keywords = pg_malloc0((argcount + 1) * sizeof(*keywords));
         values = pg_malloc0((argcount + 1) * sizeof(*values));
         keywords[i] = "dbname";
-        values[i] = dbname;
+        values[i] = (dbname == NULL) ? "replication" : dbname;
         i++;
     }

     keywords[i] = "replication";
-    values[i] = dbname == NULL ? "true" : "database";
+    values[i] = (dbname == NULL) ? "true" : "database";
     i++;
     keywords[i] = "fallback_application_name";
     values[i] = progname;

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: pg_dump --no-comments confusion
Next
From: Alexander Korotkov
Date:
Subject: Re: pgsql: Implement pg_wal_replay_wait() stored procedure