Re: bugfix: --echo-hidden is not supported by \sf statements

On Wed, Jan 23, 2013 at 12:06 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2013/1/14 Tom Lane <tgl@sss.pgh.pa.us>:
>> Well, fine, but then it should fix both of them and remove
>> minimal_error_message altogether.  I would however suggest eyeballing
>> what happens when you try "\ef nosuchfunction" (with or without -E).
>> I'm pretty sure that the reason for the special error handling in these
>> functions is that the default error reporting was unpleasant for this
>> use-case.
>
> so I rewrote patch
>
> still is simple
>
> On the end I didn't use PSQLexec - just write hidden queries directly
> from related functions - it is less invasive

Sorry for the delay, but I finally had a chance to review this patch.
I think the patch does a good job of bringing the behavior of \sf and
\ef in-line with the other meta-commands as far as --echo-hidden is
concerned.

About the code changes:

The bulk of the code change comes from factoring TraceQuery() out of
PSQLexec(), so that \ef and \sf may make use of this query-printing
without going through PSQLexec(). And not using PSQLexec() lets us
avoid a somewhat uglier error message like:
 ERROR:  function "nonexistent" does not exist LINE 1: SELECT 'nonexistent'::pg_catalog.regproc::pg_catalog.oid

Tom suggested removing minimal_error_message() entirely, which would
be nice if possible. It is a shame that "\sf nonexistent" produces a
scary-looking "ERROR: ...." message (and would cause any in-progress
transaction to need a rollback) while the other informational
metacommands do not. Actually, the other metacommands are not entirely
consistent with each other; compare "\dt nonexistent" and "\df
nonexistent".

It would be nice if the case of a matching function not found by \sf
or \ef could be handled in the same fashion as:
 # \d nonexistent Did not find any relation named "nonexistent".

though I realize that's not trivial due to the implementation of
lookup_function_oid(). At any rate, this nitpicking about the error
handling in the case that the function is not found is quibbling about
behavior unchanged by the patch. I don't have any complaints about the
patch itself, so I think it can be applied as-is, and we can attack
the error handling issue separately.

Josh