plan: maintain `HistColl` in `Selection` and `Projection`'s `statsInfo`. #7595

winoros · 2018-09-03T12:44:33Z

What problem does this PR solve?

Maintain the HistColl in Selection and Projection.

Note: Currently maintain it or not is controlled by tidb_optimizer_selectivity_level.

What is changed and how it works?

For selection it only needs to assign the HistColl of its child.(Currently we don't handle the bound value of the Histograms)

For projection, build new map.

Check List

Tests

Unit test

winoros · 2018-09-03T12:46:52Z

plan/stats.go

@@ -47,7 +47,6 @@ func (s *statsInfo) scale(factor float64) *statsInfo {
 	profile := &statsInfo{
 		count:          s.count * factor,
 		cardinality:    make([]float64, len(s.cardinality)),
-		histColl:       s.histColl,


scale is called by scaleByExpectedCnt which is called many many times when building physical plan.
It's hard to handle expectedCnt with stats estimation by Histogram. So I think in such case HistColl is not need to maintained. Thus maintain the HistColl out of this method.

But since it's the problem of scaleByExpecteCnt, why not do it in that function?

Emm.. i need some clearer explain?

shenli · 2018-09-04T02:03:59Z

sessionctx/variable/varsutil.go

@@ -293,7 +293,7 @@ func ValidateSetSystemVar(vars *SessionVars, name string, value string) (string,
 		TiDBDistSQLScanConcurrency,
 		TiDBIndexSerialScanConcurrency, TiDBDDLReorgWorkerCount,
 		TiDBBackoffLockFast, TiDBMaxChunkSize,
-		TiDBDMLBatchSize, TiDBOptimizerSelectivityLevel:


Why remove it?

This validation is not right. The default value of it is zero. But here just checks what it should not be zero.

shenli · 2018-09-04T02:05:27Z

plan/cbo_test.go

@@ -690,6 +690,28 @@ func (s *testAnalyzeSuite) TestInconsistentEstimation(c *C) {
 		))
 }

+func (s *testAnalyzeSuite) TestHistogramInPlan(c *C) {


Add comments in the test case.

shenli · 2018-09-04T02:07:56Z

plan/stats.go

 	return p.stats, nil
 }

+func (p *LogicalProjection) deriveHistStats(childProfile *statsInfo) {


Add comments in the function.

plan/stats.go

alivxxx · 2018-09-04T03:38:19Z

plan/stats.go

@@ -47,7 +47,6 @@ func (s *statsInfo) scale(factor float64) *statsInfo {
 	profile := &statsInfo{
 		count:          s.count * factor,
 		cardinality:    make([]float64, len(s.cardinality)),
-		histColl:       s.histColl,


But since it's the problem of scaleByExpecteCnt, why not do it in that function?

zz-jason · 2018-09-10T08:16:26Z

plan/stats.go

@@ -174,7 +175,23 @@ func (p *LogicalSelection) deriveStats() (*statsInfo, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	p.stats = childProfile.scale(selectionFactor)
+	factor := selectionFactor
+	if p.ctx.GetSessionVars().OptimizerSelectivityLevel >= 1 && childProfile.histColl != nil {


what does OptimizerSelectivityLevel >= 1 mean?

Just its literal meaning?
This variable controls how we maintain stats to calculate selectivity. The higher it is, the more powerful way we use. Currently, it should be 0 or 1.

How to support OptimizerSelectivityLevel=2?

For example, when it's 1, we only use the DataSource's Histogram and won't create newly one. When it's 2, we'll create new Histogram during planning.

how about making some constant values to represent different levels, for example:

const ( SLPseudo int = 0 SLStatsDataSourceOnly = 1 SLStatsAll = 2 )

zz-jason · 2018-09-10T08:17:35Z

plan/stats.go

@@ -174,7 +175,23 @@ func (p *LogicalSelection) deriveStats() (*statsInfo, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	p.stats = childProfile.scale(selectionFactor)
+	factor := selectionFactor


how about s/factor/selecity/?

Em, selecity is not a word, you mean selectivity?

plan/stats.go

zz-jason · 2018-09-10T08:29:14Z

plan/stats.go

@@ -110,11 +109,10 @@ func (p *baseLogicalPlan) deriveStats() (*statsInfo, error) {
 	return profile, nil
 }

-func (ds *DataSource) getStatsByFilter(conds expression.CNFExprs) *statsInfo {
-	profile := &statsInfo{
+func (ds *DataSource) getStatsByFilter(conds expression.CNFExprs) (profile *statsInfo, histColl *statistics.HistColl) {


It seems that all the code changes related to the functions of DataSource is not irrelevant?

Yes, we can extract the same part with selection.deriveStats

winoros · 2018-09-26T08:46:28Z

Em.. Some unresolved comments are hidden after merged master...

winoros · 2018-09-27T09:34:31Z

@zz-jason

how about making some constant values to represent different levels, for example:
const (
SLPseudo int = 0
SLStatsDataSourceOnly = 1
SLStatsAll = 2
)

I found that no other variable creates const like this. Could we add one for this variable?

winoros · 2018-10-10T08:17:59Z

Will re-push it again later

winoros added 2 commits September 3, 2018 16:35

plan: maintain HistColl in Selection and Projection's statsInfo.

317de51

tiny change.

a582bf2

winoros commented Sep 3, 2018

View reviewed changes

winoros added the sig/planner SIG: Planner label Sep 3, 2018

shenli reviewed Sep 4, 2018

View reviewed changes

alivxxx reviewed Sep 4, 2018

View reviewed changes

address comments.

239f4ff

zz-jason reviewed Sep 10, 2018

View reviewed changes

winoros added 2 commits September 26, 2018 16:39

Merge branch 'master' into hist-proj-select

e68813a

remove merge conflict

76f96e4

winoros added 3 commits September 26, 2018 17:22

fix explain test.

ca5b5a7

fix some unnecessary change in merging master

7db31b5

address comment and tiny change

6b6c285

winoros force-pushed the hist-proj-select branch from 8c9b90c to 6b6c285 Compare September 27, 2018 09:19

winoros added 2 commits September 27, 2018 20:38

address comments.

9a4cfe5

fix golint

bc371d9

winoros closed this Oct 10, 2018

winoros deleted the hist-proj-select branch March 15, 2019 07:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plan: maintain `HistColl` in `Selection` and `Projection`'s `statsInfo`. #7595

plan: maintain `HistColl` in `Selection` and `Projection`'s `statsInfo`. #7595

winoros commented Sep 3, 2018

winoros Sep 3, 2018

alivxxx Sep 4, 2018

winoros Sep 6, 2018 •

edited

Loading

shenli Sep 4, 2018

winoros Sep 6, 2018

shenli Sep 4, 2018

shenli Sep 4, 2018

alivxxx Sep 4, 2018

zz-jason Sep 10, 2018

winoros Sep 11, 2018

zz-jason Sep 11, 2018

winoros Sep 11, 2018

zz-jason Sep 11, 2018 •

edited

Loading

zz-jason Sep 10, 2018

winoros Sep 26, 2018

zz-jason Sep 26, 2018

zz-jason Sep 10, 2018

winoros Sep 26, 2018

winoros commented Sep 26, 2018

winoros commented Sep 27, 2018

winoros commented Oct 10, 2018

plan: maintain HistColl in Selection and Projection's statsInfo. #7595

plan: maintain HistColl in Selection and Projection's statsInfo. #7595

Conversation

winoros commented Sep 3, 2018

What problem does this PR solve?

What is changed and how it works?

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winoros Sep 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zz-jason Sep 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

winoros commented Sep 26, 2018

winoros commented Sep 27, 2018

winoros commented Oct 10, 2018

plan: maintain `HistColl` in `Selection` and `Projection`'s `statsInfo`. #7595

plan: maintain `HistColl` in `Selection` and `Projection`'s `statsInfo`. #7595

winoros Sep 6, 2018 •

edited

Loading

zz-jason Sep 11, 2018 •

edited

Loading