Re: tab-complete COMPLETE_WITH_ATTR can become confused by table-lists.

Peter Smith <smithpb2250(at)gmail(dot)com> writes:
> If there are spaces in the table-list like "t1, t2" then the word is
> recognized as "t2" and it works as expected.
> But, if there are no spaces in the table-list like "t1,t2" then the
> word is recognized as "t1,t2", and since that is no such table name
> the COMPLETE_WITH_ATTR does nothing.

Hmm, another variant is

=# create table foobar(z int);
CREATE TABLE
=# analyze foo<TAB> --> completes "foobar"
=# analyze foobar,foo<TAB> --> nothing

> I found that this is easily fixed just by adding a comma to the
> WORD_BREAKS. Then words all get tokenized properly and so 'prev2_wd'
> is what you'd like it to be.
> /* word break characters */
> -#define WORD_BREAKS "\t\n(at)$><=;|&{() "
> +#define WORD_BREAKS "\t\n,@$><=;|&{() "

Nice catch. Now that I look at it, that WORD_BREAKS list seems quite
random -- for example, why "{" but not "}", and why neither of "["
or "]"? If we have "><=", why not "+-*/", to say nothing of other
operator characters?

I can see reasons for not listing ' " or :, because those are handled
elsewhere. But I'm not sure about the other omissions.

Experimenting a bit, I see that

=# create table "amp&sand" (f int);
CREATE TABLE
=# insert into "amp<TAB> --> completes "amp&sand"
=# insert into "amp&<TAB> --> nothing

So populating WORD_BREAKS more fully would tend to break completion
of names using the added characters. But probably the answer for
that is to have less ad-hoc handling of quoted names. (See also
my screed at [1].) Anyway, optimizing for weird quoted names is
probably not what we want to do here.

I feel like we should populate WORD_BREAKS more fully and document
the reasons for omissions, rather than leave future hackers to
guess about it.

> OTOH, this seemed a pretty fundamental change to the 12-year-old (!!)
> code so I don't know if it may be too risky and/or could adversely
> affect something else?

It's a bit scary, and I wouldn't consider back-patching it, but
TBH tab-complete.c is all chewing gum and baling wire anyway.
What I'd *really* like to do is nuke the whole thing and build
something that's mechanically derived from the actual backend
grammar. But I don't know how to make that happen. In the
meantime, incrementally moving it towards the real SQL parsing
rules seems like it ought to be an improvement.

> The tests are all still passing, but there aren't so many tab-complete
> tests anyway so that might not mean much.

Yeah, we certainly haven't got coverage for these sorts of details.

regards, tom lane

[1] https://www.postgresql.org/message-id/3547066.1642272686%40sss.pgh.pa.us