Re: control max length of parameter values logged - Mailing list pgsql-hackers

Thanks for updating the patch.

On Thu, Apr 02, 2020 at 01:29:04AM +0100, Alexey Bashtanov wrote:
> +        If greater than zero, bind parameter values reported in non-error
> +        statement-logging messages are trimmed to no more than this many bytes.
> +        If this value is specified without units, it is taken as bytes.
> +        Zero disables logging parameters with statements.
> +        <literal>-1</literal> (the default) makes parameters logged in full.

say: "..causes parameters to be logged in full".

Also..I just realized that this truncates *each* parameter, rather than
truncating the parameter list.

So say: "
|If greater than zero, each bind parameter value reported in a non-error
|statement-logging messages is trimmed to no more than this number of bytes.

> +        Non-zero values add some overhead, as
> +        <productname>PostgreSQL</productname> will compute and store textual
> +        representations of parameter values in memory for all statements,
> +        even if they eventually do not get logged.        

say: "even if they are ultimately not logged"

> +++ b/src/backend/nodes/params.c
> @@ -280,6 +280,7 @@ BuildParamLogString(ParamListInfo params, char **knownTextValues, int maxlen)
>                  oldCxt;
>      StringInfoData buf;
>  
> +    Assert(maxlen == -1 || maxlen > 0);

You can write: >= -1

> -                    if (log_parameters_on_error)
> +                    if (log_parameter_max_length_on_error != 0)

I would write this as >= 0

> +                        if (log_parameter_max_length_on_error > 0)
> +                        {
> +                                                       /*
> +                                                        * We can trim the saved string, knowing that we
> +                                                        * won't print all of it.  But we must copy at
> +                                                        * least two more full characters than
> +                                                        * BuildParamLogString wants to use; otherwise it
> +                                                        * might fail to include the trailing ellipsis.
> +                                                        */
> +                                                       knownTextValues[paramno] =
> +                                                               pnstrdup(pstring,
> +                                                                                log_parameter_max_length_on_error
> +                                                                                + 2 * MAX_MULTIBYTE_CHAR_LEN);

The comment says we need at least 2 chars, but
log_parameter_max_length_on_error might be 1, so I think you can either add 64
byte fudge factor, like before, or do Max(log_parameter_max_length_on_error, 2).

> +                        }
> +                        else
> +                            knownTextValues[paramno] = pstrdup(pstring);

I suggest to handle the "== -1" case first and the > 0 case as "else".

Thanks,
-- 
Justin