On Fri, Sep 4, 2026 at 12:49 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
> Possible fix
> =======
> I think pg_createsubscriber can check the parameter in check_publisher().
> Attached 0002 patch does the same.
Thanks for the patch!
I have a few comments.
Regarding the pg_createsubscriber docs, isn't it better to mention
output_plugin_libraries in the Prerequisites section?
* - max_replication_slots >= current + number of dbs to be converted
* - max_wal_senders >= current + number of dbs to be converted
* - max_slot_wal_keep_size = -1 (to prevent deletion of required WAL files)
* -----------------------------------------------------------------------
check_publisher() has the above source comment. Shouldn't
it mention output_plugin_libraries as well?
+ if (!SplitGUCList(output_plugin_libraries, ',', &allowed_plugins))
+ {
+ /*
+ * Should not happen. (Frontend and backend GUC_LIST_QUOTE parsing
+ * have to remain compatible for pg_dump at minimum.)
+ */
+ pg_fatal("could not parse \"output_plugin_libraries\" setting '%s'",
+ output_plugin_libraries);
Since SplitGUCList() may modify its input, output_plugin_libraries
might no longer show the orignal GUC value when it's passed to
pg_fatal(). Isn't it better to follow pg_upgrade/check.c and pass
a separate copy to SplitGUCList(), keeping output_plugin_libraries
unchanged for error reporting?
- # Note that src/bin/pg_upgrade/check.c assumes GUC_LIST_QUOTE here.
+ # Note that src/bin/pg_upgrade/check.c and
src/bin/pg_basebackup/pg_createsubscriber assume GUC_LIST_QUOTE here.
"pg_createsubscriber" should be "pg_createsubscriber.c" here?
> Note
> ====
> BTW, I noticed that `char *max_slot_wal_keep_size` is pg_strdup'd but not
> free'd.
> 0003 fixes that.
LGTM. We can commit this together with the 0002 patch.
Regards,
--
Fujii Masao
