Dear Ian,
Thanks for making the patch.
> With the addition of "pg_sync_replication_slots()", there is now a use-case for
> including "dbname" in "primary_conninfo" and the docs have changed from
> stating [1]:
>
> Do not specify a database name in the primary_conninfo string.
>
> to [2]:
>
> For replication slot synchronization (see Section 48.2.3), it is also
> necessary to specify a valid dbname in the primary_conninfo string. This will
> only be used for slot synchronization. It is ignored for streaming.
>
> [1]
> https://www.postgresql.org/docs/16/runtime-config-replication.html#RUNTIME
> -CONFIG-REPLICATION-STANDBY
> [2]
> https://www.postgresql.org/docs/devel/runtime-config-replication.html#RUNTI
> ME-CONFIG-REPLICATION-STANDBY
>
> However, when setting up a standby (with the intent of creating a logical
> standby) with pg_basebackup, providing the -R/-write-recovery-conf option
> results in a "primary_conninfo" string being generated without a "dbname"
> parameter, which requires a manual change should one intend to use
> "pg_sync_replication_slots()".
>
> I can't see any reason for continuing to omit "dbname", so suggest it should
> only continue to be omitted for 16 and earlier; see attached patch.
Hmm, I also cannot find a reason, but we can document the change.
> Note that this does mean that if the conninfo string provided to pg_basebackup
> does not contain "dbname", the generated "primary_conninfo" GUC will default to
> "dbname=replication". It would be easy enough to suppress this, but AFAICS
> there's no way to tell if it was explicitly supplied by the user, in which case
> it would be surprising if it were omitted from the final "primary_conninfo"
> string.
I found an inconsistency. When I ran ` pg_basebackup -D data_N2 -U postgres -R`,
dbname would be set as username.
```
primary_conninfo = 'user=postgres ... dbname=postgres
```
However, when I ran `pg_basebackup -D data_N2 -d "user=postgres" -R`,
dbname would be set as "replication". Is it an intentional item?
```
primary_conninfo = 'user=postgres ... dbname=replication...
```
> The only other place where GenerateRecoveryConfig() is called is pg_rewind;
> I can't see any adverse affects from the proposed change. But it's perfectly
> possible there's something blindingly obvious I'm overlooking.
On-going feature pg_createsubscriber[1] also uses GenerateRecoveryConfig(), but
I can't see any bad effects. The output is being used to make consistent standby
from the primary. Even if dbname is set in the primary_conninfo, it would be ignored.
[1]: https://commitfest.postgresql.org/47/4637/
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/