Content-Type: multipart/mixed; boundary="------------xNCThCIDEZOnuod0kr04qvzx"
Message-ID: <4bea2dde-aa1d-46eb-b47a-dbbe43774ab2@tantorlabs.com>
Date: Tue, 18 Feb 2025 01:13:34 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: describe special values in GUC descriptions more consistently
To: Nathan Bossart <nathandbossart@gmail.com>,
 Peter Smith <smithpb2250@gmail.com>
Cc: "David G. Johnston" <david.g.johnston@gmail.com>,
 Daniel Gustafsson <daniel@yesql.se>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
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>
Content-Language: en-US
From: Ilia Evdokimov <ilya.evdokimov@tantorlabs.com>
In-Reply-To: <Z7OwZv8JuGT6Ytq0@nathan>
Archived-At: 
 <https://www.postgresql.org/message-id/4bea2dde-aa1d-46eb-b47a-dbbe43774ab2%40tantorlabs.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------xNCThCIDEZOnuod0kr04qvzx
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit


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.

--------------xNCThCIDEZOnuod0kr04qvzx
Content-Type: text/x-patch; charset=UTF-8;
 name="v9-0001-Standardize-parameter-descriptions-in-auto_explain.patch"
Content-Disposition: attachment;
 filename*0="v9-0001-Standardize-parameter-descriptions-in-auto_explain.p";
 filename*1="atch"
Content-Transfer-Encoding: base64

RnJvbSA0NGY2MDk0OWE0NjM2ZWI2NDlkMjlkNzUyYTU1YTg0MTAyMTNkMTJlIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBJbGlhIEV2ZG9raW1vdiA8aWx5YS5ldmRva2ltb3ZA
dGFudG9ybGFicy5jb20+CkRhdGU6IFR1ZSwgMTggRmViIDIwMjUgMDE6MDI6MTUgKzAzMDAK
U3ViamVjdDogW1BBVENIIHY5XSBTdGFuZGFyZGl6ZSBwYXJhbWV0ZXIgZGVzY3JpcHRpb25z
IGluIGF1dG9fZXhwbGFpbgoKQWxpZ24gdGhlIGRlc2NyaXB0aW9ucyBvZiBsb2dfbWluX2R1
cmF0aW9uIGFuZCBsb2dfcGFyYW1ldGVyX21heF9sZW5ndGgKaW4gYXV0b19leHBsYWluIHdp
dGggdGhlIGZvcm1hdHRpbmcgY29udmVudGlvbnMgdXNlZCBmb3Igb3RoZXIgR1VDIHBhcmFt
ZXRlcnMsCmFzIGVzdGFibGlzaGVkIGluIGNvbW1pdCA5NzdkODY1LgoKUmVtb3ZlIHRoZSBk
ZXNjcmlwdGlvbiBvZiB0aGUgemVybyB2YWx1ZSBmb3IgbG9nX3BhcmFtZXRlcl9tYXhfbGVu
Z3RoLAphcyB0aGVyZSBhcmUgbnVtZXJvdXMgZXhhbXBsZXMgb2Ygc2ltaWxhciAibWF4IiBH
VUMgcGFyYW1ldGVycyB0aGF0CmRvIG5vdCBleHBsaWNpdGx5IG1lbnRpb24gemVybywgYXMg
aXRzIG1lYW5pbmcgaXMgY29uc2lkZXJlZCBjbGVhci4KLS0tCiBjb250cmliL2F1dG9fZXhw
bGFpbi9hdXRvX2V4cGxhaW4uYyB8IDQgKystLQogMSBmaWxlIGNoYW5nZWQsIDIgaW5zZXJ0
aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9jb250cmliL2F1dG9fZXhw
bGFpbi9hdXRvX2V4cGxhaW4uYyBiL2NvbnRyaWIvYXV0b19leHBsYWluL2F1dG9fZXhwbGFp
bi5jCmluZGV4IGYxYWQ4NzZlODIuLjFjNmY4NDBlMzAgMTAwNjQ0Ci0tLSBhL2NvbnRyaWIv
YXV0b19leHBsYWluL2F1dG9fZXhwbGFpbi5jCisrKyBiL2NvbnRyaWIvYXV0b19leHBsYWlu
L2F1dG9fZXhwbGFpbi5jCkBAIC05Myw3ICs5Myw3IEBAIF9QR19pbml0KHZvaWQpCiAJLyog
RGVmaW5lIGN1c3RvbSBHVUMgdmFyaWFibGVzLiAqLwogCURlZmluZUN1c3RvbUludFZhcmlh
YmxlKCJhdXRvX2V4cGxhaW4ubG9nX21pbl9kdXJhdGlvbiIsCiAJCQkJCQkJIlNldHMgdGhl
IG1pbmltdW0gZXhlY3V0aW9uIHRpbWUgYWJvdmUgd2hpY2ggcGxhbnMgd2lsbCBiZSBsb2dn
ZWQuIiwKLQkJCQkJCQkiWmVybyBwcmludHMgYWxsIHBsYW5zLiAtMSB0dXJucyB0aGlzIGZl
YXR1cmUgb2ZmLiIsCisJCQkJCQkJIi0xIGRpc2FibGVzIGxvZ2dpbmcgcGxhbnMuIDAgbWVh
bnMgbG9nIGFsbCBwbGFucy4iLAogCQkJCQkJCSZhdXRvX2V4cGxhaW5fbG9nX21pbl9kdXJh
dGlvbiwKIAkJCQkJCQktMSwKIAkJCQkJCQktMSwgSU5UX01BWCwKQEAgLTEwNSw3ICsxMDUs
NyBAQCBfUEdfaW5pdCh2b2lkKQogCiAJRGVmaW5lQ3VzdG9tSW50VmFyaWFibGUoImF1dG9f
ZXhwbGFpbi5sb2dfcGFyYW1ldGVyX21heF9sZW5ndGgiLAogCQkJCQkJCSJTZXRzIHRoZSBt
YXhpbXVtIGxlbmd0aCBvZiBxdWVyeSBwYXJhbWV0ZXJzIHRvIGxvZy4iLAotCQkJCQkJCSJa
ZXJvIGxvZ3Mgbm8gcXVlcnkgcGFyYW1ldGVycywgLTEgbG9ncyB0aGVtIGluIGZ1bGwuIiwK
KwkJCQkJCQkiLTEgbWVhbnMgbG9nIHBhcmFtZXRlciB2YWx1ZXMgaW4gZnVsbC4iLAogCQkJ
CQkJCSZhdXRvX2V4cGxhaW5fbG9nX3BhcmFtZXRlcl9tYXhfbGVuZ3RoLAogCQkJCQkJCS0x
LAogCQkJCQkJCS0xLCBJTlRfTUFYLAotLSAKMi4zNC4xCgo=

--------------xNCThCIDEZOnuod0kr04qvzx--