Thread: BUG #16376: ALTER SYSTEM incorrectly quotes empty list

The following bug has been logged on the website:

Bug reference:      16376
Logged by:          Teja Mupparti
Email address:      tejeswarm@hotmail.com
PostgreSQL version: 11.0
Operating system:   Linux
Description:

Some applications set Postgres parameters using GUI(s)  Once a value is set
and deselected later, it issues a SQL
ALTER SYSTEM set shared_preload_libraries = '';
is translating into shared_preload_libraries = '""' in the
postgresql.auto.conf, which will prevent Postgres server from starting
(illegal value of "")
FATAL:  could not access file "": No such file or directory.

The quick fix is in quote_identifier()
change
safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_');
to
safe = ((ident[0] >= 'a' && ident[0] <= 'z') || ident[0] == '_' || ident[0]
== '\0');

Which will avoid the unwnated double-quotes, but is opening a can of
worms.

Easy fix is in Alter system code path

+#define EMPTY_QUOTES "\"\""
+
 /*
  * Precision with which REAL type guc values are to be printed for GUC
  * serialization.
@@ -7886,6 +7888,22 @@ AlterSystemSetConfigFile(AlterSystemStmt
*altersysstmt)
                        FreeFile(infile);
                }

+               /*
+                * There is a special case where an empty list '' is
getting
+                * translated into '""' by the quoted_identifier() logic.
+                * For example, set shared_preload_libraries = '' is
written
+                * as shared_preload_libraries = '""' in the autoconfig
file
+                * and the subsequent restart fails with the below error.
+                *
+                * FATAL:  could not access file "": No such file or
directory
+                *
+                * Fixing quoted_identifier() breaks other parts of the
code,
+                * where it depends on translating '' to "". If the list
is
+                * empty, set the value to NULL (this will remove the
entry
+                * from the auto-config file)
+                */
+               if (!strcmp(value, EMPTY_QUOTES))
+                                       value = '\0';
--

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Fri, Apr 17, 2020 at 2:21 PM PG Bug reporting form <
> noreply@postgresql.org> wrote:
>> Some applications set Postgres parameters using GUI(s)  Once a value is set
>> and deselected later, it issues a SQL
>> ALTER SYSTEM set shared_preload_libraries = '';

> Well, its their bug, not ours, given that we specify two valid ways to
> accomplish this goal.  SET TO DEFAULT, and RESET.

Yes, this is *not* a bug, it's acting as designed.  That syntax does
not result in a zero-length list, it results in a list with one
empty-string entry.  We have the same behavior with search_path, which
is parsed in the same way.  (It's somewhat masked for search_path by
the decision to ignore entries that correspond to nonexistent schemas.)

There are things we might want to do about this.  It's somewhat
unfortunate that there's no way to set a GUC_LIST_INPUT variable to an
empty list in SET (since RESET might not do that, depending on what the
default value is).  And perhaps we ought to try harder to validate
shared_preload_libraries and its ilk when they are set.  But I disagree
that there's any parsing bug in the case at hand.

Another thing that's rather unfortunate is that the syntax is
different in postgresql.conf than it is in SQL --- in a config
file, setting shared_preload_libraries = '' *does* set it to
an empty list.  Which may have contributed to the thinko here.
But I'm afraid we're a decade or two too late to redefine the
config file syntax.

            regards, tom lane