Re: pg_createsubscriber does not check output_plugin_libraries

On Wed, Sep 16, 2026 at 3:55 PM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> Hi hackers,
>
> Here is a v6 patch set. What's new:

Thanks for updating the patches!

+ The source server must also allow the <literal>pgoutput</literal> output
+ plugin by setting <xref linkend="guc-output-plugin-libraries"/> to include
+ <literal>pgoutput</literal>.

Isn't it better to clarify that this requirement applies to the effective
setting for each source database connection? For example:

The effective value of <xref linkend="guc-output-plugin-libraries"/>
for each connection to a source database must include
<literal>pgoutput</literal>.

+ if (!SplitGUCList(output_plugin_libraries_copy, ',', &allowed_plugins))
+ {
+ /* Should not happen */
+ pg_fatal("could not parse \"output_plugin_libraries\" setting '%s'
in database \"%s\"",
+ output_plugin_libraries, dbinfo[i].dbname);

The "Should not happen" comment should be removed because this can happen?
For example, if output_plugin_libraries is set to 'test_decoding, ' on
the server, pg_createsubscriber fails to parse it and reaches this point.

+ qr/publisher does not allow the "pgoutput" output plugin in database
"\Q$db2\E"/,
+ 'primary does not allow to load pgoutput plugin');

"primary" should be "publisher"? For example, how about
"publisher disallows pgoutput output plugin"?

+# output_plugin_libraries is invisible to non-superusers, so its check should
+# be skipped even when pgoutput is not allowed.

This comment seems a bit misleading because non-superusers with
pg_read_all_settings privileges can read this setting. Instead, how about
"This user cannot read output_plugin_libraries, so the check should be
skipped even when pgoutput is not allowed."?

Regarding the backpatch of the 0001 patch, I'm leaning toward treating this
as an improvement for master only.

Regards,

--
Fujii Masao