Re: bugfix: incomplete implementation of errhidecontext

Andres Freund <andres@anarazel.de> writes:
> On 2015-06-08 14:44:53 +0000, Jeevan Chalke wrote:
>> This is trivial bug fix in the area of hiding error context.
>>
>> I observed that there are two places from which we are calling this function
>> to hide the context in log messages. Those were broken.

> Broken in which sense? They did prevent stuff to go from the server log?

> I'm not convinced that hiding stuff from the client is really
> necessarily the same as hiding it from the server log. We e.g. always
> send the verbose log to the client, even if we only send the terse
> version to the server log.  I don't mind adjusting things for
> errhidecontext(), but it's not "just a bug".

Not only is it not "just a bug", I disagree that it's a bug at all.
The documentation of the errhidestmt function is crystal clear about
what it does:

 * errhidecontext --- optionally suppress CONTEXT: field of log entry

That says "log entry", not anything else.  Furthermore, this is clearly
modeled on errhidestmt(), which also only affects what's written to the
log.

Generally our position on error reporting is that it's the client's
responsibility to decide what parts of a report it will or won't show
to the user, so even if we agreed the overall behavior was undesirable,
I do not think this is the appropriate fix.

I especially object to the part of the patch that suppresses calling the
context callback stack functions; that's just introducing inconsistent
behavior for no reason.  It doesn't prevent collection of context (there
are lots of errcontext() calls directly in ereports, which this wouldn't
stop), and it will break callers that are using those callbacks for
anything more than just calling errcontext().  An example here is that in
clauses.c's sql_inline_error_callback, this would not only suppress the
CONTEXT line but also reporting of the error cursor location.

What is the actual use-case that prompted this complaint?

                        regards, tom lane