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

On Wed, Sep 3, 2025 at 6:59 PM Andrei Klychkov
<andrew(dot)a(dot)klychkov(at)gmail(dot)com> wrote:
>
> Hi Jim,
>
> Thanks a lot for reviewing! Nice catch, TIL!
> Version 2 of the patch is attached, please check it out.
> In a nutshell, the issue actually wasn't in the flatten_set_variable_args() function as initially suspected, but rather in the configuration file writing logic in the write_auto_conf_file(): more details in v2_README.md

Even with this patch, an empty string set via SET is still quoted. For example:

=# SET local_preload_libraries TO '';
SET
=# SHOW local_preload_libraries ;
local_preload_libraries
-------------------------
""
(1 row)

Is this behavior acceptable? I was thinking that an empty string should not
be quoted, regardless of whether it's set by ALTER SYSTEM SET or SET.
Thought?

If we agree it should be handled in both cases, flatten_set_variable_args()
seems to need to be updated. For example:

@@ -289,7 +289,8 @@ flatten_set_variable_args(const char *name, List *args)
* Plain string literal or
identifier. For quote mode,
* quote it if it's not a
vanilla identifier.
*/
- if (flags & GUC_LIST_QUOTE)
+ if (flags & GUC_LIST_QUOTE &&
+ !(val[0] == '\0' &&
list_length(args) == 1))

appendStringInfoString(&buf, quote_identifier(val));
else

appendStringInfoString(&buf, val);

Regards,

--
Fujii Masao