Re: PATCH: psql tab completion for SELECT

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Edmund Horner <ejrh00(at)gmail(dot)com>, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: PATCH: psql tab completion for SELECT
Date: 2018-03-05 20:52:06
Message-ID: 14268.1520283126@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> If people like this approach, I propose to commit this more or less
> as-is. The select-tab-completion patch would then need to be rewritten
> to use this infrastructure, but I think that should be straightforward.
> As a separate line of work, the infrastructure could be applied to fix
> the pre-existing places where tab completion fails against old servers.
> But that's probably work for v12 or beyond, unless somebody's really
> motivated to do it right now.

Pushed, but while looking it over a second time, I noticed a discrepancy
that ought to be resolved. In Query_for_list_of_functions, we now have
this for server version >= 11

/* selcondition */
"p.prokind IN ('f', 'w')",

and this for prior versions:

/* selcondition */
NULL,

The prokind variant is as Peter had it, and NULL is what we were using
here in v10 and earlier. But it strikes me that these are inconsistent,
in that the prokind variant rejects aggregates while the other variant
doesn't. We should decide which behavior we want and make that happen
consistently regardless of server version.

I believe the primary reason why the old code didn't reject aggregates
is that there is no GRANT ON AGGREGATE syntax, so that we really need to
include aggregates when completing GRANT ... ON FUNCTION. Also, other
commands such as DROP FUNCTION will accept an aggregate, although that's
arguably just historical laxity.

If we wanted to tighten this up, one way would be to create a separate
Query_for_list_of_functions_or_aggregates that allows both, and use it
for (only) the GRANT/REVOKE case. I'm not sure it's worth the trouble
though; I do not recall hearing field complaints about the laxity of
the existing completion rules. So my inclination is to change the
v11 code to be "p.prokind != 'p'" and leave it at that.

Thoughts?

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2018-03-05 20:59:36 Re: [HACKERS] Partition-wise aggregation/grouping
Previous Message Michael Banck 2018-03-05 20:41:51 Re: [PoC PATCH] Parallel dump to /dev/null