Hi,
On 02/01/14 10:02, Andres Freund wrote:
> > Christian's idea of a context line seems plausible to me. I don't
> > care for this implementation too much --- a global variable? Ick.
>
> Yea, the data should be stored in ErrorContextCallback.arg instead.
Fixed.
I also palloc() the ErrorContextCallback itself. But it doesn't feel
right since I could not find a piece of code doing so. What do you
think?
> > Make a wrapper function for XactLockTableWait instead, please.
>
> I don't think that'd work out all too nicely - we do the
> XactLockTableWait() inside other functions like MultiXactIdWait(),
> passing all the detail along for those would end up being ugly. So using
> error context callbacks properly seems like the best way in the end.
Wrapping XactLockTableWait()/MultiXactIdWait() at least allows the
ErrorContextCallback and ErrorContextCallback.arg to live on the
stack. So what we could do is wrap this in a macro instead of a
function (like e.g. PG_TRY) or write two different functions. And I
don't like the two functions approach since it means duplication of
code.
While writing the response I really think writing a macro in PG_TRY
style for setting up and cleaning the error context callback I begin
to think that this would be the best way to go.
> > And I'm not real sure that showing the whole tuple contents is a good
> > thing; I can see people calling that a security leak, not to mention
> > that the performance consequences could be dire.
>
> I don't think that the security argument has too much merit given
> today's PG - we already log the full tuple for various constraint
> violations. And we also accept the performance penalty there. I don't
> see any easy way to select a sensible subset of columns to print here,
> and printing the columns is what would make investigating issues around
> this so much easier. At the very least we'd need to print the pkey
> columns and the columns of the unique key that might have been violated.
I agree. Even printing only the PK values would be much more
complex. As far as I can see we would even have to gain another lock
for this (see comment for relationHasPrimaryKey() in
src/backend/catalog/index.c).
Regards,
--
Christian Kruse http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services