Re: control max length of parameter values logged

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alexey Bashtanov <bashtanov(at)imap(dot)cc>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, alvherre(at)2ndquadrant(dot)com
Subject: Re: control max length of parameter values logged
Date: 2020-03-10 22:03:02
Message-ID: 27534.1583877782@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Alexey Bashtanov <bashtanov(at)imap(dot)cc> writes:
> I personally don't think that we necessarily need to cut the values, we
> can rely on the users
> being careful when using this feature -- in the same way we trusted them
> use similarly dangerous
> log_min_duration_statement and especially log_statement for ages. At
> least it's better than having
> no option to disable it. Alvaro's opinion was different [3]. What do you
> think
> of adding a parameter to limit max logged parameter length? See patch
> attached.

This patch is failing to build docs (per the cfbot) and it also fails
check-world because you changed behavior tested by ba79cb5dc's test case.
Attached is an update that hopefully will make the cfbot happy.

I agree that something ought to be done here, but I'm not sure that
this is exactly what. It appears to me that there are three related
but distinct behaviors under discussion:

1. Truncation of bind parameters returned to clients in error message
detail fields.
2. Truncation of bind parameters written to the server log in logged
error messages.
3. Truncation of bind parameters written to the server log in non-error
statement logging actions (log_statement and variants).

Historically we haven't truncated any of these, I believe. As of
ba79cb5dc we forcibly truncate #1 and #2 at 64 bytes, but not #3.
Your patch proposes to provide a SUSET GUC that controls the
truncation length for all 3.

I think that the status quo as of ba79cb5dc is entirely unacceptable,
because there is no recourse if you want to find out why a statement
is failing and the reason is buried in some long parameter string.
However, this patch doesn't really fix it, because it still seems
pretty annoying that you need to be superuser to adjust what gets
sent back to the client. Maybe that isn't a problem in practice
(since the client presumably has the original parameter value laying
about), but it seems conceptually wrong.

On the other hand, that line of reasoning leads to wanting two
separate GUCs (one for #1 and one for the other two), which seems
like overkill, plus it's going to be hard/expensive to have the
outputs for #1 and #2 not be the same.

I do agree that it seems weird and random (not to say 100% backward)
that error cases provide only truncated values but routine query
logging insists on emitting full untruncated values. I should think
that the most common use-cases would want it the other way round.

So I feel like we'd better resolve these definitional questions about
what behavior we actually want. I agree that ba79cb5dc is not
terribly well thought out as it stands.

regards, tom lane

Attachment Content-Type Size
log_parameter_max_length_v2.patch text/x-diff 6.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-03-10 22:17:51 Re: Index Skip Scan
Previous Message Andres Freund 2020-03-10 21:40:33 Re: shared-memory based stats collector