From: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
To: Nathan Bossart <nathandbossart@gmail.com>
To: Peter Smith <smithpb2250@gmail.com>
Cc: David G. Johnston <david.g.johnston@gmail.com>
Cc: Daniel Gustafsson <daniel@yesql.se>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: describe special values in GUC descriptions more consistently
Date: Tue, 18 Feb 2025 01:13:34 +0300
Message-ID: <4bea2dde-aa1d-46eb-b47a-dbbe43774ab2@tantorlabs.com> (raw)
In-Reply-To: <Z7OwZv8JuGT6Ytq0@nathan>
References: <Z60M1AmVLCODshn8@nathan>
<CAHut+PtanfZy6gpRoSBEjsxKRc57v7zoz8oADFHpz_joknYRfA@mail.gmail.com>
<Z60k4qYL7mtPtB3O@nathan>
<CAKFQuwbEzaX7Ssw4=ZLkdRG7142MpYNkDGZmMN=RmQXLreB3Tg@mail.gmail.com>
<Z65vw3mBv8wa7Wfj@nathan>
<CAKFQuwYx_=44pZ1Pp-3fRFHt36G02Ldq2Db-phtD+EXLJ7VEFA@mail.gmail.com>
<Z65556VoM7eP-cuc@nathan>
<Z69zmd7ja4L0Ipo9@nathan>
<e82d4647-ce7f-45c7-9b01-fb900a050767@tantorlabs.com>
<CAHut+PtxgXjY+Buz+u4gLHRHZ7SeioEjZx9szgJHfukSK_9wng@mail.gmail.com>
<Z7OwZv8JuGT6Ytq0@nathan>
On 18.02.2025 00:55, Nathan Bossart wrote:
> On Tue, Feb 18, 2025 at 08:14:58AM +1100, Peter Smith wrote:
>> SUGGESTION
>> "-1 disables logging plans. 0 means log all plans."
> +1
>
>> 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.".
> I'm not sure omitting "values" changes the meaning in any discernible way,
> but I'm not opposed to adding it.
>
>> 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.
> I think you've got the meanings swapped here. It should be:
>
> -1 means log parameter values in full. 0 disables logging parameter values.
>
> But I agree that we can omit the description for 0 here.
>
Thank you for reviewing! I agree with all of them. I updated patch v9
with these changes.
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.
Attachments:v9-0001-Standardize-parameter-descriptions-in-auto_explain.patch (text/x-patch, 1.7K) download
From 44f60949a4636eb649d29d752a55a8410213d12e Mon Sep 17 00:00:00 2001
From: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
Date: Tue, 18 Feb 2025 01:02:15 +0300
Subject: [PATCH v9] Standardize parameter descriptions in auto_explain
Align the descriptions of log_min_duration and log_parameter_max_length
in auto_explain with the formatting conventions used for other GUC parameters,
as established in commit 977d865.
Remove the description of the zero value for log_parameter_max_length,
as there are numerous examples of similar "max" GUC parameters that
do not explicitly mention zero, as its meaning is considered clear.
---
contrib/auto_explain/auto_explain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.cindex f1ad876e82..1c6f840e30 100644--- a/contrib/auto_explain/auto_explain.c+++ b/contrib/auto_explain/auto_explain.c@@ -93,7 +93,7 @@ _PG_init(void)
/* Define custom GUC variables. */
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.",+ "-1 disables logging plans. 0 means log all plans.",
&auto_explain_log_min_duration,
-1,
-1, INT_MAX,
@@ -105,7 +105,7 @@ _PG_init(void)
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.",+ "-1 means log parameter values in full.",
&auto_explain_log_parameter_max_length,
-1,
-1, INT_MAX,
--
2.34.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ilya.evdokimov@tantorlabs.com, nathandbossart@gmail.com, smithpb2250@gmail.com, david.g.johnston@gmail.com, daniel@yesql.se
Subject: Re: describe special values in GUC descriptions more consistently
In-Reply-To: <4bea2dde-aa1d-46eb-b47a-dbbe43774ab2@tantorlabs.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox