From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
Cc: | robertmhaas(at)gmail(dot)com, kuroda(dot)hayato(at)fujitsu(dot)com, barwick(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Have pg_basebackup write "dbname" in "primary_conninfo"? |
Date: | 2024-02-23 06:47:07 |
Message-ID: | CAA4eK1Jh=V38dRV059wdbacT9TkQ0oWGcqDfo5V4X2shvKykfA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Feb 21, 2024 at 8:34 AM Kyotaro Horiguchi
<horikyota(dot)ntt(at)gmail(dot)com> wrote:
>
> At Tue, 20 Feb 2024 19:56:10 +0530, Robert Haas <robertmhaas(at)gmail(dot)com> 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.
From | Date | Subject | |
---|---|---|---|
Next Message | Dilip Kumar | 2024-02-23 07:36:26 | Re: SLRU optimization - configurable buffer pool and partitioning the SLRU lock |
Previous Message | Amit Kapila | 2024-02-23 06:04:43 | Re: Have pg_basebackup write "dbname" in "primary_conninfo"? |