Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow "SET list_guc TO NULL" to specify setting the GUC to empty.

  1. [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andrei Klychkov <andrew.a.klychkov@gmail.com> — 2025-08-28T09:29:03Z

    Hi Hackers,

I'm submitting a patch to fix a bug where ALTER SYSTEM SET with empty
strings for
GUC_LIST_QUOTE parameters (like shared_preload_libraries) results in
malformed
configuration entries that cause server crashes on restart.

Please take a look,

Thanks
Andrew

  2. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Jim Jones <jim.jones@uni-muenster.de> — 2025-09-02T12:16:08Z

    Hi Andrew

On 28.08.25 11:29, Andrei Klychkov wrote:
> I'm submitting a patch to fix a bug where ALTER SYSTEM SET with empty
> strings for
> GUC_LIST_QUOTE parameters (like shared_preload_libraries) results in
> malformed
> configuration entries that cause server crashes on restart.

I tested the patch and it does what you described

$ psql postgres -c "ALTER SYSTEM SET shared_preload_libraries TO '';"
ALTER SYSTEM
$ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
shared_preload_libraries = ''

However, it breaks one of the rules.sql regression tests

@@ -3552,21 +3552,7 @@
     SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '',
'0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
     IMMUTABLE STRICT;
 SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
-                                                                           
pg_get_functiondef                                                                          
 
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
- CREATE OR REPLACE FUNCTION
public.func_with_set_params()                                                                                                               
+
-  RETURNS
integer                                                                                                                                                       
+
-  LANGUAGE
sql                                                                                                                                                          
+
-  IMMUTABLE
STRICT                                                                                                                                                      
+
-  SET search_path TO
'pg_catalog'                                                                                                                                       
+
-  SET extra_float_digits TO
'2'                                                                                                                                         
+
-  SET work_mem TO
'4MB'                                                                                                                                                 
+
-  SET "DateStyle" TO 'iso,
mdy'                                                                                                                                         
+
-  SET local_preload_libraries TO 'Mixed/Case', 'c:/''a"/path', '',
'0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'+
- AS $function$select
1;$function$                                                                                                                                       
+
-
-(1 row)
-
+ERROR:  invalid list syntax in proconfig item

Best, Jim

  3. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andrei Klychkov <andrew.a.klychkov@gmail.com> — 2025-09-03T09:59:26Z

    Hi Jim,

Thanks a lot for reviewing! Nice catch, TIL!
Version 2 of the patch is attached, please check it out.
In a nutshell, the issue actually wasn't in the flatten_set_variable_args()
function as initially suspected, but rather in the configuration file
writing logic in the write_auto_conf_file(): more details in v2_README.md

Looking forward to your feedback, thanks!
Regards
Andrew

On Tue, Sep 2, 2025 at 2:16 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

> Hi Andrew
>
> On 28.08.25 11:29, Andrei Klychkov wrote:
> > I'm submitting a patch to fix a bug where ALTER SYSTEM SET with empty
> > strings for
> > GUC_LIST_QUOTE parameters (like shared_preload_libraries) results in
> > malformed
> > configuration entries that cause server crashes on restart.
>
> I tested the patch and it does what you described
>
> $ psql postgres -c "ALTER SYSTEM SET shared_preload_libraries TO '';"
> ALTER SYSTEM
> $ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
> # Do not edit this file manually!
> # It will be overwritten by the ALTER SYSTEM command.
> shared_preload_libraries = ''
>
> However, it breaks one of the rules.sql regression tests
>
> @@ -3552,21 +3552,7 @@
>      SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '',
>
> '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
>      IMMUTABLE STRICT;
>  SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
>
> -
>
> pg_get_functiondef
>
>
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------
> - CREATE OR REPLACE FUNCTION
>
> public.func_with_set_params()
> +
> -  RETURNS
>
> integer
> +
> -  LANGUAGE
>
> sql
> +
> -  IMMUTABLE
>
> STRICT
> +
> -  SET search_path TO
>
> 'pg_catalog'
> +
> -  SET extra_float_digits TO
>
> '2'
> +
> -  SET work_mem TO
>
> '4MB'
> +
> -  SET "DateStyle" TO 'iso,
>
> mdy'
> +
> -  SET local_preload_libraries TO 'Mixed/Case', 'c:/''a"/path', '',
>
> '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'+
> - AS $function$select
>
> 1;$function$
> +
> -
> -(1 row)
> -
> +ERROR:  invalid list syntax in proconfig item
>
> Best, Jim
>

  4. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-03T14:47:58Z

    On Wed, Sep 3, 2025 at 6:59 PM Andrei Klychkov
<andrew.a.klychkov@gmail.com> wrote:
>
> Hi Jim,
>
> Thanks a lot for reviewing! Nice catch, TIL!
> Version 2 of the patch is attached, please check it out.
> In a nutshell, the issue actually wasn't in the flatten_set_variable_args() function as initially suspected, but rather in the configuration file writing logic in the write_auto_conf_file(): more details in v2_README.md

Even with this patch, an empty string set via SET is still quoted. For example:

    =# SET local_preload_libraries TO '';
    SET
    =# SHOW local_preload_libraries ;
     local_preload_libraries
    -------------------------
     ""
    (1 row)

Is this behavior acceptable? I was thinking that an empty string should not
be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
Thought?

If we agree it should be handled in both cases, flatten_set_variable_args()
seems to need to be updated. For example:

@@ -289,7 +289,8 @@ flatten_set_variable_args(const char *name, List *args)
                                         * Plain string literal or
identifier.  For quote mode,
                                         * quote it if it's not a
vanilla identifier.
                                         */
-                                       if (flags & GUC_LIST_QUOTE)
+                                       if (flags & GUC_LIST_QUOTE &&
+                                               !(val[0] == '\0' &&
list_length(args) == 1))

appendStringInfoString(&buf, quote_identifier(val));
                                        else

appendStringInfoString(&buf, val);

Regards,

-- 
Fujii Masao

  5. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andrei Klychkov <andrew.a.klychkov@gmail.com> — 2025-09-04T07:41:50Z

    > Even with this patch, an empty string set via SET is still quoted. For
example:
>
>    =# SET local_preload_libraries TO '';
>    SET
>    =# SHOW local_preload_libraries ;
>     local_preload_libraries
>    -------------------------
>     ""
>    (1 row)
>
> Is this behavior acceptable? I was thinking that an empty string should
not
> be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
> Thought?
>
> If we agree it should be handled in both cases,
flatten_set_variable_args()
> seems to need to be updated.

Hello Fujii,
Thanks for your review!

I'm personally not sure because this is my first patch and I'm trying to
solve a specific issue of the postgresql.auto.conf-related server crashes.
If what your *broader-impact* suggestion makes sense to more experienced
devs in this area, I'd be happy to update the patch as you put it, test it
(as much as I can), and re-submit v3.
Otherwise, I'd be happy with the v2 implementation that seemingly solves my
specific issue.

Thanks
Regards
Andrew

On Wed, Sep 3, 2025 at 4:48 PM Fujii Masao <masao.fujii@gmail.com> wrote:

> On Wed, Sep 3, 2025 at 6:59 PM Andrei Klychkov
> <andrew.a.klychkov@gmail.com> wrote:
> >
> > Hi Jim,
> >
> > Thanks a lot for reviewing! Nice catch, TIL!
> > Version 2 of the patch is attached, please check it out.
> > In a nutshell, the issue actually wasn't in the
> flatten_set_variable_args() function as initially suspected, but rather in
> the configuration file writing logic in the write_auto_conf_file(): more
> details in v2_README.md
>
> Even with this patch, an empty string set via SET is still quoted. For
> example:
>
>     =# SET local_preload_libraries TO '';
>     SET
>     =# SHOW local_preload_libraries ;
>      local_preload_libraries
>     -------------------------
>      ""
>     (1 row)
>
> Is this behavior acceptable? I was thinking that an empty string should not
> be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
> Thought?
>
> If we agree it should be handled in both cases, flatten_set_variable_args()
> seems to need to be updated. For example:
>
> @@ -289,7 +289,8 @@ flatten_set_variable_args(const char *name, List *args)
>                                          * Plain string literal or
> identifier.  For quote mode,
>                                          * quote it if it's not a
> vanilla identifier.
>                                          */
> -                                       if (flags & GUC_LIST_QUOTE)
> +                                       if (flags & GUC_LIST_QUOTE &&
> +                                               !(val[0] == '\0' &&
> list_length(args) == 1))
>
> appendStringInfoString(&buf, quote_identifier(val));
>                                         else
>
> appendStringInfoString(&buf, val);
>
> Regards,
>
> --
> Fujii Masao
>

  6. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Jim Jones <jim.jones@uni-muenster.de> — 2025-09-04T14:58:01Z

    
On 04.09.25 09:41, Andrei Klychkov wrote:
> Even with this patch, an empty string set via SET is still quoted. For
> example:
>
>     =# SET local_preload_libraries TO '';
>     SET
>     =# SHOW local_preload_libraries ;
>      local_preload_libraries
>     -------------------------
>      ""
>     (1 row)
>
> Is this behavior acceptable? I was thinking that an empty string
> should not
> be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
> Thought?
>
> If we agree it should be handled in both cases,
> flatten_set_variable_args()
> seems to need to be updated. For example:
>
> @@ -289,7 +289,8 @@ flatten_set_variable_args(const char *name, List
> *args)
>                                          * Plain string literal or
> identifier.  For quote mode,
>                                          * quote it if it's not a
> vanilla identifier.
>                                          */
> -                                       if (flags & GUC_LIST_QUOTE)
> +                                       if (flags & GUC_LIST_QUOTE &&
> +                                               !(val[0] == '\0' &&
> list_length(args) == 1))
>
> appendStringInfoString(&buf, quote_identifier(val));
>                                         else
>
> appendStringInfoString(&buf, val);

Yeah, I also think that SET and ALTER SYSTEM SET should be consistent. I
tested your proposed changes in flatten_set_variable_args ..

/*
 * Plain string literal or identifier.  For quote mode,
 * quote it if it's not a vanilla identifier. However, if the value
 * is an empty string (val[0] == '\0') and it is the only element
 * in the list (list_length(args) == 1), display it as an empty string
 * without quotes for clarity and consistency.
 */
if (flags & GUC_LIST_QUOTE &&
    !(val[0] == '\0' && list_length(args) == 1))
    appendStringInfoString(&buf, quote_identifier(val));
else
    appendStringInfoString(&buf, val);

... and it seems to work:

$ psql postgres -c "SET local_preload_libraries TO ''; SHOW
local_preload_libraries;"
SET
 local_preload_libraries
-------------------------
 
(1 row)

$ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO '';"
ALTER SYSTEM

(restart ..)

$ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
local_preload_libraries = ''

$ psql postgres -c "SHOW local_preload_libraries;"
 local_preload_libraries
-------------------------
 
(1 row)


I'm wondering if we should add some tests, e.g. in guc.sql:

SET local_preload_libraries TO '';
SHOW local_preload_libraries;

and also it's equivalents for ALTER SYSTEM SET (still not sure where :)).

Best regards, Jim

  7. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-04T16:42:06Z

    On Thu, Sep 4, 2025 at 4:42 PM Andrei Klychkov
<andrew.a.klychkov@gmail.com> wrote:
>
> > Even with this patch, an empty string set via SET is still quoted. For example:
> >
> >    =# SET local_preload_libraries TO '';
> >    SET
> >    =# SHOW local_preload_libraries ;
> >     local_preload_libraries
> >    -------------------------
> >     ""
> >    (1 row)
> >
> > Is this behavior acceptable? I was thinking that an empty string should not
> > be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
> > Thought?
> >
> > If we agree it should be handled in both cases, flatten_set_variable_args()
> > seems to need to be updated.
>
> Hello Fujii,
> Thanks for your review!
>
> I'm personally not sure because this is my first patch and I'm trying to solve a specific issue of the postgresql.auto.conf-related server crashes.
> If what your *broader-impact* suggestion makes sense to more experienced devs in this area, I'd be happy to update the patch as you put it, test it (as much as I can), and re-submit v3.
> Otherwise, I'd be happy with the v2 implementation that seemingly solves my specific issue.

Yeah, I think my suggestion makes sense.

BTW, regarding the behavior change, I believe that users likely expect
the parameter to be reset when specifying an empty string, rather than
being set to "". So the proposed change seems reasonable. However,
the current behavior has existed for a long time, and I haven’t seen
any complaints about it. Some users may rely on the existing behavior
(I think that’s unlikely, though). So I'm not completely sure yet if this change
should be applied.

Regards,

-- 
Fujii Masao

  8. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-04T16:44:37Z

    On Thu, Sep 4, 2025 at 11:58 PM Jim Jones <jim.jones@uni-muenster.de> wrote:
> Yeah, I also think that SET and ALTER SYSTEM SET should be consistent. I
> tested your proposed changes in flatten_set_variable_args ..

Thanks for the test!


> I'm wondering if we should add some tests, e.g. in guc.sql:

+1 to add regression tests.

Regards,

-- 
Fujii Masao

  9. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-04T17:40:39Z

    Fujii Masao <masao.fujii@gmail.com> writes:
>>> Even with this patch, an empty string set via SET is still quoted. For example:
>>> 
>>> =# SET local_preload_libraries TO '';
>>> SET
>>> =# SHOW local_preload_libraries ;
>>> local_preload_libraries
>>> -------------------------
>>> ""
>>> (1 row)
>>> 
>>> Is this behavior acceptable? I was thinking that an empty string should not
>>> be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.

> BTW, regarding the behavior change, I believe that users likely expect
> the parameter to be reset when specifying an empty string, rather than
> being set to "". So the proposed change seems reasonable. However,
> the current behavior has existed for a long time, and I haven’t seen
> any complaints about it.

I think this is largely based on confusion.  In the above example,
local_preload_libraries is being set to a list containing a single
entry that is an empty string, and the output of SHOW is a fully
accurate depiction of that state.  It is *not* being set to an
empty list --- we actually don't have any syntax that would permit
doing so in SET.  For comparison, there is a big difference between

	SET local_preload_libraries = a, b;
	SET local_preload_libraries = 'a, b';

In the latter case you get a single list entry containing the
string "a, b".  We do not try to parse that into multiple entries,
and by the same token parsing an empty string into an empty list
would be the Wrong Thing.

We might want to start resolving this by inventing a syntax for
setting a list GUC to an empty list.  I'm not very sure what that
should look like, except that it mustn't be SET ... TO ''.

I'm not certain whether config-file parsing or ALTER SYSTEM
would need any code changes once we resolve the ambiguity in SET.
The config-file syntax is different and doesn't have this problem
of not being able to represent an empty list.

(Also, "let's unify the list-GUC syntax between config file and SET"
seems like a non-starter.  It'd be better no doubt if they hadn't
diverged, but at this point we'd break far more than we fix if
we change either one.)

			regards, tom lane

  10. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-04T18:24:48Z

    I wrote:
> We might want to start resolving this by inventing a syntax for
> setting a list GUC to an empty list.  I'm not very sure what that
> should look like, except that it mustn't be SET ... TO ''.

I experimented with the idea of allowing

	SET var TO ;

and got a bunch of shift-reduce conflicts from bison, which don't
look easy to avoid.  It's probably a bit too surprising anyway.

This works from a grammar standpoint:

	SET var TO NULL;

Because NULL is fully reserved, this isn't usurping any cases that
weren't syntax errors before.  It might still be too surprising.

Another idea is that we could redefine a single '' as meaning an empty
list if we were to forbid empty strings as members of GUC_LIST_QUOTE
variables.  This doesn't look like it'd be a big issue for the current
set of such variables:

local_preload_libraries
search_path
session_preload_libraries
shared_preload_libraries
temp_tablespaces
unix_socket_directories

We might break some applications that're relying on the current
behavior that an empty item would be effectively ignored.
But that seems a bit remote, and anyway they could just switch
to some other nonexistent name.

Actually, looking at the small number of GUCs that are marked
GUC_LIST_INPUT but not GUC_LIST_QUOTE, I wonder if we shouldn't
prohibit empty strings across-the-board for GUC_LIST_INPUT GUCs.
I don't see any where it'd be useful to allow them.  Then we
could allow '' to mean empty list for all of them.

			regards, tom lane

  11. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-04T21:52:49Z

    I wrote:
> Another idea is that we could redefine a single '' as meaning an empty
> list if we were to forbid empty strings as members of GUC_LIST_QUOTE
> variables.

I tried to work through what this'd imply, and arrived at the attached
patch.  I might've missed some places, and I did not think about what
documentation updates would be appropriate.

Note that the patch includes changing SplitIdentifierString and its
clones to forbid zero-length quoted elements, which were formerly
allowed.  Without this, we'd accept values from config files that
could not be represented in SET, which is exactly the situation we
are trying to fix.

I'm not entirely sure if this is the way to go, or if we want to
adopt some other solution that doesn't involve forbidding empty
list elements.  I suspect that anything else we come up with would
be less intuitive than letting SET list_var = '' do the job;
but maybe I just lack imagination today.

			regards, tom lane

  12. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Jim Jones <jim.jones@uni-muenster.de> — 2025-09-05T07:33:39Z

    
On 04.09.25 23:52, Tom Lane wrote:
> Note that the patch includes changing SplitIdentifierString and its
> clones to forbid zero-length quoted elements, which were formerly
> allowed.  Without this, we'd accept values from config files that
> could not be represented in SET, which is exactly the situation we
> are trying to fix.
>
> I'm not entirely sure if this is the way to go, or if we want to
> adopt some other solution that doesn't involve forbidding empty
> list elements.  I suspect that anything else we come up with would
> be less intuitive than letting SET list_var = '' do the job;
> but maybe I just lack imagination today.

This approach LGTM. It solves the initial issue:

$ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO '';"
ALTER SYSTEM

$ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
local_preload_libraries = ''

... making a clear distinction between empty elements and empty lists:

postgres=# SET local_preload_libraries TO '','foo';
ERROR:  SET local_preload_libraries does not accept empty-string elements

postgres=# SET local_preload_libraries TO '';
SET
postgres=# SHOW local_preload_libraries;
 local_preload_libraries
-------------------------
 
(1 row)

The ambiguity between an empty list and an empty element has always
existed in list-valued GUCs. This patch resolves the issue by
disallowing empty elements, thereby making '' an unambiguous
representation of an empty list. Personally, I find SET var TO NULL (or
perhaps a keyword like EMPTY or NONE) a more palatable syntax for
expressing empty lists in this case. However, I’m not sure the
additional complexity and compatibility implications would justify such
a change.

Best regards, Jim

  13. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andrei Klychkov <andrew.a.klychkov@gmail.com> — 2025-09-05T09:08:15Z

    As it solves the initial issue, it SGTM too.
I applied v3, updated the docs and added some tests in attached v4.
Hopefully it's OK.
Please take a look

Thanks
Regards

On Fri, Sep 5, 2025 at 9:33 AM Jim Jones <jim.jones@uni-muenster.de> wrote:

>
>
> On 04.09.25 23:52, Tom Lane wrote:
> > Note that the patch includes changing SplitIdentifierString and its
> > clones to forbid zero-length quoted elements, which were formerly
> > allowed.  Without this, we'd accept values from config files that
> > could not be represented in SET, which is exactly the situation we
> > are trying to fix.
> >
> > I'm not entirely sure if this is the way to go, or if we want to
> > adopt some other solution that doesn't involve forbidding empty
> > list elements.  I suspect that anything else we come up with would
> > be less intuitive than letting SET list_var = '' do the job;
> > but maybe I just lack imagination today.
>
> This approach LGTM. It solves the initial issue:
>
> $ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO '';"
> ALTER SYSTEM
>
> $ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
> # Do not edit this file manually!
> # It will be overwritten by the ALTER SYSTEM command.
> local_preload_libraries = ''
>
> ... making a clear distinction between empty elements and empty lists:
>
> postgres=# SET local_preload_libraries TO '','foo';
> ERROR:  SET local_preload_libraries does not accept empty-string elements
>
> postgres=# SET local_preload_libraries TO '';
> SET
> postgres=# SHOW local_preload_libraries;
>  local_preload_libraries
> -------------------------
>
> (1 row)
>
> The ambiguity between an empty list and an empty element has always
> existed in list-valued GUCs. This patch resolves the issue by
> disallowing empty elements, thereby making '' an unambiguous
> representation of an empty list. Personally, I find SET var TO NULL (or
> perhaps a keyword like EMPTY or NONE) a more palatable syntax for
> expressing empty lists in this case. However, I’m not sure the
> additional complexity and compatibility implications would justify such
> a change.
>
> Best regards, Jim
>

  14. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-09-05T21:06:17Z

    Jim Jones <jim.jones@uni-muenster.de> writes:
> On 04.09.25 23:52, Tom Lane wrote:
>> I'm not entirely sure if this is the way to go, or if we want to
>> adopt some other solution that doesn't involve forbidding empty
>> list elements.  I suspect that anything else we come up with would
>> be less intuitive than letting SET list_var = '' do the job;
>> but maybe I just lack imagination today.

> The ambiguity between an empty list and an empty element has always
> existed in list-valued GUCs. This patch resolves the issue by
> disallowing empty elements, thereby making '' an unambiguous
> representation of an empty list. Personally, I find SET var TO NULL (or
> perhaps a keyword like EMPTY or NONE) a more palatable syntax for
> expressing empty lists in this case. However, I’m not sure the
> additional complexity and compatibility implications would justify such
> a change.

Since you expressed interest, I made a draft patch that does it like
that.  Unsurprisingly, it has to touch mostly the same places that
the v3 patch did, plus the grammar.  Still ends up a bit shorter
though.

I remain unsure which way I like better.  The NULL approach has the
advantage of not foreclosing use of empty-string list elements, which
we might want someday even if there's no obvious value today.  (And
for the same reason, it's less of a behavioral change.)  But it still
feels a bit less intuitive to me.  It might flow better with some
other keyword --- but we have to use a fully-reserved keyword, and we
are surely not going to make a new one of those just for this purpose,
and NULL is the only existing one that's even slightly on-point.

			regards, tom lane

  15. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Jim Jones <jim.jones@uni-muenster.de> — 2025-09-06T14:44:30Z

    Hi Tom

On 05.09.25 23:06, Tom Lane wrote:
> I remain unsure which way I like better.  The NULL approach has the
> advantage of not foreclosing use of empty-string list elements, which
> we might want someday even if there's no obvious value today.  (And
> for the same reason, it's less of a behavioral change.)  But it still
> feels a bit less intuitive to me.  

I think this is a nice addition. The way I see it is: it provides an
unambiguous way to "clear" the variable, which, as you pointed out,
might carry different semantics in the future than an empty string. More
generally, I understand that using NULL (unknown/undefined) to represent
an empty list could be seen as a semantic stretch, but in this case it
doesn’t feel unintuitive to me. Although I prefer this new syntax, I can
definitely live without it :)

Here some tests:

== ALTER SYSTEM SET var TO ''

$ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO '';"
ALTER SYSTEM

$ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
local_preload_libraries = '""'

$ pg_ctl -D /usr/local/postgres-dev/testdb -l
/usr/local/postgres-dev/logfile restart
waiting for server to shut down.... done
server stopped
waiting for server to start.... done
server started

$ psql postgres
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
FATAL:  could not access file "$libdir/plugins/": No such file or directory

The error itself is expected, but the message does not make it
immediately clear that the problem comes from a misconfigured GUC. But
this seems to be more related to the specific variable than to the scope
of this patch.

== ALTER SYSTEM SET var TO NULL
Using the new syntax it works just fine:

$ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO NULL;"
ALTER SYSTEM

$ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
# Do not edit this file manually!
# It will be overwritten by the ALTER SYSTEM command.
local_preload_libraries = ''

$ pg_ctl -D /usr/local/postgres-dev/testdb -l
/usr/local/postgres-dev/logfile restart
waiting for server to shut down.... done
server stopped
waiting for server to start.... done
server started

$ psql postgres -c "SHOW local_preload_libraries;"
 local_preload_libraries
-------------------------
 
(1 row)

== SET var TO ''

$ psql postgres -c "SET local_preload_libraries TO ''; SHOW
local_preload_libraries;"
SET
 local_preload_libraries
-------------------------
 ""
(1 row)

== SET var TO NULL

$ psql postgres -c "SET local_preload_libraries TO NULL; SHOW
local_preload_libraries;"
SET
 local_preload_libraries
-------------------------
 
(1 row)

== SET var TO list containing empty element

$ psql postgres -c "SET local_preload_libraries TO 'foo',''; SHOW
local_preload_libraries;"
SET
 local_preload_libraries
-------------------------
 foo, ""
(1 row)

== SET var TO list containing NULL element

$ psql postgres -c "SET local_preload_libraries TO NULL,''; SHOW
local_preload_libraries;"
ERROR:  syntax error at or near ","
LINE 1: SET local_preload_libraries TO NULL,''; SHOW local_preload_l...

== SET var TO list containing multiple empty elements

$ /usr/local/postgres-dev/bin/psql postgres -c "SET
local_preload_libraries TO '',''; SHOW local_preload_libraries;"
SET
 local_preload_libraries
-------------------------
 "", ""
(1 row)


Best regards, Jim

  16. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andrei Klychkov <andrew.a.klychkov@gmail.com> — 2025-09-08T08:08:34Z

    Hello,

On 05.09.25 23:06, Tom Lane wrote:
> I remain unsure which way I like better.  The NULL approach has the
> advantage of not foreclosing use of empty-string list elements, which
> we might want someday even if there's no obvious value today.  (And
> for the same reason, it's less of a behavioral change.)  But it still
> feels a bit less intuitive to me.

Looks like the patch v5 resolves a long-standing limitation where there was
no SQL syntax to set a list-based GUC to an empty list. I like this
approach. Also the changes seem non-breaking. Thanks

1. Would be great to have some explanations in docs about this new behavior.

2. It doesn't look to me that v5 solves the original issue of a user
running ALTER SYSTEM SET <setting like shared_preload_libraries> = ''; ,
then restarting the server and not getting it back online.

Regards
Andrew


On Sat, Sep 6, 2025 at 4:44 PM Jim Jones <jim.jones@uni-muenster.de> wrote:

> Hi Tom
>
> On 05.09.25 23:06, Tom Lane wrote:
> > I remain unsure which way I like better.  The NULL approach has the
> > advantage of not foreclosing use of empty-string list elements, which
> > we might want someday even if there's no obvious value today.  (And
> > for the same reason, it's less of a behavioral change.)  But it still
> > feels a bit less intuitive to me.
>
> I think this is a nice addition. The way I see it is: it provides an
> unambiguous way to "clear" the variable, which, as you pointed out,
> might carry different semantics in the future than an empty string. More
> generally, I understand that using NULL (unknown/undefined) to represent
> an empty list could be seen as a semantic stretch, but in this case it
> doesn’t feel unintuitive to me. Although I prefer this new syntax, I can
> definitely live without it :)
>
> Here some tests:
>
> == ALTER SYSTEM SET var TO ''
>
> $ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO '';"
> ALTER SYSTEM
>
> $ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
> # Do not edit this file manually!
> # It will be overwritten by the ALTER SYSTEM command.
> local_preload_libraries = '""'
>
> $ pg_ctl -D /usr/local/postgres-dev/testdb -l
> /usr/local/postgres-dev/logfile restart
> waiting for server to shut down.... done
> server stopped
> waiting for server to start.... done
> server started
>
> $ psql postgres
> psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed:
> FATAL:  could not access file "$libdir/plugins/": No such file or directory
>
> The error itself is expected, but the message does not make it
> immediately clear that the problem comes from a misconfigured GUC. But
> this seems to be more related to the specific variable than to the scope
> of this patch.
>
> == ALTER SYSTEM SET var TO NULL
> Using the new syntax it works just fine:
>
> $ psql postgres -c "ALTER SYSTEM SET local_preload_libraries TO NULL;"
> ALTER SYSTEM
>
> $ cat /usr/local/postgres-dev/testdb/postgresql.auto.conf
> # Do not edit this file manually!
> # It will be overwritten by the ALTER SYSTEM command.
> local_preload_libraries = ''
>
> $ pg_ctl -D /usr/local/postgres-dev/testdb -l
> /usr/local/postgres-dev/logfile restart
> waiting for server to shut down.... done
> server stopped
> waiting for server to start.... done
> server started
>
> $ psql postgres -c "SHOW local_preload_libraries;"
>  local_preload_libraries
> -------------------------
>
> (1 row)
>
> == SET var TO ''
>
> $ psql postgres -c "SET local_preload_libraries TO ''; SHOW
> local_preload_libraries;"
> SET
>  local_preload_libraries
> -------------------------
>  ""
> (1 row)
>
> == SET var TO NULL
>
> $ psql postgres -c "SET local_preload_libraries TO NULL; SHOW
> local_preload_libraries;"
> SET
>  local_preload_libraries
> -------------------------
>
> (1 row)
>
> == SET var TO list containing empty element
>
> $ psql postgres -c "SET local_preload_libraries TO 'foo',''; SHOW
> local_preload_libraries;"
> SET
>  local_preload_libraries
> -------------------------
>  foo, ""
> (1 row)
>
> == SET var TO list containing NULL element
>
> $ psql postgres -c "SET local_preload_libraries TO NULL,''; SHOW
> local_preload_libraries;"
> ERROR:  syntax error at or near ","
> LINE 1: SET local_preload_libraries TO NULL,''; SHOW local_preload_l...
>
> == SET var TO list containing multiple empty elements
>
> $ /usr/local/postgres-dev/bin/psql postgres -c "SET
> local_preload_libraries TO '',''; SHOW local_preload_libraries;"
> SET
>  local_preload_libraries
> -------------------------
>  "", ""
> (1 row)
>
>
> Best regards, Jim
>

  17. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-01T20:11:46Z

    Andrei Klychkov <andrew.a.klychkov@gmail.com> writes:
> Looks like the patch v5 resolves a long-standing limitation where there was
> no SQL syntax to set a list-based GUC to an empty list. I like this
> approach. Also the changes seem non-breaking. Thanks

> 1. Would be great to have some explanations in docs about this new behavior.

Yeah, I hadn't bothered with docs, pending decisions about which way
we were going to implement this.  But it seems like we're leaning
towards using the NULL syntax, so I went ahead and did some docs work.

> 2. It doesn't look to me that v5 solves the original issue of a user
> running ALTER SYSTEM SET <setting like shared_preload_libraries> = ''; ,
> then restarting the server and not getting it back online.

[ shrug... ]  It's not supposed to "solve" that.  That command is
erroneous, and if you didn't test the setting before restarting the
server, you shouldn't be too surprised if restart fails.  What this
patch is meant to do is provide a valid way to accomplish what you
wanted, namely

	ALTER SYSTEM SET shared_preload_libraries = NULL;

Anyway, the attached v6 is the same as v5, except now with proposed
doc changes and a draft commit message.  I spent some effort on
getting the ALTER SYSTEM and SET ref pages back into sync; it seemed
like more care has been taken with the ALTER SYSTEM synopsis and
other details.

I'm not sure if we want to change anything about this in config.sgml.
There are enough GUC_LIST_INPUT GUCs that I can't see mentioning it
for each one.

			regards, tom lane

  18. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-04T16:58:00Z

    I wrote:
> Andrei Klychkov <andrew.a.klychkov@gmail.com> writes:
>> 2. It doesn't look to me that v5 solves the original issue of a user
>> running ALTER SYSTEM SET <setting like shared_preload_libraries> = ''; ,
>> then restarting the server and not getting it back online.

> [ shrug... ]  It's not supposed to "solve" that.  That command is
> erroneous, and if you didn't test the setting before restarting the
> server, you shouldn't be too surprised if restart fails.

If you are feeling excited about that specific case, I think the
correct solution would be to install a GUC check_hook for
shared_preload_libraries (and probably its siblings too).  It couldn't
go so far as to actually dlopen() the list items, but it could check
that each one resolves as an accessible file.

A potential objection is that this could result in unwanted failures
in some scenarios, say where you're restoring a dump and haven't
yet installed all the relevant extensions.  I'm not quite sure if
there are realistic scenarios where that's actually a problem.
If it is, perhaps we could adjust the check_hook so it issues
WARNINGs not hard errors.

I'm not sure it's worth the trouble though.  A quick look at dfmgr.c
suggests that it'd take quite a lot of refactoring (or else code
duplication, not good) to be able to apply the library lookup process
without actually doing dlopen.  In any case that would be a totally
different patch from what we are discussing here.

> Anyway, the attached v6 is the same as v5, except now with proposed
> doc changes and a draft commit message.  I spent some effort on
> getting the ALTER SYSTEM and SET ref pages back into sync; it seemed
> like more care has been taken with the ALTER SYSTEM synopsis and
> other details.

Hearing no further comments, I'm going to go ahead with v6.

			regards, tom lane

  19. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Andreas Karlsson <andreas@proxel.se> — 2025-11-04T17:12:43Z

    On 11/4/25 5:58 PM, Tom Lane wrote:
> Hearing no further comments, I'm going to go ahead with v6.

Honestly none of the alternatives is very appealing and v6 is probably 
the least bad.

When I ran into this issue I thought about adding and EMPTY keyword (we 
do not want more keywords) or adding support for specifying array 
literals like this [] or [a, b], but that would be confusing since 
postgresql.conf does not allow any such syntax.

Andreas

  20. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-04T17:24:41Z

    Andreas Karlsson <andreas@proxel.se> writes:
> Honestly none of the alternatives is very appealing and v6 is probably 
> the least bad.

Yeah, that's about where I'm at with this.

> When I ran into this issue I thought about adding and EMPTY keyword (we 
> do not want more keywords) or adding support for specifying array 
> literals like this [] or [a, b], but that would be confusing since 
> postgresql.conf does not allow any such syntax.

I would have preferred a different keyword.  But AFAICS it'd have to
be a fully-reserved word, and creating a new one of those has to
clear a pretty high bar.  NULL is not so far off...

			regards, tom lane

  21. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Maciek Sakrejda <maciek@pganalyze.com> — 2025-11-04T18:43:29Z

    On Tue, Nov 4, 2025 at 8:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > Andrei Klychkov <andrew.a.klychkov@gmail.com> writes:
> >> 2. It doesn't look to me that v5 solves the original issue of a user
> >> running ALTER SYSTEM SET <setting like shared_preload_libraries> = ''; ,
> >> then restarting the server and not getting it back online.
>
> > [ shrug... ]  It's not supposed to "solve" that.  That command is
> > erroneous, and if you didn't test the setting before restarting the
> > server, you shouldn't be too surprised if restart fails.
>
> If you are feeling excited about that specific case, I think the
> correct solution would be to install a GUC check_hook for
> shared_preload_libraries (and probably its siblings too).  It couldn't
> go so far as to actually dlopen() the list items, but it could check
> that each one resolves as an accessible file.
>
> A potential objection is that this could result in unwanted failures
> in some scenarios, say where you're restoring a dump and haven't
> yet installed all the relevant extensions.  I'm not quite sure if
> there are realistic scenarios where that's actually a problem.
> If it is, perhaps we could adjust the check_hook so it issues
> WARNINGs not hard errors.
>
> I'm not sure it's worth the trouble though.  A quick look at dfmgr.c
> suggests that it'd take quite a lot of refactoring (or else code
> duplication, not good) to be able to apply the library lookup process
> without actually doing dlopen.  In any case that would be a totally
> different patch from what we are discussing here.

For what it's worth, there was a patch that took a stab at this a
while ago, but it ended up RWF:
https://www.postgresql.org/message-id/flat/Z1kfMUoZkl9P0egB%40paquier.xyz#4a73293daf92aefbbdb43adc9688f082

I was a reviewer and I still think something like that would be useful
and prevent a lot of mistakes.

Thanks,
Maciek

  22. Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-11-04T18:59:55Z

    Maciek Sakrejda <maciek@pganalyze.com> writes:
> On Tue, Nov 4, 2025 at 8:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> If you are feeling excited about that specific case, I think the
>> correct solution would be to install a GUC check_hook for
>> shared_preload_libraries (and probably its siblings too).

> For what it's worth, there was a patch that took a stab at this a
> while ago, but it ended up RWF:
> https://www.postgresql.org/message-id/flat/Z1kfMUoZkl9P0egB%40paquier.xyz#4a73293daf92aefbbdb43adc9688f082
> I was a reviewer and I still think something like that would be useful
> and prevent a lot of mistakes.

Hah, yeah, I thought that idea seemed familiar.

Re-reading that thread, it seems like a whole lot of the difficulties
arose precisely from not wanting to make the check_hook's complaints
be hard errors.  Maybe we should abandon the idea that we need to
permit setting the GUC to a value that we know will not work.  It was
argued that there were use-cases for that, but the argument seems
rather thin and not worth tying the behavior in knots for.

Another idea, considering our experience with search_path and
temp_tablespaces, is maybe it shouldn't be a hard error if the
GUC contains references to nonexistent libraries, only if the
syntax is bad.  As long as an empty-string item is bad syntax,
the check_hook could still prevent the problem we started this
thread with.

			regards, tom lane