-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
added logic to handle semicolon in the query #7861
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7861 +/- ##
============================================
- Coverage 71.65% 65.26% -6.40%
+ Complexity 4080 4078 -2
============================================
Files 1581 1536 -45
Lines 81350 79486 -1864
Branches 12128 11926 -202
============================================
- Hits 58293 51878 -6415
- Misses 19117 23910 +4793
+ Partials 3940 3698 -242
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
Outdated
Show resolved
Hide resolved
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
Outdated
Show resolved
Hide resolved
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
Outdated
Show resolved
Hide resolved
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
Show resolved
Hide resolved
Hi @Manis99803 this may not be the right approach because stripping semicolons in a context insensitive way will break when literals which contain semicolons. |
Hi @richardstartin , |
Looks good assuming the tests pass 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix and adding the test
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
Outdated
Show resolved
Hide resolved
* added logic to handle semicolon in the query * updated the logic to handle semicolon * removed the comment * updated comments * added trim function to sql * updated the logic of semicolon handling in the sql Co-authored-by: Manish Soni <manisson@cisco.com>
Description
Currently if we have a semicolon in the query, Pinot throws exception. However, semicolon are used in the SQL query to mark the termination of the query, and if a semicolon is present in the Pinot query exception shouldn't be thrown.
This PR implements the fix for the same
Issue Link: #7713