Subject: Re: [HACKERS] PATCH: psql tab completion for SELECT
To: Edmund Horner <ejrh00@gmail.com>, pgsql-hackers@postgresql.org
References: 
 <CAMyN-kB_xrU4iYdcF1j=tijgO1DSyjtb3j96O4UEj91XZrZcMg@mail.gmail.com>
From: Vik Fearing <vik.fearing@2ndquadrant.com>
Organization: 2ndQuadrant France
Message-ID: <034f9a0a-d426-251a-5f81-6083d730ee9e@2ndquadrant.com>
Date: Tue, 9 Jan 2018 16:45:41 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: 
 <CAMyN-kB_xrU4iYdcF1j=tijgO1DSyjtb3j96O4UEj91XZrZcMg@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/06/2017 05:28 AM, Edmund Horner wrote:
> Hi pgsql-hackers,
> 
> Here's a little draft patch to add *some* tab completion ability for
> SELECT in psql.  I have often missed the ability, especially with
> invocations of utility functions.

Yes, indeed!  The vast majority of what I use interactive psql for is to
call functions.  The fact that it does columns as well is nice, and not
doing anything after the first isn't a problem as we have many other
places where that's the case.  It's annoying, but consistent with
current annoying behavior.  So no points off for that.

> What my patch does:
> 
> For a command line with the single word SELECT, column names and
> function names will be used for completions of the next word.
> Function names have a "(" suffix added, largely because it makes them
> easier to distinguish in the completion list.

And because the user is going to have to type one anyway.  The
completion mechanism adds a space after the paren, which is a needle in
the eye of my OCD, but that's not this patch's fault.

> Only the first select-list item can be tab-completed; i.e. SELECT foo,
> b<tab> won't find column bar.

That's not a problem for me.

> How it works:
> 
> The implementation uses a normal non-schema query, because columns
> don't have schemas.
> 
> The eligible columns are normal, visible columns.

At first I thought this was too broad, but of course it's not because
the column will be visible if the user schema-qualifies the table it's in.

> I have tried to filter out the various functions which aren't likely
> to be directly used in queries.
> 
> The eligible functions are those which are:
>   - visible

Yes.

>   - not aggregate or window functions

Why?  I think this is a big mistake.

>   - not RI_FKey_* functions

Agreed.

>   - not support functions for types, aggregates, operators, languages,
> casts, access methods.

That list looks good to me.  But you forgot to exclude trigger functions.

> Is this likely to be a useful addition to PostgreSQL?

Yes, very much so.

> Some other questions about how it should be done:
> 
>   - Are my criteria for the columns and function names appropriate?

No, I don't think so, as mentioned above.

>   - Should I try to use a SchemaQuery so that function schema names can be used?
> 
>   - Should I try to support simple cases of multiple columns?  (How?
> We can use TailMatches1(",") but how do we tell we aren't into the
> FROM-clause or later?)

I think these two can be pushed to a later patch.  I'd rather have what
you've got now than nothing at all.

>   - How do we make it work with older servers, e.g. those that predate
> some of the newer columns used in the function criteria?

This is something that's going to have to be addressed if this patch is
to be committed.  The way to do this is by writing a query for each
different version you need to support and then call the right one
depending on the pset.sversion global variable.

Marking as Waiting on Author.
-- 
Vik Fearing                                          +33 6 46 75 15 36
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support