Re: PATCH: psql tab completion for SELECT

(trimmed CC list to evade gmail's spam filter, sorry)

On 21/03/18 08:51, Edmund Horner wrote:
> Hi all, I haven't heard anything for a while and so assume you're
> beavering away on real features. :)
>
> I've been dogfooding this patch at work, and I am personally pretty
> happy with it.
>
> I still think the number of completions on an empty string is a bit
> too big, but I don't know what to omit. There are around 1700
> completions on the empty "postgres" database in my testing, and we
> show the first 1000 (arbitrarily limited, as the generated SQL has
> LIMIT 1000 but no ORDER BY).
>
> Should we just leave it as is?

> Whether we improve the filtering or not, I'm optimistic the feature
> will be committed in this CF or the next.
>
> I've also been working on adding support for completions after commas,
> but I really want to see the current feature finished first.

Playing around with this a little bit, I'm not very satisfied with the
completions. Sometimes this completes too much, and sometimes too
little. All of this has been mentioned in this and the other thread [1]
already, this just my opinion on all this.

Too much:

postgres=# \d lp
Table "public.lp"
Column | Type | Collation | Nullable | Default
----------+---------+-----------+----------+---------
id | integer | | |
partkey | text | | |
partcol1 | text | | |
partcol2 | text | | |
Partition key: LIST (partkey)
Number of partitions: 1000 (Use \d+ to list them.)

postgres=# select pa[TAB]
pad_attribute parameter_default parameter_style partattrs
partcol2 partexprs partrelid
page parameter_mode parameter_types partclass
partcollation partkey partstrat
pageno parameter_name parse_ident( partcol1
partdefid partnatts passwd

Too little:

postgres=# select partkey, p[TAB]
[no completions]

Then there's the multi-column case, which seems weird (to a user - I
know the implementation and understand why):

postgres=# select partkey, partc[TAB]
[no completions]

And I'd love this case, where go back to edit the SELECT list, after
already typing the FROM part, to be smarter:

postgres=# select p[TAB] FROM lp;
Display all 370 possibilities? (y or n)

There's something weird going on with system columns, from a user's
point of view:

SELECT oi[TAB]
oid oidvectortypes(

postgres=# select xm[TAB]
xmin xmlcomment( xml_is_well_formed_content(
xmlvalidate(
xmlagg( xml_is_well_formed(
xml_is_well_formed_document(

So oid and xmin are completed. But "xmax" and "ctid" are not. I think
this is because in fact none of the system columns are completed, but
there happen to be some tables with regular columns named "oid" and
"xmin". So it makes sense once you know that, but it's pretty confusing
to a user. Tab-completion is a great way for a user to explore what's
available, so it's weird that some system columns are effectively
completed while others are not.

I'd like to not include set-returning functions from the list. Although
you can do "SELECT generate_series(1,10)", I'd like to discourage people
from doing that, since using set-returning functions in the target list
is a PostgreSQL extension to the SQL standard, and IMHO the "SELECT *
FROM generate_series(1,10)" syntax is easier to understand and works
more sanely with joins etc. Conversely, it would be really nice to
include set-returning function in the completions after FROM.

I understand that there isn't much you can do about some of those
things, short of adding a ton of new context information about previous
queries and heuristics. I think the completion needs to be smarter to be
acceptable. I don't know what exactly to do, but perhaps someone else
has ideas.

I'm also worried about performance, especially of the query to get all
the column names. It's pretty much guaranteed to do perform a
SeqScan+Sort+Unique on pg_attribute. pg_attribute can be very large. In
my little test database, with the above 1000-partition table, hitting
tab after "SELECT " takes about 1 second, until you get the "Display all
1000 possibilities" prompt. And that's not a particularly large schema.

- Heikki

PS. All the references to "pg_attribute" and other system tables, and
functions, need to be schema-qualified, as "pg_catalog.pg_attribute" and
so forth.

[1]
https://www.postgresql.org/message-id/1328820579.11241.4.camel@vanquo.pezone.net