Thread: Logging parameters values on bind error

Logging parameters values on bind error

From
Jehan-Guillaume de Rorthais
Date:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

We were struggling today with some java code binding values violating
some constraints in a prepared statement.

We don't own the code and couldn't make tests with it. So we tried to
find if PostgreSQL was able to log binded values when the BIND
operation fail and found that this is not possible in current release:
the error raised while planing the statement is not caught.

PFA a patch that catch any error while creating the query plan and add
parameters values to the error message if log_statement or
log_min_duration_statement would have logged it.

Comments?
- --
Jehan-Guillaume de Rorthais
http://www.dalibo.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlBy/cgACgkQXu9L1HbaT6Li5QCdEa9Zc4g302znpHmrwB9dnRBI
JSwAnR2Poil0QAP6b+TflM2ebDCPLq3G
=HU3H
-----END PGP SIGNATURE-----

Attachment

Re: Logging parameters values on bind error

From
Tom Lane
Date:
Jehan-Guillaume de Rorthais <jgdr@dalibo.com> writes:
> PFA a patch that catch any error while creating the query plan and add
> parameters values to the error message if log_statement or
> log_min_duration_statement would have logged it.

If that works, it's only by accident --- you're not supposed to add more
info to an error object after the error has been thrown.  What's worse,
if the original error had a DETAIL message, you're overwriting it.

The right way to do this type of thing, which is also cheaper than
PG_TRY on most platforms, is to set up an ErrorContextCallback stack
entry to call a function that adds extra information as a CONTEXT line.
There are lots of examples in the code base.

However ... I'm unconvinced that this can work safely at all.  Note the
check in errdetail_params that causes it to not do anything in an
aborted transaction.  Once you have caught an error, the current
transaction really has to be considered aborted, even if xact.c hasn't
been told about it yet.  So this patch is cheating with both hands, and
it will break given the right combination of error and parameter
datatypes.

A less dangerous approach would be to only attempt to print parameter
values that were supplied in text format.
        regards, tom lane