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.
--
nathan
