From: Vik Fearing <vik.fearing@2ndquadrant.com>
To: Edmund Horner <ejrh00@gmail.com>
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: PATCH: psql tab completion for SELECT
Date: Mon, 22 Jan 2018 19:41:20 +0100
Message-ID: <326b26f4-e1d4-c2f1-9110-aae396193f15@2ndquadrant.com> (raw)
In-Reply-To: <CAMyN-kCyeh2E7k1Dhb3YQKa-bSp_wHvqZsWoGRHKh_ukfLm9WQ@mail.gmail.com>
References: <CAMyN-kB_xrU4iYdcF1j=tijgO1DSyjtb3j96O4UEj91XZrZcMg@mail.gmail.com>
<CAMyN-kCyeh2E7k1Dhb3YQKa-bSp_wHvqZsWoGRHKh_ukfLm9WQ@mail.gmail.com>
On 01/12/2018 06:59 AM, Edmund Horner wrote:
> Hi, here's a new patch.
>
> This one makes some changes to the criteria for which functions to
> include; namely filtering out trigger functions and those that take or
> return values of type "internal"; and including aggregate and window
> functions. Some of the other checks could be removed as they were
> covered by the "internal" check.
This looks better. One minor quibble I have is your use of != (which
PostgreSQL accepts) instead of <> (the SQL standard). I'll let the
committer worry about that.
> It also uses the server version to determine which query to run. I
> have not written a custom query for every version from 7.1! I've
> split up the server history into:
>
> pre-7.3 - does not even have pg_function_is_visible. Not supported.
> pre-9.0 - does not have several support functions in types, languages,
> etc. We don't bother filtering using columns in other tables.
> pre-9.6 - does not have various aggregate support functions.
> 9.6 or later - the full query
I'm not sure how overboard we need to go with this going backwards so
what you did is probably fine.
> I was able to test against 9.2, 9.6, and 10 servers, but compiling and
> testing the older releases was a bit much for a Friday evening. I'm
> not sure there's much value in support for old servers, as long as we
> can make completion queries fail a bit more gracefully.
pg_dump stops at 8.0, we can surely do the same.
Now for some other points:
Your use of Matches1 is wrong, you should use TailMatches1 instead.
SELECT is a fully reserved keyword, so just checking if it's the
previous token is sufficient. By using Matches1, you miss cases like
this: SELECT (SELECT <tab>
It also occurred to me that SELECT isn't actually a complete command (or
whatever you want to call it), it's an abbreviation for SELECT ALL. So
you should check for SELECT, SELECT ALL, and SELECT DISTINCT. (The FROM
tab completion is missing this trick but that's a different patch)
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: vik.fearing@2ndquadrant.com, ejrh00@gmail.com
Subject: Re: PATCH: psql tab completion for SELECT
In-Reply-To: <326b26f4-e1d4-c2f1-9110-aae396193f15@2ndquadrant.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox