Re: [HACKERS] PATCH: psql tab completion for SELECT

On 11 January 2018 at 03:28, Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> wrote:
> On 01/10/2018 06:38 AM, Edmund Horner wrote:
>> Regarding support for older versions, psql fails silently if a tab
>> completion query fails.
>
> No it doesn't, see below.
>
>> We could just let it do this, which is what
>> happens with, for example, ALTER PUBLICATION against a 9.6 server. I
>> can't see any existing completions that check the server version --
>> but completions that don't work against older versions, like ALTER
>> PUBLICATION, also aren't useful for older versions. SELECT is a bit
>> different as it can be useful against older versions that don't have
>> the pg_aggregate.aggcombinefn that my query uses for filtering out
>> aggregation support functions.
>
> That's a bug in my opinion, but not one that needs to be addressed by
> this patch.
>
> There are no completions that cater to the server version (yet) but all
> the \d stuff certainly does. You can see that in action in
> src/bin/psql/describe.c as well as all over the place in pg_dump and the
> like.
>
>> There's also the small irritation that when a completion query fails,
>> it aborts the user's current transaction to provide an incentive for
>> handling older versions gracefully.
>
> That is actively hostile and not at all what I would consider "silently
> failing". If you don't want to do the multiple versions thing, you
> should at least check if you're on 9.6+ before issuing the query.

There's also the less-critical problem that you can't complete
anything from an already-failed transaction.

Let's just talk about a separate patch that might improve this.

I can think of two options:

1. Use a separate connection for completions. The big problem with
this is people want to complete on objects created in their current
transaction. Maybe there's a way to use SET TRANSACTION SNAPSHOT to
access the user's transaction but this seems far too intrusive just
for a bit of tab completion.

2. Use savepoints. In exec_query you'd have:

SAVEPOINT _psql_tab_completion;
run the query
ROLLBACK TO _psql_tab_completion; // Just in case there was an
error, but safe to do anyway.
RELEASE SAVEPOINT _psql_tab_completion; // This may not be worth doing.

It doesn't help with tab completion in already-failed transactions.
But would it be a reasonable way to make tab completion safer? I
don't know whether savepoint creation/rollback/release has some
cumulative cost that we'd want to avoid incurring.