Re: Allow specifying a dbname in pg_basebackup connection string

Daniel Gustafsson <daniel@yesql.se> writes:
> On 18 Sep 2023, at 14:11, Daniel Gustafsson <daniel@yesql.se> wrote:
>> Unless something sticks out in a second pass over it I will go ahead and apply
>> it.

> And applied, closing the CF entry.

I believe this patch (commit cca97ce6a) is the cause of bug #18685,
which reports that pg_basebackup fails to use ~/.pgpass for
connections [1].  Specifically, it replaced this code:

-   keywords[i] = "dbname";
-   values[i] = dbname == NULL ? "replication" : dbname;

with this code:

+       keywords[i] = "dbname";
+       values[i] = dbname;

perhaps under the illusion that dbname and connection_string can't
both be NULL.  But that ends in passing no dbname to libpq, rather
than passing "replication" which is the correct thing.  Apparently
that has no effect on the server connection, but it does break
matching of such connections to ~/.pgpass.

I think the attached will fix it, but I wonder if there are edge
cases I'm not thinking of.

            regards, tom lane

[1] https://www.postgresql.org/message-id/18685-fee2dd142b9688f1%40postgresql.org

diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index 30b3d9a377..d074b5ddd7 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -120,7 +120,7 @@ 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++;
     }