Re: [PATCH] Proof of concept for GUC improvements

On Thu, Aug 19, 2021 at 3:17 PM David Christensen <
david(dot)christensen(at)crunchydata(dot)com> wrote:

> -hackers,
>
> Enclosed, find a POC patch that implements "special values" for int GUCs.
> We have quite a few GUCs
> with values that have special meaning atop other settings. I have
> attempted to identify these and
> make it so you can specify a symbol name for these values instead of just
> relying on the magic
> number instead.
>
> For instance, many GUCs have -1 for "disabled", so I've just made it so
> you can
> specify something like:
>
> SET log_min_duration_statement = disabled;
>
> And the raw value will be set to -1 in this case. For the purposes of
> testing, I have also added a
> new GUC "output_special_values" to affect whether `SHOW` or anything that
> relies on _ShowOption()
> can show with the special value instead of just the raw magic value,
> allowing tools to consume the
> original raw value, or provide the output to the user in the nicer format.
>
> This has only been done for ints, and the passthru I did was very quick,
> so I have probably missed
> some options that didn't explicitly have their interpretations in the file
> and/or I didn't know
> about it already. I do not think there are these sorts of values in other
> non-int GUCs, but there
> might be, so a similar approach could be taken to expand things to other
> config types in the future.
>
> Let me know your thoughts; I personally find this to be useful, and would
> be a nicer way for some
> configs to be displayed in the postgresql.conf file.
>
> Best,
>
> David
>
>
> --
>
Hi,
For parse_special_int():

+ * true. If it's not found, return false and retval is set to 0.
...
+ /* don't touch the return value in other case */
+ return false;

It seems the two comments are not consistent with each other (retval is not
set in case no entry is found).

For special_int_to_value():

+ * true. If it's not found, return false and retval is set to 0.

First, there is no assignment to retval at the end of the method. Second,
retval points to string, so it shouldn't be set to 0.

Cheers