Peter Smith <smithpb2250@gmail.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@$><=;|&{() "
> +#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