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

From: Josh Kupershmidt <schmiddy(at)gmail(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: bugfix: --echo-hidden is not supported by \sf statements
Date: 2013-02-20 03:07:16
Message-ID: CAK3UJRH4J4Trg6OsdWB=hMKNxhJyaS8=gGwpzMJe-k=Y++qTcQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jan 23, 2013 at 12:06 PM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> 2013/1/14 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Joachim Wieland 2013-02-20 03:16:30 Re: posix_fadvise missing in the walsender
Previous Message Etsuro Fujita 2013-02-20 03:07:14 Re: Comment typo