On Wed, Mar 20, 2024 at 2:24 PM vignesh C <vignes...@gmail.com> wrote: > > On Tue, 19 Mar 2024 at 17:18, Hayato Kuroda (Fujitsu) > <kuroda.hay...@fujitsu.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