Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added Support for SQL Server INSERT INTO Parsing #29230

Closed
wants to merge 5 commits into from
Closed

Added Support for SQL Server INSERT INTO Parsing #29230

wants to merge 5 commits into from

Conversation

BykaWF
Copy link

@BykaWF BykaWF commented Nov 28, 2023 •
edited
Loading

Fixes #29194.

Changes proposed in this pull request:
It adds support for parsing the following SQL Server INSERT INTO statements:

INSERT INTO #MyTempTable VALUES (1)
INSERT INTO #t VALUES (2)
SELECT Test2Col = x FROM #t
INSERT INTO #t VALUES (1)
SELECT Test1Col = x FROM #t

Before committing this PR, I'm sure that I have checked the following options:

  • My code follows the code of conduct of this project.
  • I have self-reviewed the commit code.
  • I have (or in comment I request) added corresponding labels for the pull request.
  • I have passed maven check locally : ./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.
  • I have made corresponding changes to the documentation.
  • I have added corresponding unit tests for my changes.

@strongduanmu strongduanmu added this to the 5.4.2 milestone Nov 29, 2023
strongduanmu
strongduanmu requested changes Nov 29, 2023
View reviewed changes
parser/sql/dialect/sqlserver/src/main/antlr4/imports/sqlserver/BaseRule.g4 Outdated
@@ -154,6 +154,10 @@ tableName
: (owner DOT_)? name
;

tempTableName
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the different between tableName and tempTableName?

test/it/parser/src/main/resources/case/dml/insert.xml Outdated
@@ -2900,4 +2900,36 @@
</from>
</select>
</insert>
<insert sql-case-id="insert_into_my_temp_table_1">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the orginal sql?

test/it/parser/src/main/resources/case/dml/insert.xml Outdated
</values>
</insert>

<insert sql-case-id="insert_into_temp_table_2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename sql-case-id to more meaningful name.

test/it/parser/src/main/resources/case/dml/select-expression.xml Outdated
</expression-projection>
</projections>
<from>
<simple-table name="#t" start-index="23" stop-index="25">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use <simple-table><simple-table /> if it doesn't have child node.

@BykaWF
Copy link
Author

BykaWF commented Dec 2, 2023

I've encountered an issue during testing where I consistently receive the following error:

  org.apache.shardingsphere.sql.parser.exception.SQLParsingException: You have an error in your SQL syntax: SELECT Test1Col = x FROM #t, no viable alternative at input '<EOF>' at line 1, position 27, near [@6,27:26='<EOF>',<-1>,1:27]

Could someone provide a hint or suggestions on what might be causing this issue? I think parser doesn't get it what is '#', but I have tried add '#' to IDENTIFIER_ to tableName.

@strongduanmu
Copy link
Member

IDENTIFIER_

Hi @BykaWF, What does # mean in SQLServer dialect?

@BykaWF
Copy link
Author

BykaWF commented Dec 3, 2023

@strongduanmu In SQL Server, the # symbol means temporary tables, which are session-specific and automatically dropped when the session ends.

@strongduanmu
Copy link
Member

@strongduanmu In SQL Server, the # symbol means temporary tables, which are session-specific and automatically dropped when the session ends.

@BykaWF Thank you for your explaination, I think you can add a temp table rule for this case.

@TherChenYang TherChenYang mentioned this pull request Dec 13, 2023
Merged
3 tasks
@TherChenYang
Copy link
Collaborator

TherChenYang commented Dec 13, 2023 •
edited
Loading

@strongduanmu @BykaWF
org.apache.shardingsphere.sql.parser.exception.SQLParsingException: You have an error in your SQL syntax: SELECT Test1Col = x FROM #t, no viable alternative at input '<EOF>' at line 1, position 27, near [@6,27:26='<EOF>',<-1>,1:27]

This problem is because # is set to comment in Comment.g4, but there are only two annotation rules ’-- ‘ and ’/* */‘ in the syntax of sqlServer, and there is no #, so this problem occurs. You can refer to this pr #29394

@BykaWF
Copy link
Author

BykaWF commented Dec 13, 2023

@TherChenYang thanks for helping

@strongduanmu
Copy link
Member

Hi @BykaWF, can you solve code conflict?

@BykaWF BykaWF closed this Dec 22, 2023
@BykaWF BykaWF reopened this Dec 23, 2023
@BykaWF
Copy link
Author

BykaWF commented Dec 23, 2023 •
edited
Loading

I've addressed the issues pointed out in the previous review. Could you @strongduanmu please review this updated version? Thank you!

@BykaWF BykaWF requested a review from strongduanmu January 4, 2024 09:38
@strongduanmu
Copy link
Member

Hi @BykaWF, can you solve the latest code conflict?

@BykaWF
Copy link
Author

BykaWF commented Jan 15, 2024

Hi @strongduanmu, yes I can

BykaWF and others added 4 commits January 17, 2024 16:37
@BykaWF
Merge remote-tracking branch 'origin/master'
55885bf 
# Conflicts:
#	test/it/parser/src/main/resources/case/dml/insert.xml
#	test/it/parser/src/main/resources/sql/supported/dml/insert.xml
@BykaWF
Merge pull request #1 from BykaWF/master
257e257 
Update brunch
@BykaWF
Added support for INSERT statement
e9181a0
@strongduanmu
Merge branch 'master' into insert_into
0cfa07b
@strongduanmu
Copy link
Member

Since this pr has long time no response, I will close it. You can submit new pr when you are free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strongduanmu strongduanmu Awaiting requested review from strongduanmu

Assignees
No one assigned
Labels
db: SQLServer in: SQL parse type: enhancement
Projects
None yet
Milestone
5.5.0
Development

Successfully merging this pull request may close these issues.

Support parsing SQL Server INSERT INTO sql
3 participants
@BykaWF @strongduanmu @TherChenYang