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