-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix the parser to allow multiple strings one after the other #15076
Fix the parser to allow multiple strings one after the other #15076
Conversation
Signed-off-by: Manan Gupta <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: Manan Gupta <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #15076 +/- ##
==========================================
+ Coverage 47.29% 47.69% +0.39%
==========================================
Files 1137 1155 +18
Lines 238684 240151 +1467
==========================================
+ Hits 112895 114542 +1647
+ Misses 117168 117010 -158
+ Partials 8621 8599 -22 ☔ View full report in Codecov by Sentry. |
@@ -2996,7 +2996,7 @@ INPUT | |||
select concat(a, if(b>10, 'x' 'æ', 'y' 'ß')) from t1; | |||
END | |||
OUTPUT | |||
select concat(a, if(b > 10, 'x' as `æ`, 'y' as `ß`)) from t1 |
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.
this is where I found the issue
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.
❤️
Description
This PR fixes the bug described in #15075.
According to the ANSI standard, strings that follow one after the other, should be concatenated together. We see the same behaviour in MySQL.
This PR replicates this behavior in Vitess as well. We now allow multiple strings one after the other, and like MySQL we concatenate them. A new precedence rule had to be introduced in the parser to remove the shift-reduce conflicts caused by the initial changes to the grammar.
Although we parse the queries now and our results are going to match MySQL, we are not making the effort to replicate the weird field name that MySQL has. i.e. -
MySQL output -
Vitess output -
Related Issue(s)
Checklist
Deployment Notes