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.jo...@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
>
From dd33d106fc67086dde59056d5ad39c61621b4dde Mon Sep 17 00:00:00 2001
From: Andrew Klychkov <andrew.a.klych...@gmail.com>
Date: Fri, 5 Sep 2025 10:59:08 +0200
Subject: [PATCH] Allow SET to an empty list
---
doc/src/sgml/config.sgml | 2 +
doc/src/sgml/ref/set.sgml | 6 +++
src/backend/utils/adt/ruleutils.c | 10 +++--
src/backend/utils/adt/varlena.c | 12 ++++--
src/backend/utils/misc/guc_funcs.c | 43 ++++++++++++++++---
src/bin/pg_dump/dumputils.c | 12 ++++--
src/bin/pg_dump/pg_dump.c | 8 +++-
src/test/modules/test_misc/meson.build | 1 +
.../modules/test_misc/t/009_alter_system.pl | 31 +++++++++++++
src/test/regress/expected/guc.out | 18 ++++++++
src/test/regress/expected/rules.out | 28 ++++++------
src/test/regress/sql/guc.sql | 8 ++++
src/test/regress/sql/rules.sql | 3 +-
13 files changed, 150 insertions(+), 32 deletions(-)
create mode 100644 src/test/modules/test_misc/t/009_alter_system.pl
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 0a4b3e55ba..8d2fba2a5e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -52,6 +52,8 @@
quotes within the value. Quotes can usually be omitted if the value
is a simple number or identifier, however.
(Values that match an SQL keyword require quoting in some contexts.)
+ For parameters that take a list of strings, do not use an empty
+ string as an element, as that is not allowed.
</para>
</listitem>
diff --git a/doc/src/sgml/ref/set.sgml b/doc/src/sgml/ref/set.sgml
index 2218f54682..ce81993911 100644
--- a/doc/src/sgml/ref/set.sgml
+++ b/doc/src/sgml/ref/set.sgml
@@ -141,6 +141,12 @@ SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">value</replac
value it would have had if no <command>SET</command> had been executed
in the current session).
</para>
+ <para>
+ For a parameter that takes a list of strings, a single empty string
+ (<literal>''</literal>) may be specified to set the parameter to an
+ empty list. This is a special case; empty strings are not otherwise
+ allowed as elements of such lists.
+ </para>
</listitem>
</varlistentry>
</variablelist>
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3d6e6bdbfd..27ba1920e6 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3086,9 +3086,10 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
* rules used there aren't exactly like SQL's, we have to
* break the list value apart and then quote the elements as
* string literals. (The elements may be double-quoted as-is,
- * but we can't just feed them to the SQL parser; it would do
- * the wrong thing with elements that are zero-length or
- * longer than NAMEDATALEN.)
+ * but we can't just feed them to the SQL parser that way; it
+ * would truncate elements that are longer than NAMEDATALEN,
+ * which would be wrong if they're paths.) Also, we need a
+ * special case for empty lists.
*
* Variables that are not so marked should just be emitted as
* simple string literals. If the variable is not known to
@@ -3106,6 +3107,9 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
/* this shouldn't fail really */
elog(ERROR, "invalid list syntax in proconfig item");
}
+ /* Special case: represent an empty list as '' */
+ if (namelist == NIL)
+ appendStringInfoString(&buf, "''");
foreach(lc, namelist)
{
char *curname = (char *) lfirst(lc);
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 2c398cd9e5..d5ef492ee4 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -2753,7 +2753,7 @@ SplitIdentifierString(char *rawstring, char separator,
nextp++; /* skip leading whitespace */
if (*nextp == '\0')
- return true; /* allow empty string */
+ return true; /* empty string represents empty list */
/* At the top of the loop, we are at start of a new identifier. */
do
@@ -2777,6 +2777,8 @@ SplitIdentifierString(char *rawstring, char separator,
nextp = endp;
}
/* endp now points at the terminating quote */
+ if (curname == endp)
+ return false; /* empty quoted name not allowed */
nextp = endp + 1;
}
else
@@ -2880,7 +2882,7 @@ SplitDirectoriesString(char *rawstring, char separator,
nextp++; /* skip leading whitespace */
if (*nextp == '\0')
- return true; /* allow empty string */
+ return true; /* empty string represents empty list */
/* At the top of the loop, we are at start of a new directory. */
do
@@ -2904,6 +2906,8 @@ SplitDirectoriesString(char *rawstring, char separator,
nextp = endp;
}
/* endp now points at the terminating quote */
+ if (curname == endp)
+ return false; /* empty quoted name not allowed */
nextp = endp + 1;
}
else
@@ -3001,7 +3005,7 @@ SplitGUCList(char *rawstring, char separator,
nextp++; /* skip leading whitespace */
if (*nextp == '\0')
- return true; /* allow empty string */
+ return true; /* empty string represents empty list */
/* At the top of the loop, we are at start of a new identifier. */
do
@@ -3025,6 +3029,8 @@ SplitGUCList(char *rawstring, char separator,
nextp = endp;
}
/* endp now points at the terminating quote */
+ if (curname == endp)
+ return false; /* empty quoted name not allowed */
nextp = endp + 1;
}
else
diff --git a/src/backend/utils/misc/guc_funcs.c b/src/backend/utils/misc/guc_funcs.c
index b9e26982ab..a0557d164d 100644
--- a/src/backend/utils/misc/guc_funcs.c
+++ b/src/backend/utils/misc/guc_funcs.c
@@ -210,12 +210,30 @@ flatten_set_variable_args(const char *name, List *args)
else
flags = 0;
- /* Complain if list input and non-list variable */
- if ((flags & GUC_LIST_INPUT) == 0 &&
- list_length(args) != 1)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("SET %s takes only one argument", name)));
+ /*
+ * Handle special cases for list input.
+ */
+ if (flags & GUC_LIST_INPUT)
+ {
+ /* A single empty-string item is treated as an empty list. */
+ if (list_length(args) == 1)
+ {
+ Node *arg = (Node *) linitial(args);
+
+ if (IsA(arg, A_Const) &&
+ nodeTag(&((A_Const *) arg)->val) == T_String &&
+ *strVal(&((A_Const *) arg)->val) == '\0')
+ return pstrdup("");
+ }
+ }
+ else
+ {
+ /* Complain if list input and non-list variable. */
+ if (list_length(args) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("SET %s takes only one argument", name)));
+ }
initStringInfo(&buf);
@@ -269,6 +287,9 @@ flatten_set_variable_args(const char *name, List *args)
Datum interval;
char *intervalout;
+ /* gram.y ensures this is only reachable for TIME ZONE */
+ Assert(!(flags & GUC_LIST_QUOTE));
+
typenameTypeIdAndMod(NULL, typeName, &typoid, &typmod);
Assert(typoid == INTERVALOID);
@@ -286,9 +307,17 @@ flatten_set_variable_args(const char *name, List *args)
else
{
/*
- * Plain string literal or identifier. For quote mode,
+ * Plain string literal or identifier. In a list GUC,
+ * disallow empty-string elements (so that the preceding
+ * hack for empty lists is unambiguous). For quote mode,
* quote it if it's not a vanilla identifier.
*/
+ if ((flags & GUC_LIST_INPUT) && *val == '\0')
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("SET %s does not accept empty-string elements",
+ name)));
+
if (flags & GUC_LIST_QUOTE)
appendStringInfoString(&buf, quote_identifier(val));
else
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 05b84c0d6e..00a369c886 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -781,7 +781,7 @@ SplitGUCList(char *rawstring, char separator,
nextp++; /* skip leading whitespace */
if (*nextp == '\0')
- return true; /* allow empty string */
+ return true; /* empty string represents empty list */
/* At the top of the loop, we are at start of a new identifier. */
do
@@ -805,6 +805,8 @@ SplitGUCList(char *rawstring, char separator,
nextp = endp;
}
/* endp now points at the terminating quote */
+ if (curname == endp)
+ return false; /* empty quoted name not allowed */
nextp = endp + 1;
}
else
@@ -891,8 +893,9 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
* array. However, because the quoting rules used there aren't exactly
* like SQL's, we have to break the list value apart and then quote the
* elements as string literals. (The elements may be double-quoted as-is,
- * but we can't just feed them to the SQL parser; it would do the wrong
- * thing with elements that are zero-length or longer than NAMEDATALEN.)
+ * but we can't just feed them to the SQL parser that way; it would
+ * truncate elements that are longer than NAMEDATALEN, which would be
+ * wrong if they're paths.) Also, we need a special case for empty lists.
*
* Variables that are not so marked should just be emitted as simple
* string literals. If the variable is not known to
@@ -908,6 +911,9 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
/* this shouldn't fail really */
if (SplitGUCList(pos, ',', &namelist))
{
+ /* Special case: represent an empty list as '' */
+ if (*namelist == NULL)
+ appendPQExpBufferStr(buf, "''");
for (nameptr = namelist; *nameptr; nameptr++)
{
if (nameptr != namelist)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bea793456f..9d36a6a5aa 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -13699,8 +13699,9 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
* aren't exactly like SQL's, we have to break the list value apart
* and then quote the elements as string literals. (The elements may
* be double-quoted as-is, but we can't just feed them to the SQL
- * parser; it would do the wrong thing with elements that are
- * zero-length or longer than NAMEDATALEN.)
+ * parser that way; it would truncate elements that are longer than
+ * NAMEDATALEN, which would be wrong if they're paths.) Also, we need
+ * a special case for empty lists.
*
* Variables that are not so marked should just be emitted as simple
* string literals. If the variable is not known to
@@ -13716,6 +13717,9 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
/* this shouldn't fail really */
if (SplitGUCList(pos, ',', &namelist))
{
+ /* Special case: represent an empty list as '' */
+ if (*namelist == NULL)
+ appendPQExpBufferStr(q, "''");
for (nameptr = namelist; *nameptr; nameptr++)
{
if (nameptr != namelist)
diff --git a/src/test/modules/test_misc/meson.build b/src/test/modules/test_misc/meson.build
index 6b1e730bf4..40d50e245d 100644
--- a/src/test/modules/test_misc/meson.build
+++ b/src/test/modules/test_misc/meson.build
@@ -17,6 +17,7 @@ tests += {
't/006_signal_autovacuum.pl',
't/007_catcache_inval.pl',
't/008_replslot_single_user.pl',
+ 't/009_alter_system.pl.pl',
],
},
}
diff --git a/src/test/modules/test_misc/t/009_alter_system.pl b/src/test/modules/test_misc/t/009_alter_system.pl
new file mode 100644
index 0000000000..1f2d1e28e6
--- /dev/null
+++ b/src/test/modules/test_misc/t/009_alter_system.pl
@@ -0,0 +1,31 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+#
+# Test ALTER SYSTEM for list-based GUCs
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Initialize a new node and start it
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# Test setting a list GUC to an empty list
+$node->safe_psql('postgres', "ALTER SYSTEM SET search_path TO ''");
+$node->reload;
+
+my ($ret, $stdout, $stderr) = $node->psql('postgres', "SHOW search_path");
+is($stdout, '', 'ALTER SYSTEM SET search_path TO \'\' results in an empty list');
+
+# Test that ALTER SYSTEM rejects a list with an empty element
+($ret, $stdout, $stderr) = $node->psql('postgres', "ALTER SYSTEM SET search_path TO 'foo', ''");
+isnt($ret, 0, 'ALTER SYSTEM rejects list with empty element');
+like($stderr, qr/does not accept empty-string elements/, 'Correct error message for empty element in list');
+
+# Clean up GUC settings
+$node->safe_psql('postgres', "ALTER SYSTEM RESET search_path");
+$node->reload;
+
+done_testing();
diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out
index 7f9e29c765..e49e609415 100644
--- a/src/test/regress/expected/guc.out
+++ b/src/test/regress/expected/guc.out
@@ -31,6 +31,24 @@ SELECT '2006-08-13 12:34:56'::timestamptz;
2006-08-13 12:34:56-07
(1 row)
+-- Check handling of list GUCs
+SET search_path = 'pg_catalog', Foo, 'Bar';
+SHOW search_path;
+ search_path
+------------------------
+ pg_catalog, foo, "Bar"
+(1 row)
+
+SET search_path = ''; -- means empty list
+SHOW search_path;
+ search_path
+-------------
+
+(1 row)
+
+SET search_path = '', 'foo'; -- error, empty list elements not OK
+ERROR: SET search_path does not accept empty-string elements
+RESET search_path;
-- SET LOCAL has no effect outside of a transaction
SET LOCAL vacuum_cost_delay TO 50;
WARNING: SET LOCAL can only be used in transaction blocks
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 35e8aad770..43d5cf1026 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -3549,21 +3549,23 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
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'
+ SET temp_tablespaces to ''
+ 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$ +
+ 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 temp_tablespaces TO '' +
+ SET local_preload_libraries TO 'Mixed/Case', 'c:/''a"/path', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'+
+ AS $function$select 1;$function$ +
(1 row)
diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql
index f65f84a263..65630135f1 100644
--- a/src/test/regress/sql/guc.sql
+++ b/src/test/regress/sql/guc.sql
@@ -12,6 +12,14 @@ SHOW vacuum_cost_delay;
SHOW datestyle;
SELECT '2006-08-13 12:34:56'::timestamptz;
+-- Check handling of list GUCs
+SET search_path = 'pg_catalog', Foo, 'Bar';
+SHOW search_path;
+SET search_path = ''; -- means empty list
+SHOW search_path;
+SET search_path = '', 'foo'; -- error, empty list elements not OK
+RESET search_path;
+
-- SET LOCAL has no effect outside of a transaction
SET LOCAL vacuum_cost_delay TO 50;
SHOW vacuum_cost_delay;
diff --git a/src/test/regress/sql/rules.sql b/src/test/regress/sql/rules.sql
index fdd3ff1d16..5c5ff5e9cc 100644
--- a/src/test/regress/sql/rules.sql
+++ b/src/test/regress/sql/rules.sql
@@ -1217,7 +1217,8 @@ CREATE FUNCTION func_with_set_params() RETURNS integer
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'
+ SET temp_tablespaces to ''
+ SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
IMMUTABLE STRICT;
SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
--
2.47.0
