Support autocomplete for tables in a specific schema #4

2022-05-27T10:59:25+02:00

vadimdemedes commented

2022-05-27 10:59:25 +02:00

(Migrated from github.com)

This PR adds support for specifying schema alongside a table, like so:

select * from my_schema.users where my_schema.users.id = 1

Currently, this package doesn't show any autocomplete suggestions when you specify a schema, even if you pass a config like this:

const config = {
  schema: {
    "my_schema.users": ["id", "email"]
  }
}

It also supports a default public schema. However, it's specified as a SQL keyword in this package, so this PR needs to specifically handle this case as an exception from other schema names. See schemaBefore function for details. I extracted the code to parse a schema into a function to avoid code duplication and to make it cleaner.

I also added the tests for various combinations for queries with and without quotes.

This PR adds support for specifying schema alongside a table, like so: ```sql select * from my_schema.users where my_schema.users.id = 1 ``` Currently, this package doesn't show any autocomplete suggestions when you specify a schema, even if you pass a config like this: ```js const config = { schema: { "my_schema.users": ["id", "email"] } } ``` It also supports a default `public` schema. However, it's specified as a SQL keyword in this package, so this PR needs to specifically handle this case as an exception from other schema names. See `schemaBefore` function for details. I extracted the code to parse a schema into a function to avoid code duplication and to make it cleaner. I also added the tests for various combinations for queries with and without quotes.

marijnh commented

2022-05-27 11:58:56 +02:00

(Migrated from github.com)

This looks good, on the whole. But if I add a test like this:

  it("completes table names under schema", () => {
    ist(str(get('select public.us|', {schema: schema2})), "public.users")
  })

It completes both other.users and public.users.

This looks good, on the whole. But if I add a test like this: ``` it("completes table names under schema", () => { ist(str(get('select public.us|', {schema: schema2})), "public.users") }) ``` It completes both `other.users` and `public.users`.

vadimdemedes commented

2022-05-27 12:41:00 +02:00

(Migrated from github.com)

Good catch, will work on this on Monday.

vadimdemedes commented

2022-05-30 12:52:14 +02:00

(Migrated from github.com)

Updated to support autocomplete of tables under a certain schema.

vadimdemedes (Migrated from github.com) reviewed

2022-05-30 12:54:06 +02:00

src/complete.ts

					
				@ -27,2 +48,2 @@

				      if (before && before.name == "Identifier" || before.name == "QuotedIdentifier")

				        parent = stripQuotes(state.sliceDoc(before.from, before.to))

				      if (before && isIdentifier(state, before)) {

				        let table = stripQuotes(state.sliceDoc(before.from, before.to))

I extracted this logic into isIdentifier function, so that the same sourceContext function supports autocomplete of tables under a schema without any modification to it. It was also repeated in 3 places, so I think it made sense to extract it either way.

I extracted this logic into `isIdentifier` function, so that the same `sourceContext` function supports autocomplete of tables under a schema without any modification to it. It was also repeated in 3 places, so I think it made sense to extract it either way.

vadimdemedes (Migrated from github.com) reviewed

2022-05-30 12:54:58 +02:00

src/complete.ts

This is used to create autocomplete for tables after user has typed a schema name, e.g. public.u| would return users completion item.

This is used to create autocomplete for tables after user has typed a schema name, e.g. `public.u|` would return `users` completion item.

vadimdemedes (Migrated from github.com) reviewed

2022-05-30 12:55:33 +02:00

src/complete.ts

Renamed columns to completions, because this variable can no longer hold columns from byTable, but also tables from bySchema.

Renamed `columns` to `completions`, because this variable can no longer hold columns from `byTable`, but also tables from `bySchema`.

marijnh commented

2022-05-30 14:31:30 +02:00

(Migrated from github.com)

I've merged this as 3bcac26b62 and largely rewritten it in 104322083b to make the nesting less ad-hoc and more reflected in the code structure (so that it also, for example, supports completing schema names).

I've merged this as 3bcac26b62d6bd78537 and largely rewritten it in 104322083bae6bbfa8045 to make the nesting less ad-hoc and more reflected in the code structure (so that it also, for example, supports completing schema names).

👍 1

Pull request closed

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

Sign in to join this conversation.

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!4

Rows
Columns