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-05 09:08:15 |
Message-ID: | CA+mfrmxBgGq8Co+LK=-uNA3H7aHp44_9QM5-5_spygCGs_ZwiA@mail.gmail.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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(dot)jones(at)uni-muenster(dot)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
>
Attachment | Content-Type | Size |
---|---|---|
v4-allow-SET-to-an-empty-list.patch | text/x-patch | 19.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | vignesh C | 2025-09-05 09:16:20 | Re: Logical Replication of sequences |
Previous Message | Chao Li | 2025-09-05 08:56:22 | Re: Raw parse tree is not dumped to log |