Re: Adding some error context for lock wait failures

I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> There are a few places that do
>> ereport(...);
>> /* Flush any strings created in ErrorContext */
>> FlushErrorState();
>> That'd be obsoleted by this change, right?

> Oh, I see them, all in guc.c.  Yeah, we should get rid of those;
> they seem not too safe anyway given that they're unconditional.

Oh, I take that back: we need to keep those, because what they
are for is to clean up strings created by GUC_check_errdetail
and friends, which will happen before the ereport call.  The
case where it's problematic is if the error logging level is
high enough that errstart decides there's nothing to do: then
we won't reach errfinish and that cleanup won't happen.

Conceivably we could deal with that scenario by having errstart
do the MemoryContextReset if it takes the no-op path, but I find
that a bit scary.  Besides, that path is supposed to be fast.

So we'd better keep those calls in guc.c, but I'll change their
comments ...

            regards, tom lane