> 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.
Hello Fujii,
Thanks for your review!
I'm personally not sure because this is my first patch and I'm trying to solve a specific issue of the postgresql.auto.conf-related server crashes.
If what your *broader-impact* suggestion makes sense to more experienced devs in this area, I'd be happy to update the patch as you put it, test it (as much as I can), and re-submit v3.
Otherwise, I'd be happy with the v2 implementation that seemingly solves my specific issue.
Thanks
Regards
Andrew
On Wed, Sep 3, 2025 at 6:59 PM Andrei Klychkov
<andrew.a.klychkov@gmail.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
