Skip to content
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

planner: generate IndexMergePath in physical optimization #10512

Merged
merged 24 commits into from
Jul 5, 2019
Merged

planner: generate IndexMergePath in physical optimization #10512

merged 24 commits into from
Jul 5, 2019

Conversation

hailanwhu
Copy link
Contributor

@hailanwhu hailanwhu commented May 16, 2019

What problem does this PR solve?

Fix #10518

Part of implement for Proposal Access a table using multiple indexes. This pr implements IndexMergeOrPath generation.

What is changed and how it works?

Add path generation and generate IndexMergePath

Check List

Tests

  • Unit test

Code changes
add planner/core/indexmerge_test.go
add generateIndexMergeOrPaths() in planner/core/stats.go
modify (ds *DataSource) deriveIndexPathStats()

@codecov
Copy link

codecov bot commented May 16, 2019

Codecov Report

Merging #10512 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #10512   +/-   ##
===========================================
  Coverage   81.1334%   81.1334%           
===========================================
  Files           419        419           
  Lines         90801      90801           
===========================================
  Hits          73670      73670           
  Misses        11877      11877           
  Partials       5254       5254

@eurekaka eurekaka added sig/planner SIG: Planner contribution This PR is from a community contributor. type/enhancement The issue or PR belongs to an enhancement. labels May 17, 2019
@eurekaka eurekaka changed the title IndexMergePath generation planner: generate IndexMergePath in physical optimization May 17, 2019
planner/core/logical_plans.go Outdated Show resolved Hide resolved
sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/logical_plans.go Outdated Show resolved Hide resolved
planner/core/logical_plans.go Outdated Show resolved Hide resolved
planner/core/logical_plans.go Outdated Show resolved Hide resolved
planner/core/logical_plans.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Show resolved Hide resolved
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM

@eurekaka eurekaka added the status/LGT1 Indicates that a PR has LGTM 1. label May 29, 2019
planner/core/stats.go Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
@qw4990 qw4990 self-requested a review May 30, 2019 09:44
@hailanwhu
Copy link
Contributor Author

@lamxTyler @winoros @zz-jason i add the case where DNF includes a pk column and an indexed column.

planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Show resolved Hide resolved
@eurekaka
Copy link
Contributor

@hailanwhu please resolve conflicts.

planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
@zhouqiang-cl
Copy link
Contributor

/rebuild

planner/core/stats.go Outdated Show resolved Hide resolved
path.index = ds.possibleAccessPaths[i].index
noIntervalRanges, err := ds.deriveIndexPathStats(path, conditions)
if err != nil {
logutil.BgLogger().Info("can not derive statistics of a path")
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@eurekaka eurekaka requested review from alivxxx and qw4990 July 2, 2019 02:33
sessionctx/variable/tidb_vars.go Outdated Show resolved Hide resolved
planner/core/logical_plans.go Outdated Show resolved Hide resolved
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

add the new variable in NewSessionVars.

@eurekaka eurekaka requested review from alivxxx and winoros July 5, 2019 02:31
Copy link
Member

@winoros winoros left a comment

Choose a reason for hiding this comment

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

lgtm

@eurekaka eurekaka added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 5, 2019
@eurekaka
Copy link
Contributor

eurekaka commented Jul 5, 2019

/run-all-tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor. sig/planner SIG: Planner status/LGT2 Indicates that a PR has LGTM 2. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate IndexMergePath for DNF filters
6 participants