RE: [PATCH] Add a check_hook for output_plugin_libraries - Mailing list pgsql-hackers

From Hayato Kuroda (Fujitsu)
Subject RE: [PATCH] Add a check_hook for output_plugin_libraries
Date
Msg-id OS7PR01MB1831729BFD27DCA5A327B0ADAF5802@OS7PR01MB18317.jpnprd01.prod.outlook.com
Whole thread
In response to Re: [PATCH] Add a check_hook for output_plugin_libraries  (Jacob Champion <jacob.champion@enterprisedb.com>)
List pgsql-hackers
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


Attachment

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: add list of major features to the v19 release notes
Next
From: Manu
Date:
Subject: Re: Recovery at replica stuck because recovery incorrectly trusts an old high-water mark