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, statistics: build new histogram using range information #7921

Merged
merged 32 commits into from Jan 14, 2019

Conversation

@winoros
Copy link
Member

commented Oct 16, 2018

What problem does this PR solve?

Not a problem. To enhance the histColl held by DataSoure's StatsInfo if we will hold it.

What is changed and how it works?

Cut the histograms' buckets by the given range information.
Currently only numeric column's bucket' bound will be changed during this process.
Other column and index will only remove the bucket that have no intersection with ranges.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change
  • Has exported variable/fields change

Side effects

  • Possible performance regression
  • Increased code complexity

Related changes

  • Need to update the documentation
  • Need to be included in the release note

This change is Reviewable

@winoros winoros changed the title Selectivity planner, statistics: build new histogram using range information Oct 16, 2018
planner/core/stats.go Outdated Show resolved Hide resolved
statistics/feedback.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/scalar.go Outdated Show resolved Hide resolved
statistics/table.go Outdated Show resolved Hide resolved
@zz-jason

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

@winoros any update?

@winoros

This comment has been minimized.

Copy link
Member Author

commented Oct 24, 2018

Updated @lamxTyler

winoros added 2 commits Oct 25, 2018
@winoros winoros force-pushed the winoros:selectivity branch from 6f352a2 to e9df1a3 Oct 25, 2018
@winoros

This comment has been minimized.

Copy link
Member Author

commented Oct 25, 2018

Please take a look at Histogram.CalcRangeFraction.... Any better idea to do it?

statistics/histogram.go Outdated Show resolved Hide resolved
@winoros

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2018

For index, we need to encode ranges if we want to split them by bucket bound while we need to use non-encode ranges to call GetRowCountByIndexRanges.
So we need to encode it, decode it then encode it again to get the result. It's a little inefficiency. So keep it in original logic.

@winoros

This comment has been minimized.

Copy link
Member Author

commented Nov 12, 2018

@lamxTyler PTAL

statistics/selectivity.go Outdated Show resolved Hide resolved
statistics/histogram.go Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
@winoros winoros force-pushed the winoros:selectivity branch from a028508 to 27886bb Nov 15, 2018
@@ -24,7 +24,7 @@ type StatsInfo struct {
RowCount float64
Cardinality []float64

HistColl statistics.HistColl
HistColl *statistics.HistColl

This comment has been minimized.

Copy link
@lamxTyler

lamxTyler Nov 15, 2018

Member

So it may be nil now? We should check all the places that use it to avoid nil pointer reference.

This comment has been minimized.

Copy link
@winoros

winoros Jan 8, 2019

Author Member

In this pr, it's safe. DataSource always holds a histogram.

statistics/histogram.go Outdated Show resolved Hide resolved
planner/core/stats.go Outdated Show resolved Hide resolved
planner/core/stats.go Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Outdated Show resolved Hide resolved
statistics/histogram.go Show resolved Hide resolved
statistics/histogram.go Show resolved Hide resolved
@winoros winoros requested review from lamxTyler and zz-jason Jan 8, 2019
Copy link
Member

left a comment

LGTM

@@ -465,10 +466,17 @@ func (hg *Histogram) totalRowCount() float64 {
return float64(hg.Buckets[hg.Len()-1].Count + hg.NullCount)
}

func (hg *Histogram) notNullCount() float64 {
if hg.Len() == 0 {

This comment has been minimized.

Copy link
@zz-jason

zz-jason Jan 11, 2019

Member

@winoros OK. Could you simplify totalRowCount() in this PR?

@zz-jason zz-jason dismissed their stale review Jan 11, 2019

Oh, there is a leaving comment

@zz-jason

This comment has been minimized.

Copy link
Member

commented Jan 11, 2019

@winoros Please merge master and resolve conflicts.

@winoros winoros dismissed a stale review via 3a9aa6c Jan 11, 2019
@zz-jason

This comment has been minimized.

Copy link
Member

commented Jan 14, 2019

@winoros CI failed, PTAL

@winoros

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

@zz-jason It's the thing i mentioned internally. Statistics' feedback has something wrong with UINT range.

@winoros

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

/run-all-tests

@winoros

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

@lamxTyler I changed the place where calling the SplitRange for feedback. You can see the subset changes for details.

@winoros

This comment has been minimized.

Copy link
Member Author

commented Jan 14, 2019

Also, add some code when doing ColumnPrune to make sure that if the table has unsigned pk, the generated extra rowid is also unsigned.

Copy link
Member

left a comment

LGTM

@lamxTyler lamxTyler added status/LGT2 and removed status/LGT1 labels Jan 14, 2019
lamxTyler and others added 2 commits Jan 14, 2019
@zz-jason zz-jason merged commit 82d2726 into pingcap:master Jan 14, 2019
5 checks passed
5 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details
@winoros winoros deleted the winoros:selectivity branch Mar 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.