-
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
parser: add explain format for optimizer trace #684
base: master
Are you sure you want to change the base?
Conversation
edfc884
to
14df345
Compare
Codecov Report
@@ Coverage Diff @@
## master #684 +/- ##
==========================================
+ Coverage 79.82% 79.82% +<.01%
==========================================
Files 37 37
Lines 13625 13635 +10
==========================================
+ Hits 10876 10884 +8
- Misses 2103 2105 +2
Partials 646 646
Continue to review full report at Codecov.
|
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
@@ -482,6 +482,7 @@ var tokenMap = map[string]int{ | |||
"OPTIMIZE": optimize, | |||
"OPTION": option, | |||
"OPTIONALLY": optionally, | |||
"OPT_TRACE": opttrace, |
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.
Seems that we don't need it? Check https://github.com/pingcap/parser/pull/654/files
6957be4
to
a166fb1
Compare
ExplainFormatROW = "row" | ||
ExplainFormatDOT = "dot" | ||
ExplainFormatHint = "hint" | ||
ExplainFormatTrace = "opt_trace" |
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.
Simply trace
is enough?
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.
+1
@@ -3958,6 +3959,8 @@ ExplainStmt: | |||
Stmt: $2, | |||
Format: "row", | |||
} | |||
startOffset := parser.startOffset(&yyS[yypt]) | |||
$2.SetText(string(parser.src[startOffset:])) |
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.
Could you give an example to explain the purpose of these 2 lines?
@@ -3979,6 +3982,8 @@ ExplainStmt: | |||
Stmt: $5, | |||
Format: $4, | |||
} | |||
startOffset := parser.startOffset(&yyS[yypt]) | |||
$5.SetText(string(parser.src[startOffset:])) | |||
} | |||
| ExplainSym "FORMAT" "=" ExplainFormatType "FOR" "CONNECTION" NUM |
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.
We cannot get the optimizer trace from explain for connection
since the plan may have already been generated?
@winoros: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #684 +/- ##
=======================================
Coverage 79.82% 79.82%
=======================================
Files 37 37
Lines 13625 13635 +10
=======================================
+ Hits 10876 10884 +8
- Misses 2103 2105 +2
Partials 646 646 |
What problem does this PR solve?
Add one more new explain format to enable optimizer trace.
Check List
Tests