Skip to content

planner: Add the tableInfo parameter for the GetNetworkFactor in the TableReaderImpl.CalcCost#25134

Closed
mmyj wants to merge 7 commits into
pingcap:masterfrom
mmyj:feat-cost
Closed

planner: Add the tableInfo parameter for the GetNetworkFactor in the TableReaderImpl.CalcCost#25134
mmyj wants to merge 7 commits into
pingcap:masterfrom
mmyj:feat-cost

Conversation

@mmyj
Copy link
Copy Markdown
Member

@mmyj mmyj commented Jun 4, 2021

What problem does this PR solve?

Problem Summary:

In the PR #25046, TableReaderImpl don't have tableInfo property, so I had left a todo comment, using nil as parameter to get network factor. Now I found that GetTableScan function can get the tableInfo of TableReaderImpl and resolve above problem.

What is changed and how it works?

How it Works:

Using GetTableScan to get the tableInfo property of TableReaderImpl

Related changes

  • N/A

Check List

Tests

  • No code

Side effects

  • N/A

Release note

  • Add the tableInfo parameter for the GetNetworkFactor in the TableReaderImpl.CalcCost

@mmyj mmyj requested a review from a team as a code owner June 4, 2021 04:57
@mmyj mmyj requested review from rebelice and removed request for a team June 4, 2021 04:57
@ti-chi-bot
Copy link
Copy Markdown
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 4, 2021
@mmyj mmyj marked this pull request as draft June 4, 2021 05:13
@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 4, 2021
@github-actions github-actions Bot added the sig/execution SIG execution label Jun 4, 2021
Comment thread planner/implementation/datasource.go Outdated
tableInfo *model.TableInfo
ts = reader.GetTableScan()
)
if ts != nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So when will it be nil? That could make cost estimation inaccurate again.

@mmyj mmyj closed this Jun 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. sig/execution SIG execution size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants