-
Notifications
You must be signed in to change notification settings - Fork 489
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
ast: change the order of visiting select stmt #522
Conversation
Codecov Report
@@ Coverage Diff @@
## master #522 +/- ##
==========================================
- Coverage 71.77% 71.62% -0.15%
==========================================
Files 32 32
Lines 7787 7740 -47
==========================================
- Hits 5589 5544 -45
+ Misses 1672 1671 -1
+ Partials 526 525 -1
Continue to review full report at Codecov.
|
@@ -764,6 +764,8 @@ type SelectStmt struct { | |||
IsAfterUnionDistinct bool | |||
// IsInBraces indicates whether it's a stmt in brace. | |||
IsInBraces bool | |||
// QueryBlockOffset indicates the order of this SelectStmt if counted from left to right in the sql text. | |||
QueryBlockOffset int |
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.
Where would this field be set?
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.
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.
It looks pretty weird that the values is set from another repo, can we set it during the induction of parser?
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.
It seems that there is no way to set it correctly here, since the subpart of select stmt will get the offset first. I don't find good ways to set it correctly here.
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.
LGTM
This reverts commit 52dcfa0.
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.
lgtm
What problem does this PR solve?
Adjust the visit order in select stmt so we can tranverse from left to right in the origin text.
For pingcap/tidb#11861
What is changed and how it works?
Adjust the vist order and add a offset variable in select stmt.
Check List
Tests
Code changes
Side effects
Related changes