Re: pg_createsubscriber does not check output_plugin_libraries

On Fri, Sep 4, 2026 at 12:49 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> 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