From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Ilia Evdokimov <ilya(dot)evdokimov(at)tantorlabs(dot)com> |
Cc: | Nathan Bossart <nathandbossart(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: describe special values in GUC descriptions more consistently |
Date: | 2025-02-17 21:14:58 |
Message-ID: | CAHut+PtxgXjY+Buz+u4gLHRHZ7SeioEjZx9szgJHfukSK_9wng@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Feb 17, 2025 at 8:50 PM Ilia Evdokimov
<ilya(dot)evdokimov(at)tantorlabs(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2025-02-17 21:33:12 | Re: Commitfest app release on Feb 17 with many improvements |
Previous Message | Andres Freund | 2025-02-17 21:01:42 | Re: AIO v2.3 |