On Wed, Feb 21, 2024 at 4:09 PM Hayato Kuroda (Fujitsu) <
kuroda.hay...@fujitsu.com> wrote:
> Dear Horiguchi-san,
>
> > 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)
>
> I was also missing, but the requirement was that the dbname should be
> included
> only when the dbname option was explicitly specified [1]. Even mine and
> yours
> cannot handle like that. Libpq function
> PQconnectdbParams()->pqConnectOptions2()
> fills all the parameter to PGconn, at that time the information whether it
> is
> intentionally specified or not is discarded. Then,
> GenerateRecoveryConfig() would
> just write down all the connection parameters from PGconn, they cannot
> recognize.
>
> Well, one option is that if a default dbname should be written to the
configuration file, then "postgres' is a better option than "replication"
or "username" as the default option, at least a db of that name exists.
regards,
Ajin Cherian
Fujitsu Australia
