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.

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

TSQL: fix statement delimitation #1612

Merged
merged 20 commits into from Oct 12, 2021

Conversation

jpers36
Copy link
Contributor

@jpers36 jpers36 commented Oct 12, 2021

Fixes #1581
Fixes #1555

The TSQL dialect allows for optional semicolon delimiters between statements. Since they are optional, we cannot rely on them to delimit the codebase. This code change removes all places where we assume semicolons will delimit and replaces them with optional references.

As part of this I discovered a bug with MetaSegment. If a MetaSegment leads a Sequence (as in FromExpressionSegment) it might be queried as to whether it is simple. The simple() function calls match() by default, which does not exist for MetaSegment. I added a simple() function override in MetaSegment which returns None.

TSQL changes:
+SelectClauseSegmentGrammar to override ANSI standard and remove delimitation logic that assumes the statements have been delimited
+SelectClauseElementSegment to override ANSI standard and remove GreedyUntil that assumes the statements have been delimited
+SelectClauseSegment to override ANSI standard and remove StartsWith logic that assumes the statements have been delimited
+DeleteStatementSegment to override ANSI standard and remove StartsWith logic that assumes the statements have been delimited
+FromClauseSegment to override ANSI standard and remove Delimited logic that assumes the statements have been delimited
+OrderByClauseSegment to override ANSI standard and remove StartsWith logic that assumes the statements have been delimited
Adjust SelectStatementSegment to allow for an optional semi-colon instead of an expected one.
Adjust CreateIndexStatementSegment to allow for an optional semi-colon instead of an expected one.
Adjust PivotUnpivotStatementSegment to remove StartsWith logic that assumes the statements have been delimited
Adjust CreateProcedureStatementSegment to remove StartsWith logic that assumes the statements have been delimited
Adjust CreateViewStatementSegment to allow for an optional semi-colon instead of an expected one.
Adjust CreateTableStatementSegment to allow for an optional semi-colon instead of an expected one.
Adjust TransactionStatementSegment to allow for an optional semi-colon instead of an expected one.
Adjust BatchSegment to clearly delineate logic between statements which can be bundled and statements which can't
Adjust BeginEndSegment to only include statements which can be bundled. As a result, we remove the reference to BatchSegment
+begin_end_no_semicolon test case to confirm that statements are parsed properly without semicolons
Adjust various test case YML scripts to reflect the changes above

meta.py changes:
import ParseContext, Optional, and List
Override simple() to return None instead of attempting to call match()

@jpers36 jpers36 marked this pull request as draft October 12, 2021 17:57
@codecov
Copy link

codecov bot commented Oct 12, 2021

Codecov Report

Merging #1612 (48bf20f) into main (853ceef) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #1612   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          131       131           
  Lines         9184      9208   +24     
=========================================
+ Hits          9184      9208   +24     
Impacted Files Coverage Δ
src/sqlfluff/core/parser/segments/meta.py 100.00% <100.00%> (ø)
src/sqlfluff/dialects/dialect_tsql.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 853ceef...48bf20f. Read the comment docs.

@jpers36 jpers36 marked this pull request as ready for review October 12, 2021 19:22
Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

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

Looks good. One suggestion.

src/sqlfluff/dialects/dialect_tsql.py Outdated Show resolved Hide resolved
src/sqlfluff/dialects/dialect_tsql.py Outdated Show resolved Hide resolved
src/sqlfluff/dialects/dialect_tsql.py Outdated Show resolved Hide resolved
jpers36 and others added 2 commits October 12, 2021 17:15
Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>
@tunetheweb tunetheweb merged commit 633a23f into sqlfluff:main Oct 12, 2021
ttomasz pushed a commit to ttomasz/sqlfluff that referenced this pull request Oct 12, 2021
* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* black

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* black

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Update src/sqlfluff/dialects/dialect_tsql.py

Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Fix Statement Delimitation Issue

* Apply suggestions from code review

Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>

* Fix Statement Delimitation Issue

Co-authored-by: jpersons <jpersons@iuhealth.org>
Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TSQL: BatchSegment doesn't know where to END without semicolon TSQL: Incorrect indent in if block
2 participants