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

On Wed, Mar 20, 2024 at 2:24 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> 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.

Thank you for the tests! These results look good to me too.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com