Add support for MSSQL bracket quoted identifiers

m-yst-ery commented

2025-09-16 07:48:00 +02:00

(Migrated from github.com)

This PR adds support for the MSSQL specifc style of quoting identifiers using square brackets.

i.e. in select [u].[Name] from [dbo].[Users] [u] the schema identifiers or aliases in brackets are now correctly recognized as QuotedIdentifiers. This then also makes the schema completion source work as normal with this syntax style.

The unit tests I added should reflect all relevant cases.

To bring the identifierQuotes property in line I decided to simply hard match the bracket pair.
So the identifierQuotes need to be explicitly set to '[' in order to correctly generate the completions wrapped in square brackets.
For me this seemed like the simplest and least intrusive solution, but I'm also open to other suggestions on how this should be implemented.

This PR adds support for the MSSQL specifc style of quoting identifiers using square brackets. i.e. in select [u].[Name] from [dbo].[Users] [u] the schema identifiers or aliases in brackets are now correctly recognized as QuotedIdentifiers. This then also makes the schema completion source work as normal with this syntax style. The unit tests I added should reflect all relevant cases. To bring the identifierQuotes property in line I decided to simply hard match the bracket pair. So the identifierQuotes need to be explicitly set to '[' in order to correctly generate the completions wrapped in square brackets. For me this seemed like the simplest and least intrusive solution, but I'm also open to other suggestions on how this should be implemented.

marijnh commented

2025-09-16 10:53:58 +02:00

(Migrated from github.com)

What the reason behind having a separate mssqlBracketQuotingMechanism field when it sounds like that could be derived from [ being present in identifierQuotes?

(Also, please put else on the line of the closing bracket for the if branch, for consistency with the rest of the code.)

What the reason behind having a separate `mssqlBracketQuotingMechanism` field when it sounds like that could be derived from `[` being present in `identifierQuotes`? (Also, please put `else` on the line of the closing bracket for the `if` branch, for consistency with the rest of the code.)

m-yst-ery commented

2025-09-16 12:35:41 +02:00

(Migrated from github.com)

I made them seperate to not unnecessarily restrict the configuration.
This way one could configure other quotes to be used in completion, but still benefit from bracket quoted identifiers being correctly highlighted and generally recognized as such.

I made them seperate to not unnecessarily restrict the configuration. This way one could configure other quotes to be used in completion, but still benefit from bracket quoted identifiers being correctly highlighted and generally recognized as such.

marijnh commented

2025-09-16 12:56:56 +02:00

(Migrated from github.com)

I cannot really think of a situation where you'd want these to be parsed but not handled by completion, or the other way around. But I see you're not adding [ to identifierQuotes in the definition of MSSQL. But then adding them back in in a test. Why is that? Am I missing something?

I cannot really think of a situation where you'd want these to be parsed but not handled by completion, or the other way around. But I see you're not adding `[` to `identifierQuotes` in the definition of `MSSQL`. But then adding them back in in a test. Why is that? Am I missing something?

m-yst-ery commented

2025-09-16 13:32:59 +02:00

(Migrated from github.com)

Ah, I got confused by how the completionSource only ever reads the first character and missed the actual usage in the parser.
So yes, I guess it could also be done like this:

// sql.ts
export const MSSQL = SQLDialect.define({
  ...
  identifierQuotes: '"[' // double-quote first so the current completion behavior is retained
})

// tokens.ts
...
if (inString(Ch.BracketL, d.identifierQuotes)) {
    readLiteral(input, Ch.BracketR, false)
    input.acceptToken(QuotedIdentifier)
}
...

Then if you want you could redefine the dialect to put the bracket in the first spot so the completion source uses them as default when quoting completions.
Or maybe I'm missing something else and theres actually a different way to do this?

const dialect = SQLDialect.define({ ...MSSQL.spec, identifierQuotes: '["' })

The parser would still need to handle this case before it accepts the bracket as just a bracket.
So either add this into the block for BracketL or move the QuotedIdentifier block up and add it there. Not sure if the second option would break something though.

Ah, I got confused by how the completionSource only ever reads the first character and missed the actual usage in the parser. So yes, I guess it could also be done like this: ```ts // sql.ts export const MSSQL = SQLDialect.define({ ... identifierQuotes: '"[' // double-quote first so the current completion behavior is retained }) ``` ```ts // tokens.ts ... if (inString(Ch.BracketL, d.identifierQuotes)) { readLiteral(input, Ch.BracketR, false) input.acceptToken(QuotedIdentifier) } ... ``` Then if you want you could redefine the dialect to put the bracket in the first spot so the completion source uses them as default when quoting completions. Or maybe I'm missing something else and theres actually a different way to do this? ```ts const dialect = SQLDialect.define({ ...MSSQL.spec, identifierQuotes: '["' }) ``` The parser would still need to handle this case before it accepts the bracket as just a bracket. So either add this into the block for BracketL or move the QuotedIdentifier block up and add it there. Not sure if the second option would break something though.

marijnh commented

2025-09-16 13:49:47 +02:00

(Migrated from github.com)

Then if you want you could redefine the dialect to put the bracket in the first spot so the completion source uses them as default when quoting completions.

Yes, that's what I was thinking as well.

So either add this into the block for BracketL or move the QuotedIdentifier block up and add it there.

The latter sounds most reasonable.

> Then if you want you could redefine the dialect to put the bracket in the first spot so the completion source uses them as default when quoting completions. Yes, that's what I was thinking as well. > So either add this into the block for BracketL or move the QuotedIdentifier block up and add it there. The latter sounds most reasonable.

m-yst-ery commented

2025-09-16 14:19:02 +02:00

(Migrated from github.com)

Alright, I have added this adjustment

marijnh commented

2025-09-16 15:15:43 +02:00

(Migrated from github.com)

Looks mostly good now. Could you remove the meaningless changes that your IDE or linter made (just adding a comma) to reduce patch noise?

marijnh commented

2025-09-16 15:35:06 +02:00

(Migrated from github.com)

Thanks, merged! I noticed some random semicolons, which I removed myself in 3dc83af

Thanks, merged! I noticed some random semicolons, which I removed myself in 3dc83af

marijnh commented

2025-09-16 15:35:53 +02:00

(Migrated from github.com)

(Let me know whether you're planning any other patches. If not I'll cut a release.)

m-yst-ery commented

2025-09-16 15:40:45 +02:00

(Migrated from github.com)

Great, and thank you!

No, I don't plan on anything else. So a release would be appreciated.

Great, and thank you! No, I don't plan on anything else. So a release would be appreciated.

marijnh commented

2025-09-16 15:49:05 +02:00

(Migrated from github.com)

Tagged 6.10.0

Rows
Columns

Add support for MSSQL bracket quoted identifiers #20