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

We have never had a SET syntax that allows setting a GUC_LIST_INPUT
parameter to be an empty list.  A locution such as
	SET search_path = '';
doesn't mean that; it means setting the GUC to contain a single item
that is an empty string.  (For search_path the net effect is much the
same, because search_path ignores invalid schema names and '' must be
invalid.)  This is confusing, not least because configuration-file
entries and the set_config() function can easily produce empty-list
values.

We considered making the empty-string syntax do this, but that would
foreclose ever allowing empty-string items to be valid in list GUCs.
While there isn't any obvious use-case for that today, it feels like
the kind of restriction that might hurt someday.  Instead, let's
accept the forbidden-up-to-now value NULL and treat that as meaning an
empty list.  (An objection to this could be "what if we someday want
to allow NULL as a GUC value?".  That seems unlikely though, and even
if we did allow it for scalar GUCs, we could continue to treat it as
meaning an empty list for list GUCs.)

Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Andrei Klychkov <andrew.a.klychkov@gmail.com>
Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
Discussion: https://postgr.es/m/CA+mfrmwsBmYsJayWjc8bJmicxc3phZcHHY=yW5aYe=P-1d_4bg@mail.gmail.com

doc/src/sgml/ref/alter_system.sgml

@@ -84,6 +84,8 @@ ALTER SYSTEM RESET ALL
       constants, identifiers, numbers, or comma-separated lists of
       these, as appropriate for the particular parameter.
       Values that are neither numbers nor valid identifiers must be quoted.
+      If the parameter accepts a list of values, <literal>NULL</literal>
+      can be written to specify an empty list.
       <literal>DEFAULT</literal> can be written to specify removing the
       parameter and its value from <filename>postgresql.auto.conf</filename>.
      </para>
@@ -136,7 +138,15 @@ ALTER SYSTEM SET wal_level = replica;
    in <filename>postgresql.conf</filename>:
 <programlisting>
 ALTER SYSTEM RESET wal_level;
-</programlisting></para>
+</programlisting>
+  </para>
+
+  <para>
+   Set the list of preloaded extension modules to be empty:
+<programlisting>
+ALTER SYSTEM SET shared_preload_libraries TO NULL;
+</programlisting>
+  </para>
  </refsect1>
 
  <refsect1>

doc/src/sgml/ref/set.sgml

@@ -21,8 +21,8 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-SET [ SESSION | LOCAL ] <replaceable class="parameter">configuration_parameter</replaceable> { TO | = } { <replaceable class="parameter">value</replaceable> | '<replaceable class="parameter">value</replaceable>' | DEFAULT }
+SET [ SESSION | LOCAL ] <replaceable class="parameter">configuration_parameter</replaceable> { TO | = } { <replaceable class="parameter">value</replaceable> [, ...] | DEFAULT }
-SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">value</replaceable> | '<replaceable class="parameter">value</replaceable>' | LOCAL | DEFAULT }
+SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">value</replaceable> | LOCAL | DEFAULT }
 </synopsis>
  </refsynopsisdiv>
 
@@ -123,7 +123,7 @@ SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">value</replac
     <term><replaceable class="parameter">configuration_parameter</replaceable></term>
     <listitem>
      <para>
-      Name of a settable run-time parameter.  Available parameters are
+      Name of a settable configuration parameter.  Available parameters are
       documented in <xref linkend="runtime-config"/> and below.
      </para>
     </listitem>
@@ -133,9 +133,12 @@ SET [ SESSION | LOCAL ] TIME ZONE { <replaceable class="parameter">value</replac
     <term><replaceable class="parameter">value</replaceable></term>
     <listitem>
      <para>
-      New value of parameter.  Values can be specified as string
+      New value of the parameter.  Values can be specified as string
       constants, identifiers, numbers, or comma-separated lists of
       these, as appropriate for the particular parameter.
+      Values that are neither numbers nor valid identifiers must be quoted.
+      If the parameter accepts a list of values, <literal>NULL</literal>
+      can be written to specify an empty list.
       <literal>DEFAULT</literal> can be written to specify
       resetting the parameter to its default value (that is, whatever
       value it would have had if no <command>SET</command> had been executed
@@ -283,6 +286,19 @@ SELECT setseed(<replaceable>value</replaceable>);
    Set the schema search path:
 <programlisting>
 SET search_path TO my_schema, public;
+</programlisting>
+   Note that this is not the same as
+<programlisting>
+SET search_path TO 'my_schema, public';
+</programlisting>
+   which would have the effect of setting <varname>search_path</varname>
+   to contain a single, probably-nonexistent schema name.
+  </para>
+
+  <para>
+   Set the list of temporary tablespace names to be empty:
+<programlisting>
+SET temp_tablespaces TO NULL;
 </programlisting>
   </para>
 

src/backend/parser/gram.y

@@ -1716,6 +1716,26 @@ generic_set:
 					n->location = @3;
 					$$ = n;
 				}
+			| var_name TO NULL_P
+				{
+					VariableSetStmt *n = makeNode(VariableSetStmt);
+
+					n->kind = VAR_SET_VALUE;
+					n->name = $1;
+					n->args = list_make1(makeNullAConst(@3));
+					n->location = @3;
+					$$ = n;
+				}
+			| var_name '=' NULL_P
+				{
+					VariableSetStmt *n = makeNode(VariableSetStmt);
+
+					n->kind = VAR_SET_VALUE;
+					n->name = $1;
+					n->args = list_make1(makeNullAConst(@3));
+					n->location = @3;
+					$$ = n;
+				}
 			| var_name TO DEFAULT
 				{
 					VariableSetStmt *n = makeNode(VariableSetStmt);

src/backend/utils/adt/ruleutils.c

@@ -3087,7 +3087,8 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
 				 * 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.)
+				 * longer than NAMEDATALEN.)  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
@@ -3105,6 +3106,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 NULL */
+					if (namelist == NIL)
+						appendStringInfoString(&buf, "NULL");
 					foreach(lc, namelist)
 					{
 						char	   *curname = (char *) lfirst(lc);

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
@@ -2880,7 +2880,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
@@ -3001,7 +3001,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

src/backend/utils/misc/guc_funcs.c

@@ -210,12 +210,29 @@ 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 &&
+	 * Handle special cases for list input.
-		list_length(args) != 1)
+	 */
-		ereport(ERROR,
+	if (flags & GUC_LIST_INPUT)
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+	{
-				 errmsg("SET %s takes only one argument", name)));
+		/* NULL represents an empty list. */
+		if (list_length(args) == 1)
+		{
+			Node	   *arg = (Node *) linitial(args);
+
+			if (IsA(arg, A_Const) &&
+				((A_Const *) arg)->isnull)
+				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);
 
@@ -246,6 +263,12 @@ flatten_set_variable_args(const char *name, List *args)
 			elog(ERROR, "unrecognized node type: %d", (int) nodeTag(arg));
 		con = (A_Const *) arg;
 
+		/* Complain if NULL is used with a non-list variable. */
+		if (con->isnull)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("NULL is an invalid value for %s", name)));
+
 		switch (nodeTag(&con->val))
 		{
 			case T_Integer:
@@ -269,6 +292,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);
 

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
@@ -893,6 +893,7 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
 	 * 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.)
+	 * 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 +909,9 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
 		/* this shouldn't fail really */
 		if (SplitGUCList(pos, ',', &namelist))
 		{
+			/* Special case: represent an empty list as NULL */
+			if (*namelist == NULL)
+				appendPQExpBufferStr(buf, "NULL");
 			for (nameptr = namelist; *nameptr; nameptr++)
 			{
 				if (nameptr != namelist)

src/bin/pg_dump/pg_dump.c

@@ -13764,7 +13764,8 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 		 * 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.)
+		 * zero-length or longer than NAMEDATALEN.)  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
@@ -13780,6 +13781,9 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
 			/* this shouldn't fail really */
 			if (SplitGUCList(pos, ',', &namelist))
 			{
+				/* Special case: represent an empty list as NULL */
+				if (*namelist == NULL)
+					appendPQExpBufferStr(q, "NULL");
 				for (nameptr = namelist; *nameptr; nameptr++)
 				{
 					if (nameptr != namelist)

src/test/regress/expected/guc.out

@@ -31,6 +31,28 @@ 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 = null;  -- means empty list
+SHOW search_path;
+ search_path 
+-------------
+ 
+(1 row)
+
+SET search_path = null, null;  -- syntax error
+ERROR:  syntax error at or near ","
+LINE 1: SET search_path = null, null;
+                              ^
+SET enable_seqscan = null;  -- error
+ERROR:  NULL is an invalid value for enable_seqscan
+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

src/test/regress/expected/rules.out

@@ -3572,6 +3572,7 @@ 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 temp_tablespaces to NULL
     SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
     IMMUTABLE STRICT;
 SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
@@ -3585,6 +3586,7 @@ SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);
   SET extra_float_digits TO '2'                                                                                                                                          +
   SET work_mem TO '4MB'                                                                                                                                                  +
   SET "DateStyle" TO 'iso, mdy'                                                                                                                                          +
+  SET temp_tablespaces TO NULL                                                                                                                                           +
   SET local_preload_libraries TO 'Mixed/Case', 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'+
  AS $function$select 1;$function$                                                                                                                                        +
  

src/test/regress/sql/guc.sql

@@ -12,6 +12,15 @@ 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 = null;  -- means empty list
+SHOW search_path;
+SET search_path = null, null;  -- syntax error
+SET enable_seqscan = null;  -- error
+RESET search_path;
+
 -- SET LOCAL has no effect outside of a transaction
 SET LOCAL vacuum_cost_delay TO 50;
 SHOW vacuum_cost_delay;

src/test/regress/sql/rules.sql

@@ -1217,6 +1217,7 @@ 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 temp_tablespaces to NULL
     SET local_preload_libraries TO "Mixed/Case", 'c:/''a"/path', '', '0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789'
     IMMUTABLE STRICT;
 SELECT pg_get_functiondef('func_with_set_params()'::regprocedure);