Support autocompletion with table aliases #7

Closed

neon-dev wants to merge 5 commits from patch-1 into main

neon-dev commented

2022-08-19 23:42:09 +02:00

(Migrated from github.com)

Table names are only searched for between FROM and WHERE (if present) and only on the same level as the alias usage, so false positives should not appear.
It might not be implemented in the most performant way since I'm not familiar with token traversal in CM 6, but I hope it's not too bad. I only tested small queries, which are fine.

Table names are only searched for between FROM and WHERE (if present) and only on the same level as the alias usage, so false positives should not appear. It might not be implemented in the most performant way since I'm not familiar with token traversal in CM 6, but I hope it's not too bad. I only tested small queries, which are fine.

marijnh commented

2022-08-20 17:58:31 +02:00

(Migrated from github.com)

Could you add some tests so I can see what kind of situations this is targeting?

neon-dev commented

2022-08-20 18:28:29 +02:00

(Migrated from github.com)

Oh, unfortunately I don't know how to write tests for this. What it should do is provide completions for an aliased schema:

{ "table": [ "column" ] }

Some basic tests:

SELECT * FROM table AS t WHERE t.<suggest column>
SELECT * FROM table t WHERE t.<suggest column>
SELECT * FROM `table` t WHERE t.<suggest column>
SELECT * FROM table t WHERE t.c<suggest column>
SELECT * FROM table t WHERE t.ca<don't suggest column>
SELECT t.<suggest column> FROM table t
SELECT t.<suggest column> FROM table AS t
SELECT t.<suggest column> FROM something AS s JOIN table t
SELECT s.<don't suggest column> FROM something AS s JOIN table t
SELECT t.<suggest column> FROM something s, table t

Same goes for a schema like { "db.table": [ "column" ] } and when parts or all identifiers are quoted in SQL.

What's currently not included in this PR is autocompleting aliases themselves, since it would be much more involved and would have to be executed on any completion, even emtpy ones. This would be cool, but need much more testing and maybe a larger rewrite of existing functions for better (and more performant) integration.

Oh, unfortunately I don't know how to write tests for this. What it should do is provide completions for an aliased schema: ```js { "table": [ "column" ] } ``` Some basic tests: ```sql SELECT * FROM table AS t WHERE t.<suggest column> SELECT * FROM table t WHERE t.<suggest column> SELECT * FROM `table` t WHERE t.<suggest column> SELECT * FROM table t WHERE t.c<suggest column> SELECT * FROM table t WHERE t.ca<don't suggest column> SELECT t.<suggest column> FROM table t SELECT t.<suggest column> FROM table AS t SELECT t.<suggest column> FROM something AS s JOIN table t SELECT s.<don't suggest column> FROM something AS s JOIN table t SELECT t.<suggest column> FROM something s, table t ``` Same goes for a schema like `{ "db.table": [ "column" ] }` and when parts or all identifiers are quoted in SQL. What's currently not included in this PR is autocompleting aliases themselves, since it would be much more involved and would have to be executed on any completion, even emtpy ones. This would be cool, but need much more testing and maybe a larger rewrite of existing functions for better (and more performant) integration.

marijnh commented

2022-08-20 18:34:09 +02:00

(Migrated from github.com)

See test/test-complete.ts for examples of autocompletion tests.

See `test/test-complete.ts` for examples of autocompletion tests.

neon-dev commented

2022-08-20 18:39:31 +02:00

(Migrated from github.com)

I did already, but still don't fully understand the syntax.
Is | the cursor position and what's the difference between the last arguments ("email, id" / '"email", "id"') in
github.com/codemirror/lang-sql@905c81a436/test/test-complete.ts (L71)
and
github.com/codemirror/lang-sql@905c81a436/test/test-complete.ts (L76)
?

I did already, but still don't fully understand the syntax. Is `|` the cursor position and what's the difference between the last arguments (`"email, id"` / `'"email", "id"'`) in https://github.com/codemirror/lang-sql/blob/905c81a436a84d2a1063ba391c196eaeba239233/test/test-complete.ts#L71 and https://github.com/codemirror/lang-sql/blob/905c81a436a84d2a1063ba391c196eaeba239233/test/test-complete.ts#L76 ?

marijnh commented

2022-08-20 18:58:31 +02:00

(Migrated from github.com)

Is | the cursor position

Yes.

what's the difference between the last arguments ("email, id" / '"email", "id"')

One has quotes in the completion options, one doesn't.

> Is | the cursor position Yes. > what's the difference between the last arguments ("email, id" / '"email", "id"') One has quotes in the completion options, one doesn't.

neon-dev commented

2022-08-20 19:05:27 +02:00

(Migrated from github.com)

Thanks. My bad, I just misread something when looking at the test file and only noticed right after posting my comment.
Added some tests but don't know how to write tests for false positives, but maybe they are not that important?
Also I did not include tests for completing quoted columns since the alias handling only affects schema and table names.

Thanks. My bad, I just misread something when looking at the test file and only noticed right after posting my comment. Added some tests but don't know how to write tests for false positives, but maybe they are not that important? Also I did not include tests for completing quoted columns since the alias handling only affects schema and table names.

marijnh (Migrated from github.com) reviewed

2022-08-20 19:19:53 +02:00

src/complete.ts

Names with periods in them will be tokenized as separate tokens for the identifiers and the periods, so this doesn't look like it'll work.

marijnh (Migrated from github.com) reviewed

2022-08-20 19:20:26 +02:00

src/complete.ts

We're targeting browsers that don't have endsWith—could you change this to /Identifier$/.test?

We're targeting browsers that don't have `endsWith`—could you change this to `/Identifier$/.test`?

neon-dev (Migrated from github.com) reviewed

2022-08-20 19:30:55 +02:00

src/complete.ts

Are names with periods in them valid?
If so can you recommend a nice way to split or tokenize this? I would like to avoid an ugly regex with lookaheads because those are hard to read and maintain.

Are names with periods in them valid? If so can you recommend a nice way to split or tokenize this? I would like to avoid an ugly regex with lookaheads because those are hard to read and maintain.

neon-dev (Migrated from github.com) reviewed

2022-08-20 19:47:40 +02:00

src/complete.ts

Sure

neon-dev (Migrated from github.com) reviewed

2022-08-22 21:38:24 +02:00

src/complete.ts

I noticed that a CompositeIdentifier is already tokenized (as children of the node), so there's no need to split. Pushed new code where I addressed this issue and also refactored the code a bit.

I noticed that a `CompositeIdentifier` is already tokenized (as children of the node), so there's no need to split. Pushed new code where I addressed this issue and also refactored the code a bit.

marijnh commented

2022-08-23 10:27:53 +02:00

(Migrated from github.com)

I ended up going with a different approach (see ca17998) that is a bit less ad-hoc (yours had a few potential null-dereferences and didn't work with some query formats) and allows completing the names of aliases. Does that patch work for you?

I ended up going with a different approach (see ca17998) that is a bit less ad-hoc (yours had a few potential null-dereferences and didn't work with some query formats) and allows completing the names of aliases. Does that patch work for you?

❤️ 1

neon-dev commented

2022-08-23 13:53:33 +02:00

(Migrated from github.com)

Works great for me, thank you.
Now that it autocompletes aliases you could make it less strict to also include names of aliases for unknown entities (like a subselect).

Works great for me, thank you. Now that it autocompletes aliases you could make it less strict to also include names of aliases for unknown entities (like a subselect).

neon-dev commented

2022-08-23 14:20:21 +02:00

(Migrated from github.com)

Maybe this idea sounds also interesting to you, otherwise sorry for going off-topic here:
github.com/codemirror/lang-sql@ca17998ec7/src/complete.ts (L64)
Additionally search prevSiblings for a USE statement to set/override the default schema. It's nothing I need but looks like a low hanging fruit and an intuitive feature.

Maybe this idea sounds also interesting to you, otherwise sorry for going off-topic here: https://github.com/codemirror/lang-sql/blob/ca17998ec7cfd431cf94b3b61e09b53f8c87771c/src/complete.ts#L64 Additionally search prevSiblings for a `USE` statement to set/override the default schema. It's nothing I need but looks like a low hanging fruit and an intuitive feature.

marijnh commented

2022-08-23 15:18:38 +02:00

(Migrated from github.com)

Tagged as 6.3.0. I don't currently have time to add more features to this.

👍 1

Pull request closed

This pull request cannot be reopened because the branch was deleted.

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

codemirror/lang-sql!7

No description provided.

Rows
Columns