Re: log bind parameter values on error - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: log bind parameter values on error
Date
Msg-id 20191205231550.GA28677@alvherre.pgsql
Whole thread Raw
In response to Re: log bind parameter values on error  (Alvaro Herrera <alvherre@2ndquadrant.com>)
Responses Re: log bind parameter values on error
List pgsql-hackers
On 2019-Dec-05, Alvaro Herrera wrote:

> > Makes me wonder though, if we ought to invent something similar to the
> > errdata fields we have for schema, table, etc...
> 
> Hmm, perhaps we should do that.  It avoids the problem altogether.

... then again, I'm not up for writing everything that would be required
to do that.  If somebody wants to propose that separately, I volunteer
to review it.  But let's not have that block this patch, since this is
already a clear improvement.

So here's v16.  I got rid of all that strlen() business of the output
values; instead, preallocate a reasonable guess at the final length (64
bytes for each of the first 128 parameters, plus 16 bytes for for
parameter from 129 to 1024).  This boils down to exactly the initial
size of a stringinfo (1024 bytes) when there are 8 parameters or less.
64 bytes avg of guessed length should be plenty (and it will be even
more so when I add the patch to limit the output length to 64+epsilon
bytes per param)  I admit there are perhaps too many magical numbers in
those three lines, though.

I don't understand how we ended up with the params put in
errdetail_log() -- seems to have been introduced silently between v13
and v14.  What's the reason for that?  I put it back as errdetail().
Alexey added some discussion about using context/detail when he sent
that version, but I think that's an issue we can leave for the
hypothetical errparameters() patch.


One problem I noticed is that we don't log parameters when
exec_bind_message fetches the parameter values.  So the very first
example problem in testlibpq5 fails to print the values of any
parameters previously seen.  I don't think this is a real problem in
practice.  You still get the unparseable value in the error message from
the input function, so not having the errdetail() does not seem very
important.

If anybody would like to review it once more, now's the time.  I'm
aiming at getting it pushed tomorrow (including the length-limited
appendStringInfoStringQuoted stuff).

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: Misleading comment in pg_upgrade.c
Next
From: Tom Lane
Date:
Subject: Re: Runtime pruning problem