On Mon, Feb 17, 2025 at 8:50 PM Ilia Evdokimov
<ilya.evdoki...@tantorlabs.com> wrote:
>
>
> On 14.02.2025 19:47, Nathan Bossart wrote:
> > On Thu, Feb 13, 2025 at 05:01:59PM -0600, Nathan Bossart wrote:
> >> Okay, I took your suggestions in v7.
> > Committed. Thanks, David, Peter, and Daniel!
> >
>
> Hi,
>
> Maybe I'm being picky, but in auto_explain, the parameters
> log_min_duration and log_parameter_max_length do not follow the
> conventions we have adopted. I mean we should use numerals instead of
> words. I'm attaching a patch to fix this.
>
+1 for numerals, but I think there are still some problems with the v8 patch
- 0 and -1 are the wrong way around.
- We should separate the special values using a period (.) instead of
a comma (,) for consistency with all the others.
- It should adopt new wording that is more similar to the other GUCs.
For example.
DefineCustomIntVariable("auto_explain.log_min_duration",
"Sets the minimum execution time above which plans will be logged.",
- "Zero prints all plans. -1 turns this feature off.",
+ "0 prints all plans. -1 turns this feature off.",
SUGGESTION
"-1 disables logging plans. 0 means log all plans."
~~~
DefineCustomIntVariable("auto_explain.log_parameter_max_length",
"Sets the maximum length of query parameters to log.",
- "Zero logs no query parameters, -1 logs them in full.",
+ "0 logs no query parameters, -1 logs them in full.",
SUGGESTION #1 (main description part)
In fact that whole description seems incompatible with the
documentation. IMO it should be "Sets the maximum length of query
parameter VALUES to log.".
SUGGESTION #2 (special value part)
"-1 disables logging parameter values."
OR
"-1 disables logging parameter values. 0 means log parameter values in full"
TBH, I am unsure if this one should even mention the value 0 because
there are already examples of "max" GUCs similar to this that say
nothing about 0 since the meaning should be clear.
e.g. log_parameter_max_length doesn't mention 0.
e.g. log_parameter_max_length_on_error doesn't mention 0.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
