-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
plan: refine explain result format #7011
Conversation
LGTM |
plan/explain.go
Outdated
@@ -116,7 +116,7 @@ func (p *PhysicalIndexReader) ExplainInfo() string { | |||
|
|||
// ExplainInfo implements PhysicalPlan interface. | |||
func (p *PhysicalIndexLookUpReader) ExplainInfo() string { | |||
return fmt.Sprintf("index:%s, table:%s", p.indexPlan.ExplainID(), p.tablePlan.ExplainID()) | |||
return "" |
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.
Maybe better to add comment to explain why only return ""
to its caller?
Well Done. Except the comment of function, other Part looks good to me. If you resolve this, just ping me so that I can approve this PR. ;) |
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
/run-all-tests |
/run-common-test |
/run-integration-common-test |
What have you changed? (mandatory)
Delete redundant IndexLookUp description and move the
cost
column next toid
to make it more noticeable.What are the type of the changes (mandatory)?
How has this PR been tested (mandatory)?
unit test