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/