Re: Patch: show relation and tuple infos of a lock to acquire

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