Tsql grammar fixes by JPMarichal · Pull Request #356 · DerekStride/tree-sitter-sql

JPMarichal · 2026-02-12T12:13:36Z

No description provided.

…o support optional semicolons and the `GO` keyword.

@param

- Resolve grammar conflicts for statement vs _tsql_function_body_statement - Enable CREATE PROCEDURE parsing with T-SQL parameter support (@param) - Add SET NOCOUNT ON/OFF support - Fix DECLARE TABLE with T-SQL variables (@tablevar) - Allow INSERT into table variables (@user) - Fix UPDATE...FROM with table variables and JOIN precedence - Add relation to accept $.parameter for T-SQL table vars - Create queries/structure.scm for code navigation (procedures, tables, variables)

Copilot

Pull request overview

This PR updates the SQL tree-sitter grammar to improve T-SQL support (procedures, identifiers/parameters, and additional statements) and adds new corpus/queries to validate and enable code navigation.

Changes:

Add/adjust T-SQL grammar support (procedures, GO, SET NOCOUNT, variables/parameters, bracket identifiers, statement forms).
Add a new procedure corpus (test/corpus/procedures.txt) and a structure query file for code navigation.
Refactor/remove prior transaction parsing and tweak several statement grammars (e.g., SET, SELECT, TRUNCATE, DROP).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
test_tsql_select.sql	Adds a basic T-SQL select sample using bracket identifiers and `@` parameters.
test_simple.sql	Adds a procedure sample, currently as a quoted string literal.
test_semi.sql	Adds a procedure sample with semicolon, currently as a quoted string literal.
test_ref.sql	Adds a reference sample, currently as a quoted string literal.
test_proc_final.sql	Adds a T-SQL procedure sample, currently as a quoted string literal.
test/corpus/procedures.txt	New corpus coverage for procedure parsing across dialects (Postgres + T-SQL).
queries/structure.scm	Adds structure queries for procedures/functions/tables/table variables.
grammar/transactions.js	Removes the `transaction` rule set (now empty).
grammar/statements/truncate.js	Requires at least one truncate target.
grammar/statements/set.js	Expands T-SQL-related `SET` support (e.g., `NOCOUNT`) and adjusts assignment parsing.
grammar/statements/select.js	Updates term parsing (including T-SQL `alias = expr`) and join associativity tweaks.
grammar/statements/insert.js	Allows insert target to be an object reference or parameter.
grammar/statements/index.js	Adds multiple T-SQL statement forms (IF/RETURN/PRINT/RAISERROR/EXEC/GOTO/labels/transaction statements) and semicolon flexibility.
grammar/statements/drop.js	Adds `DROP PROCEDURE/PROC` support.
grammar/statements/create.js	Wires in procedure rules but also adds a separate local T-SQL `create_procedure` implementation.
grammar/statements/create-procedure.js	New procedure grammar supporting multiple procedure body styles including T-SQL.
grammar/statements/create-function.js	Adjusts precedence to reduce conflicts with T-SQL function bodies/returns.
grammar/statements/comment.js	Adds `COMMENT ON PROCEDURE ...` target support.
grammar/keywords.js	Adds T-SQL keywords (e.g., `proc`, `go`, `nocount`, `print`, `raiserror`, `goto`, `exec`).
grammar/expressions.js	Expands object references for T-SQL multi-part names, adds bracket identifiers and `@` parameters.
grammar.js	Updates top-level `program` rule (adds `GO`, optional semicolons) and conflict declarations.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T12:22:20Z

grammar/expressions.js

-    ),
-  ),
+  _tsql_bracket_identifier: _ => /\[[^\]]*\]/,
+  _tsql_parameter: _ => /@[A-Za-z_0-9]*/,


_tsql_parameter currently matches @ by itself (because of *) and also allows names starting with digits. That will accept invalid T-SQL and can create ambiguous parses. Tighten the regex to require at least one identifier character after @ and (ideally) enforce a valid starting character, e.g. @[A-Za-z_][A-Za-z_0-9]*.

Suggested change

_tsql_parameter: _ => /@[A-Za-z_0-9]*/,

_tsql_parameter: _ => /@[_A-Za-z\u00C0-\u017F][0-9A-Za-z_\u00C0-\u017F]*/,

Copilot · 2026-02-12T12:22:21Z

test_ref.sql

@@ -0,0 +1 @@
+"SELECT * FROM TestProc"


This .sql fixture contains a quoted string literal rather than SQL text. If it’s intended to be parsed as SQL, drop the surrounding quotes; otherwise, consider relocating/documenting the fixture to avoid confusion.

Suggested change

"SELECT * FROM TestProc"

SELECT * FROM TestProc;

Copilot · 2026-02-12T12:22:21Z

test_proc_final.sql

@@ -0,0 +1 @@
+"CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;"


This .sql fixture contains a quoted string literal rather than SQL text. If it’s intended to exercise the SQL grammar directly, remove the surrounding quotes; otherwise, consider relocating/documenting the fixture.

Suggested change

"CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;"

CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;

Copilot · 2026-02-12T12:22:21Z

queries/structure.scm

+(create_procedure
+  (object_reference) @name) @definition.procedure
+
+; CREATE FUNCTION definitions  


There are trailing spaces at the end of this comment line. Please trim them to avoid unnecessary diffs and keep formatting consistent.

Suggested change

; CREATE FUNCTION definitions

; CREATE FUNCTION definitions

Copilot · 2026-02-12T12:22:22Z

grammar/statements/create.js

      $.create_extension,
      $.create_trigger,
      $.create_policy,
+      $.create_procedure,


_create_statement lists $.create_procedure twice, which is redundant and can mask issues when adjusting create parsing. Remove the duplicate entry so each create variant appears once.

Suggested change

$.create_procedure,

that is correct, remove one entry of create_procedure

Copilot · 2026-02-12T12:22:24Z

grammar.js

+      seq(
+        choice(
+          $.statement,
+          $.block,


program no longer includes the transaction rule (it now only allows statement, block, or GO). Since the repository still has test/corpus/transaction.txt asserting a (transaction ...) node, this change will break existing corpus tests unless the transaction corpus (and any dependent queries) are updated accordingly or the transaction rule is reintroduced.

Suggested change

$.block,

$.block,

$.transaction,

Copilot · 2026-02-12T12:22:24Z

grammar/statements/set.js

        choice(
          seq(
-            $.object_reference,
+            choice($.identifier, $.parameter),


set_statement previously used $.object_reference for the setting name, which matches the existing corpus expectations in test/corpus/set.txt. Switching to choice($.identifier, $.parameter) changes the parse tree shape (it will no longer produce an object_reference node) and will break those tests. If you need to support T-SQL variables here, consider using choice($.object_reference, $.parameter) so existing object_reference captures remain stable.

Suggested change

choice($.identifier, $.parameter),

choice($.object_reference, $.parameter),

Copilot · 2026-02-12T12:22:24Z

grammar/keywords.js

+  keyword_raiserror: _ => make_keyword("raiserror"),
+  keyword_goto: _ => make_keyword("goto"),
+  keyword_exec: _ => make_keyword("exec"),
+  keyword_out: _ => make_keyword("out"),


keyword_out is defined twice in this file (once earlier and again here). Even though the definitions are identical, the duplicate key is easy to miss and can cause confusion during future edits; remove one of them to keep the keyword list canonical.

Suggested change

keyword_out: _ => make_keyword("out"),

Copilot · 2026-02-12T12:22:24Z

grammar/expressions.js

      $.array,
      $.interval,
      $.between_expression,
      $.parenthesized_expression,
-      $.object_id,
    )


The _expression choice list no longer includes an object_id expression, but the repo’s corpus still contains a "TSQL OBJECT_ID" test (test/corpus/expressions.txt) that expects an (object_id ...) node. Restoring object_id parsing (and keyword_object_id) or updating the corpus is needed to keep CI passing.

Copilot · 2026-02-12T12:22:25Z

grammar/keywords.js

  keyword_security: _ => make_keyword("security"),
  keyword_version: _ => make_keyword("version"),
-  keyword_extension: _ => make_keyword("extension"),
  keyword_out: _ => make_keyword("out"),


This property is duplicated in a later property.

Suggested change

keyword_out: _ => make_keyword("out"),

remobe duplicate

matthias-Q

Sorry, but this PR mixes so many different changes. There are cross cutting changes that affect other parts, added unrelated files, formatting changes in places where no change is required.

Please go over the PR again and clean it up. Your copilot review also provided many helpful hints.

matthias-Q · 2026-02-12T18:38:11Z

test_proc_final.sql

@@ -0,0 +1 @@
+"CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;"


why are these SQL files part of the PR?

matthias-Q · 2026-02-12T18:38:17Z

test_ref.sql

@@ -0,0 +1 @@
+"SELECT * FROM TestProc"


why are these SQL files part of the PR?

matthias-Q · 2026-02-12T18:38:23Z

test_semi.sql

@@ -0,0 +1 @@
+"CREATE PROCEDURE TestProc AS SELECT 1;"


why are these SQL files part of the PR?

matthias-Q · 2026-02-12T18:38:30Z

test_simple.sql

@@ -0,0 +1 @@
+"CREATE PROCEDURE TestProc AS SELECT 1"


why are these SQL files part of the PR?

matthias-Q · 2026-02-12T18:40:54Z

grammar/statements/create.js

      $.create_extension,
      $.create_trigger,
      $.create_policy,
+      $.create_procedure,


that is correct, remove one entry of create_procedure

matthias-Q · 2026-02-12T18:44:22Z

grammar/statements/create.js


+  create_procedure: $ => seq(
+    $.keyword_create,
+    optional($._or_replace),
+    choice($.keyword_procedure, $.keyword_proc),
+    $.object_reference,
+    optional(
+      repeat(
+        seq(
+          optional(','),
+          $.procedure_argument
+        )
+      )
+    ),
+    optional($.keyword_as),
+    choice(
+      $.statement,
+      $.block
+    )
+  ),
+
+  procedure_argument: $ => seq(
+    field('name', $.parameter),
+    $._type,
+    optional(seq('=', choice($.literal, $.keyword_null))),
+    optional(choice($.keyword_out, $.keyword_output))
+  ),


This is correct, create_procedure is defined twice

matthias-Q · 2026-02-12T18:45:47Z

grammar/statements/index.js

    repeat(
      seq(
        $.statement,
-        ';'


@DerekStride what do you think? should the ; be optional in general? It is a breaking change

I think it's pretty load-bearing for other dialects, and optionality is deprecated even in SQL Server. An optional semicolon after the last statement of the seq might be nice.

matthias-Q · 2026-02-12T18:50:24Z

grammar/keywords.js

  keyword_security: _ => make_keyword("security"),
  keyword_version: _ => make_keyword("version"),
-  keyword_extension: _ => make_keyword("extension"),
  keyword_out: _ => make_keyword("out"),


remobe duplicate

matthias-Q · 2026-02-12T18:51:06Z

queries/structure.scm

@@ -0,0 +1,19 @@
+; Structure queries for T-SQL code navigation


Whats the purpose of having an extra file with "structure" queries?

dmfay · 2026-02-12T22:49:12Z

Sorry, but this PR mixes so many different changes. There are cross cutting changes that affect other parts, added unrelated files, formatting changes in places where no change is required.

Please go over the PR again and clean it up. Your copilot review also provided many helpful hints.

I don't think this is salvageable as-is; it'll be all but impossible to keep discussion straight on dozens of material changes in one massive block. @JPMarichal please factor your changes out into individual, more focused pull requests.

JPMarichal1 added 3 commits February 11, 2026 07:31

feat: Implement core DDL/DML statements and enhance program parsing t…

12f9a6d

…o support optional semicolons and the `GO` keyword.

Merge remote main with T-SQL changes - resolve drop.js conflict

0d007bc

Copilot AI review requested due to automatic review settings February 12, 2026 12:13

Copilot started reviewing on behalf of JPMarichal February 12, 2026 12:14 View session

Merge branch 'main' into tsql-grammar-fixes

df87b94

Copilot AI reviewed Feb 12, 2026

View reviewed changes

matthias-Q requested changes Feb 12, 2026

View reviewed changes

matthias-Q requested a review from dmfay February 12, 2026 18:55

dmfay closed this Feb 12, 2026

	_tsql_parameter: _ => /@[A-Za-z_0-9]*/,
	_tsql_parameter: _ => /@[_A-Za-z\u00C0-\u017F][0-9A-Za-z_\u00C0-\u017F]*/,

		@@ -0,0 +1 @@
		"CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;"

	"CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;"
	CREATE PROCEDURE [dbo].[TestProc] @ID INT AS BEGIN SELECT 1 END;

	choice($.identifier, $.parameter),
	choice($.object_reference, $.parameter),

		@@ -0,0 +1,19 @@
		; Structure queries for T-SQL code navigation

Conversation

JPMarichal commented Feb 12, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthias-Q left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmfay commented Feb 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants