Re: Have pg_basebackup write "dbname" in "primary_conninfo"?

On Tue, 19 Mar 2024 at 17:18, Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Dear Sawada-san,
>
> Thanks for giving comments!
>
> > This behavior makes sense to me. But do we want to handle the case of
> > using environment variables too?
>
> Yeah, v5 does not consider which libpq parameters are specified by environment
> variables. Such a variable should be used when the dbname is not expressly written
> in the connection string.
> Such a path was added in the v6 patch. If the dbname is not determined after
> parsing the connection string, we call PQconndefaults() to get settings from
> environment variables and service files [1], then start to search dbname again.
> Below shows an example.
>
> ```
> PGPORT=5431 PGUSER=kuroda PGDATABASE=postgres pg_basebackup -D data_N2 -R -v
> ->
> primary_conninfo = 'user=kuroda ... port=5431 ... dbname=postgres ... '
> ```
>
> > IIUC,
> >
> > pg_basebackup -D tmp -d "user=masahiko dbname=test_db"
> >
> > is equivalent to:
> >
> > PGDATABASE="user=masahiko dbname=test_db" pg_basebackup -D tmp
>
> The case won't work. I think You assumed that expanded_dbname like
> PQconnectdbParams() [2] can be used for enviroment variables, but it is not correct
> - it won't parse as connection string again.
>
> In the libpq layer, connection parameters are parsed in PQconnectStartParams()->conninfo_array_parse().
> When expand_dbname is specified, the entry "dbname" is firstly checked and
> parsed its value. They are done at fe-connect.c:5846.
>
> The environment variables are checked and parsed in conninfo_add_defaults(), which
> is called from conninfo_array_parse(). However, it is done at fe-connect.c:5956 - the
> expand_dbname has already been done at that time. This means there is no chance
> that PGDATABASE is parsed as an expanded style.
>
> For example, if the pg_basebackup runs like below:
>
> PGDATABASE="user=kuroda dbname=postgres" pg_basebackup -D data_N2 -R -v
>
> The primary_conninfo written in the file will be:
>
> primary_conninfo = 'user=hayato ... dbname=''user=kuroda dbname=postgres'''

Thanks for the patch.
Here are the test results for various tests by specifying connection
string, environment variable, service file, and connection URIs with
the patch:
case 1:
pg_basebackup -D test10 -p 5431 -X s -P -R -d "user=vignesh dbname=db1"
primary_conninfo will have dbname=db1

case 2:
pg_basebackup -D test10 -p 5431 -X s -P -R -d "user=vignesh dbname=replication"
primary_conninfo will have dbname=replication

case 3:
pg_basebackup -D test10 -p 5431 -X s -P -R -U vignesh
primary_conninfo will not have dbname

case 4:
pg_basebackup -D test10 -p 5431 -X s -P -R
primary_conninfo will not have dbname

case 5:
pg_basebackup -D test10 -p 5431 -X s -P -R -d "user=vignesh"
primary_conninfo will not have dbname

case 6:
pg_basebackup -D test10 -p 5431 -X s -P -R -d ""
primary_conninfo will not have dbname

--- Testing through PGDATABASE environment variable
case 7:
export PGDATABASE="user=postgres dbname=test"
./pg_basebackup -D test10 -p 5431 -X s -P -R
primary_conninfo will have dbname=''user=postgres dbname=test'' like below:
primary_conninfo = 'user=vignesh passfile=''/home/vignesh/.pgpass''
channel_binding=prefer port=5431 sslmode=prefer sslcompression=0
sslcertmode=allow sslsni=1 ssl_min_protocol_version=TLSv1.2
gssencmode=disable krbsrvname=postgres gssdelegation=0
target_session_attrs=any load_balance_hosts=disable
dbname=''user=postgres dbname=test'''

case 8:
export PGDATABASE=db1
./pg_basebackup -D test10 -p 5431 -X s -P -R
primary_conninfo will have dbname=db1

--- Testing through pg_service
case 9:
Create .pg_service.conf with the following info:
[conn1]
dbname=db2

export PGSERVICE=conn1

./pg_basebackup -D test10 -p 5431 -X s -P -R
primary_conninfo will have dbname=db2

case 10:
Create .pg_service.conf with the following info, i.e. there is no
database specified:
[conn1]

./pg_basebackup -D test10 -p 5431 -X s -P -R
primary_conninfo will not have dbname

--- Testing through Connection URIs
case 11:
./pg_basebackup -D test10 -X s -P -R -d "postgresql://localhost:5431"
primary_conninfo will not have dbname

case 12:
./pg_basebackup -D test10 -p 5431 -X s -P -R -d
"postgresql://localhost/db3:5431"
primary_conninfo will have dbname=''db3:5431'' like below:
primary_conninfo = 'user=vignesh passfile=''/home/vignesh/.pgpass''
channel_binding=prefer host=localhost port=5431 sslmode=prefer
sslcompression=0 sslcertmode=allow sslsni=1
ssl_min_protocol_version=TLSv1.2 gssencmode=disable
krbsrvname=postgres gssdelegation=0 target_session_attrs=any
load_balance_hosts=disable dbname=''db3:5431'''

case 13:
./pg_basebackup -D test10 -p 5431 -X s -P -R -d "postgresql://localhost/db3"
primary_conninfo will have dbname=db3

case 14:
./pg_basebackup -D test10 -X s -P -R -d "postgresql://localhost:5431/db3"
primary_conninfo will have dbname=db3

case 15:
./pg_basebackup -D test10 -X s -P -R -d
"postgresql://localhost:5431/db4,127.0.0.1:5431/db5"
primary_conninfo will have dbname=''db4,127.0.0.1:5431/db5'' like below:
primary_conninfo = 'user=vignesh passfile=''/home/vignesh/.pgpass''
channel_binding=prefer host=localhost port=5431 sslmode=prefer
sslcompression=0 sslcertmode=allow sslsni=1
ssl_min_protocol_version=TLSv1.2 gssencmode=disable
krbsrvname=postgres gssdelegation=0 target_session_attrs=any
load_balance_hosts=disable dbname=''db4,127.0.0.1:5431/db5'''

case 16:
./pg_basebackup -D test10 -X s -P -R -d
"postgresql://localhost:5431,127.0.0.1:5431/db5"
primary_conninfo will have dbname=db5

case 17:
./pg_basebackup -D test10 -X s -P -R -d
"postgresql:///db6?host=localhost&port=5431"
primary_conninfo will have dbname=db6

case 18:
./pg_basebackup -D test10 -p 5431 -X s -P -R -d
"postgresql:///db7?host=/home/vignesh/postgres/inst/bin"
primary_conninfo will have dbname=db7

case 19:
./pg_basebackup -D test10 -p 5431 -X s -P -R -d
"postgresql:///db8?host=%2Fhome%2Fvignesh%2Fpostgres%2Finst%2Fbin"
primary_conninfo will have dbname=db8

In these cases, the database name specified will be written to the
conf file. The test results look good to me.

Regards,
Vignesh