> 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 4:48 PM Fujii Masao <[email protected]> wrote: > On Wed, Sep 3, 2025 at 6:59 PM Andrei Klychkov > <[email protected]> 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 >
