Re: [PATCH] Proof of concept for GUC improvements - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: [PATCH] Proof of concept for GUC improvements
Date
Msg-id CALNJ-vQvLJ5RaQOSY=qKqKdcT1V6gcdqpQpyPHRLVDmtL=hrsQ@mail.gmail.com
Whole thread Raw
In response to [PATCH] Proof of concept for GUC improvements  (David Christensen <david.christensen@crunchydata.com>)
Responses Re: [PATCH] Proof of concept for GUC improvements  (David Christensen <david.christensen@crunchydata.com>)
List pgsql-hackers


On Thu, Aug 19, 2021 at 3:17 PM David Christensen <david.christensen@crunchydata.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

pgsql-hackers by date:

Previous
From: David Christensen
Date:
Subject: Re: [PATCH] Proof of concept for GUC improvements
Next
From: "David G. Johnston"
Date:
Subject: Re: [PATCH] Proof of concept for GUC improvements