Dear Jacob, > Where did your double-quotes come from? If they came from a previous > `SET output_plugin_libraries = 'pgoutput, test_decoding'`, then that > wasn't a correct command; see below. Here's the output on my machine > after a fresh initdb: > > postgres=# show output_plugin_libraries; > output_plugin_libraries > ------------------------- > pgoutput, test_decoding > (1 row)
You're right. When I ran for the just initialized instance, I got the same. ``` postgres=# SHOW output_plugin_libraries ; output_plugin_libraries ------------------------- pgoutput, test_decoding (1 row) ``` > I think that behavior is confusing for people who expect to have to > quote the whole thing (including me). But it's presumably tied to how > we want search_path to behave for everyone, so changing it would > probably be a very big project. Hmm, okay. I confirmed the same behavior for the search_path. At very least fixing all here is not realistic. Below are my comments: 01. ``` + /* + * XXX SplitGUCList won't respect guc_malloc requirements, but this is + * consistent with other check_hook implementations... + */ ``` Let me clarify: it's because the SplitGUCList()->lappend() can raise OOM error, right? Maybe we need to add another function to handle around here, but it can be in PG20... 02. Is the same check in StartupDecodingContext() still needed? I.e. we can check the returned value by Assert() here. 03. ``` +# check.c assumes the list syntax of output_plugin_libraries is validated by the +# server, so take a moment to confirm that now. (This is difficult to test via +# regression suite, because our SET grammar won't accept the bad syntax.) ``` Per my experiment, an SQL function set_config() is usable. So can't we put in guc.sql? PSA my idea. Best regards, Hayato Kuroda FUJITSU LIMITED
kuroda_atopv1.diff
Description: kuroda_atopv1.diff
