public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Have pg_basebackup write "dbname" in "primary_conninfo"?
Date: Fri, 23 Feb 2024 12:17:07 +0530
Message-ID: <CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+TgmoYfagyp5AvKMfGpRUgXNJZ0ymuqiWTzOc2tV_ZMM0UkCA@mail.gmail.com>
<TYCPR01MB1207777150173002564A753B8F5502@TYCPR01MB12077.jpnprd01.prod.outlook.com>
<CA+TgmoaLRCcCeY3_n7h0rw-r-UfJt_GiFZiJQ1pFZ-iQNUozug@mail.gmail.com>
<[email protected]>
On Wed, Feb 21, 2024 at 8:34 AM Kyotaro Horiguchi
<[email protected]> wrote:
>
> At Tue, 20 Feb 2024 19:56:10 +0530, Robert Haas <[email protected]> wrote in
> > It seems like maybe somebody should look into why this is happening,
> > and perhaps fix it.
>
> GetConnection()@streamutil.c wants to ensure conninfo has a fallback
> database name ("replication"). However, the function seems to be
> ignoring the case where neither dbname nor connection string is given,
> which is the first case Kuroda-san raised. The second case is the
> intended behavior of the function.
>
> > /* pg_recvlogical uses dbname only; others use connection_string only. */
> > Assert(dbname == NULL || connection_string == NULL);
>
> And the function incorrectly assumes that the connection string
> requires "dbname=replication".
>
> > * Merge the connection info inputs given in form of connection string,
> > * options and default values (dbname=replication, replication=true, etc.)
>
> But the name is a pseudo database name only used by pg_hba.conf
> (maybe) , which cannot be used as an actual database name (without
> quotation marks, or unless it is actually created). The function
> should not add the fallback database name because the connection
> string for physical replication connection doesn't require the dbname
> parameter. (attached patch)
>
We do append dbname=replication even in libpqrcv_connect for .pgpass
lookup as mentioned in comments. See below:
libpqrcv_connect()
{
....
else
{
/*
* The database name is ignored by the server in replication mode,
* but specify "replication" for .pgpass lookup.
*/
keys[++i] = "dbname";
vals[i] = "replication";
}
...
}
I think as part of this effort we should just add dbname to
primary_conninfo written in postgresql.auto.conf file. As above, the
question is valid whether we should do it just for 17 or for prior
versions. Let's discuss more on that. I am not sure of the use case
for versions before 17 but commit cca97ce6a665 mentioned that some
middleware or proxies might however need to know the dbname to make
the correct routing decision for the connection. Does that apply here
as well? If so, we can do it and update the docs, otherwise, let's do
it just for 17.
--
With Regards,
Amit Kapila.
view thread (13+ messages) latest in thread
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], [email protected], [email protected], [email protected]
Subject: Re: Have pg_basebackup write "dbname" in "primary_conninfo"?
In-Reply-To: <CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.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