Repository: DerekStride/tree-sitter-sql (language: python)

Issue:
T-SQL Alter Table
TSQL:
```sql
ALTER TABLE Class_Session
ADD
type varchar(255) NOT NULL DEFAULT ('online');
```

Error:
```
Expected [A-Za-z0-9_:] or [A-Za-z0-9_] but " " found. (sql)
```

Expected:
No error, this is valid sql

<img width="669" alt="image" src="https://user-images.githubusercontent.com/664434/230105485-13133413-832b-4474-a095-cc799a7dc8a0.png">

WIP: add multiple alter statements add once (t-sql)
Closes #135

This is an attempt to allow for multiple `alter table` `add` statements, as it is supported by T-SQL.

This does not work at the moment. I need a right precedence on the repeated section for at the `add_column` node, but this breaks the `_alter_specification` one hierarchy level up. I have currently not solution for this problem. @DerekStride, do you have an idea? The issue is that there can be multiple `_alter_specification` that are separated by commas, same as the `_inner_add_column` node that I have added.

Reference:
https://learn.microsoft.com/de-de/sql/t-sql/statements/alter-table-transact-sql?view=sql-server-ver16
WIP: add multiple alter statements add once (t-sql)
Closes #135

This is an attempt to allow for multiple `alter table` `add` statements, as it is supported by T-SQL.

This does not work at the moment. I need a right precedence on the repeated section for at the `add_column` node, but this breaks the `_alter_specification` one hierarchy level up. I have currently not solution for this problem. @DerekStride, do you have an idea? The issue is that there can be multiple `_alter_specification` that are separated by commas, same as the `_inner_add_column` node that I have added.

Reference:
https://learn.microsoft.com/de-de/sql/t-sql/statements/alter-table-transact-sql?view=sql-server-ver16


Discussion / hints:
The SQL statement works fine for my:
![image](https://user-images.githubusercontent.com/35303817/230116418-0939ba0b-c6ef-4526-9909-5447da0e89c2.png)

Can you update the grammar on your machine?
Hmm. I am using the latest parser from `nvim-treesitter`. I believe it is https://github.com/nvim-treesitter/nvim-treesitter/commit/411e771d511442ab81670701f5c830f6c74aadd7. I'm not too familiar with how to update grammar directly. Can you clarify how I should do that?
I apologize the error appears to be coming from https://github.com/fannheyward/coc-sql and not tree-sitter-sql 
OK, no problem. Upon further investigation, T-SQL seems to support to add multiple columns in one alter table statement (like in your screenshot).
This parse aims to be permissive and we mostly consider MySQL, MariaDB, PostgreSQL and a some Hive statements. These dialect do not support this style of altering tables.

If you want you can reopen it. I definately get an error for statements like:
```
alter table classname 
add
col1 varchar(255) not null default('empty'),
col2 varchar(255) not null default('empty2');
```

The SQL in nvim-treesitter gets updated when you update the plugin. `:TSUpdate` should also work.




Ok in that case we should probably reopen this one as well since there appear to be two separate issues in the same statement. The issue I originally reported was fixed in https://github.com/taozhi8833998/node-sql-parser/issues/1372. Thanks for your help! Happy to test out a potential solution.
t-sql also allows for adding a constraint without saying the word `constraint`, so maybe the thing to do here is to have a general `add` rule (and possibly the other verbs) that offers a choice between a seq of column definitions or a seq of constraint definitions?
Thanks, @dmfay for the suggestion. I will try this later today
@dmfay I have issues fixing the test alter.txt L428
```
ALTER TABLE IF EXISTS my_table
  ADD COLUMN IF NOT EXISTS val4 DATE,
  ALTER COLUMN val5 DROP NOT NULL, -- comment, ignore me!
  RENAME COLUMN val6 TO val7,
  DROP COLUMN IF EXISTS val8;
```

I have checked this statement in Postgres and Mysql and in both dialects it is not valid. The `rename` is the issue. Is this a bug in our implementation, or is is valid in another dialect? 

This is very confusing. When just running the alter table with only the renaming part, if works fine.
> @dmfay I have issues fixing the test alter.txt L428
>
> ```
> ALTER TABLE IF EXISTS my_table
>   ADD COLUMN IF NOT EXISTS val4 DATE,
>   ALTER COLUMN val5 DROP NOT NULL, -- comment, ignore me!
>   RENAME COLUMN val6 TO val7,
>   DROP COLUMN IF EXISTS val8;
> ```
>
> I have checked this statement in Postgres and Mysql and in both dialects it is not valid. The `rename` is the issue. Is this a bug in our implementation, or is is valid in another dialect?
>
> This is very confusing. When just running the alter table with only the renaming part, if works fine.

It looks like that was added [here](https://github.com/DerekStride/tree-sitter-sql/commit/be20ed053a6d0a6125e485f4796a488f91df987d) along with enabling more combinations of actions under `alter table` for Postgres, but you're right, Postgres' `alter table` allows column renames only as a single action. I don't know if any other dialects do allow it as one of multiple actions.
t-sql also allows for adding a constraint without saying the word `constraint`, so maybe the thing to do here is to have a general `add` rule (and possibly the other verbs) that offers a choice between a seq of column definitions or a seq of constraint definitions?
Thanks, @dmfay for the suggestion. I will try this later today
@dmfay I have issues fixing the test alter.txt L428
```
ALTER TABLE IF EXISTS my_table
  ADD COLUMN IF NOT EXISTS val4 DATE,
  ALTER COLUMN val5 DROP NOT NULL, -- comment, ignore me!
  RENAME COLUMN val6 TO val7,
  DROP COLUMN IF EXISTS val8;
```

I have checked this statement in Postgres and Mysql and in both dialects it is not valid. The `rename` is the issue. Is this a bug in our implementation, or is is valid in another dialect? 

This is very confusing. When just running the alter table with only the renaming part, if works fine.
> @dmfay I have issues fixing the test alter.txt L428
>
> ```
> ALTER TABLE IF EXISTS my_table
>   ADD COLUMN IF NOT EXISTS val4 DATE,
>   ALTER COLUMN val5 DROP NOT NULL, -- comment, ignore me!
>   RENAME COLUMN val6 TO val7,
>   DROP COLUMN IF EXISTS val8;
> ```
>
> I have checked this statement in Postgres and Mysql and in both dialects it is not valid. The `rename` is the issue. Is this a bug in our implementation, or is is valid in another dialect?
>
> This is very confusing. When just running the alter table with only the renaming part, if works fine.

It looks like that was added [here](https://github.com/DerekStride/tree-sitter-sql/commit/be20ed053a6d0a6125e485f4796a488f91df987d) along with enabling more combinations of actions under `alter table` for Postgres, but you're right, Postgres' `alter table` allows column renames only as a single action. I don't know if any other dialects do allow it as one of multiple actions.

Fix patch:
diff --git a/grammar.js b/grammar.js
index 9edac624..d4a347e4 100644
--- a/grammar.js
+++ b/grammar.js
@@ -1131,8 +1131,10 @@ module.exports = grammar({
       $.change_ownership,
     ),
 
+    // TODO: optional `keyword_add` is necessary to allow for chained alter statements in t-sql
+    // maybe needs refactoring
     add_column: $ => seq(
-      $.keyword_add,
+      optional($.keyword_add),
       optional(
         $.keyword_column,
       ),
@@ -1143,7 +1145,7 @@ module.exports = grammar({
 
     add_constraint: $ => seq(
       $.keyword_add,
-      $.keyword_constraint,
+      optional($.keyword_constraint),
       $.identifier,
       $.constraint,
     ),


Test patch:
diff --git a/test/corpus/alter.txt b/test/corpus/alter.txt
index 0ea97e3f..45d9ec9e 100644
--- a/test/corpus/alter.txt
+++ b/test/corpus/alter.txt
@@ -425,7 +425,6 @@ Alter table and multiple
 ALTER TABLE IF EXISTS my_table
   ADD COLUMN IF NOT EXISTS val4 DATE,
   ALTER COLUMN val5 DROP NOT NULL, -- comment, ignore me!
-  RENAME COLUMN val6 TO val7,
   DROP COLUMN IF EXISTS val8;
 
 --------------------------------------------------------------------------------
@@ -456,12 +455,6 @@ ALTER TABLE IF EXISTS my_table
         (keyword_not)
         (keyword_null))
       (comment)
-      (rename_column
-        (keyword_rename)
-        (keyword_column)
-        old_name: (identifier)
-        (keyword_to)
-        new_name: (identifier))
       (drop_column
         (keyword_drop)
         (keyword_column)
@@ -626,3 +619,45 @@ new_table TO old_table;
     (keyword_to)
     (object_reference
       (identifier))))
+
+================================================================================
+T-SQL: alter table add multiple columns at once
+================================================================================
+
+ALTER TABLE tab
+ADD
+col1 VARCHAR(255) NOT NULL DEFAULT('EMPTY'),
+col2 VARCHAR(255) NOT NULL DEFAULT('EMPTY');
+
+--------------------------------------------------------------------------------
+
+(program
+  (statement
+    (alter_table
+      (keyword_alter)
+      (keyword_table)
+      (object_reference
+        (identifier))
+      (add_column
+        (keyword_add)
+        (column_definition
+          (identifier)
+          (varchar
+            (keyword_varchar)
+            (literal))
+          (keyword_not)
+          (keyword_null)
+          (keyword_default)
+          (list
+            (literal))))
+      (add_column
+        (column_definition
+          (identifier)
+          (varchar
+            (keyword_varchar)
+            (literal))
+          (keyword_not)
+          (keyword_null)
+          (keyword_default)
+          (list
+            (literal)))))))