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

From: Andrei Klychkov <andrew(dot)a(dot)klychkov(at)gmail(dot)com>
To: Jim Jones <jim(dot)jones(at)uni-muenster(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Fix ALTER SYSTEM empty string bug for GUC_LIST_QUOTE parameters
Date: 2025-09-08 08:08:34
Message-ID: CA+mfrmyc6pEGE9nY70nEJMdtyf2Hn_Qe=dC1jcLm1V55SROrLw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)jones(at)uni-muenster(dot)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
>

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2025-09-08 08:21:30 Re: Reduce "Var IS [NOT] NULL" quals during constant folding
Previous Message Eric Marsden 2025-09-08 08:01:00 doc patch: protocol major.minor numbers in NegotiateProtocolVersion message