Re: PATCH: psql tab completion for SELECT

On 6 April 2018 at 13:29, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> I looked at this a bit now. I think it still needs some work.

Hi Peter, thanks for the feedback.

> Some of the queries for older versions contain syntax errors that causes
> them not to work.
>
> For example, in 80100:
>
> "'internal'::regtype != ALL ([.proargtypes) "
>
> The query definition structures are missing a comma between selcondition
> and viscondition. This causes all the following fields to be
> misassigned. I'm not quite sure how it actually works at all some of
> the time. There might be another bug that offsets this one.

One of the problems was a missing comma before the viscondition value
in the struct!

/* selcondition */
"p.prorettype NOT IN ('trigger'::regtype, 'internal'::regtype) "
...
"AND p.oid NOT IN (SELECT amproc FROM pg_amproc) " <--
Should be a comma here!
/* viscondition */
"pg_catalog.pg_function_is_visible(p.oid)",

I did some fairly thorough testing against older servers, but that was
before I rewrote it to fit in Tom's versioned SchemaQuery. I'll fix
this and repeat the testing.

> I'd like to see a short comment what is different between each of the
> version queries. You had a list earlier in the thread.

Ok, I'll see if I can add that.

> The selection of which functions to show can be further refined.
>
> I don't think the contents of pg_amproc and pg_cast should be excluded.
> Some of those functions are useful. Similarly for pg_operator.oprcode.
>
> Things like oprrest and oprjoin will already be excluded because they
> have "internal" arguments. Similarly for some columns in pg_aggregate.
>
> There are also additional pseudo-types that should be excluded. See
> pg_type.typtype = 'p', except some of those should not be excluded.
> Needs more thought.

Ok I'll play with these.

Selection of which functions and attributes to show is something with
probably no right answer, so we might have to settle for "close
enough" at some point.

> Considering these issues, I think it would be appropriate to move this
> patch to the next commitfest.