*: make analyze buckets number configurable #7619
Conversation
alivxxx
added
component/statistics
type/enhancement
statistics/update_test.go
Outdated
| @@ -890,7 +886,7 @@ func (s *testStatsUpdateSuite) TestLogDetailedInfo(c *C) { | |||
| for i := 0; i < 20; i++ { | |||
| testKit.MustExec(fmt.Sprintf("insert into t values (%d, %d, %d)", i, i, i)) | |||
| } | |||
| testKit.MustExec("analyze table t") | |||
| testKit.MustExec("analyze table t limit 4 buckets") | |||
There was a problem hiding this comment.
It is the test's requirement. Each bucket should contain more than 1 values.
parser/parser.y
Outdated
| { | ||
| $$ = uint64(0) | ||
| } | ||
| | "LIMIT" NUM "BUCKETS" |
There was a problem hiding this comment.
how about change the syntax to analyze table xx with xx buckets?
maybe we can lean from mysql: https://dev.mysql.com/doc/refman/8.0/en/analyze-table.html
executor/builder.go
Outdated
| @@ -1385,10 +1387,14 @@ func (b *executorBuilder) buildAnalyze(v *plan.Analyze) Executor { | |||
| baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ExplainID()), | |||
| tasks: make([]*analyzeTask, 0, len(v.ColTasks)+len(v.IdxTasks)), | |||
| } | |||
| maxNumBuckets := v.MaxNumBuckets | |||
| if maxNumBuckets == 0 { | |||
There was a problem hiding this comment.
IMHO, it's better to more this validation to the parser or planner.
|
@shenli It depends on the user. Maybe the user wants to limit the stats memory usage, or just want to try and see if the plan gets better. Anyway, it is optional. |
|
@lamxTyler |
|
@coocood Yes, it will. We can fix it in the following pr. |
plan/planbuilder.go
Outdated
| @@ -680,7 +680,12 @@ func (b *planBuilder) buildAnalyzeAllIndex(as *ast.AnalyzeTableStmt) Plan { | |||
| return p | |||
| } | |||
|
|
|||
| const defaultNumBuckets = 256 | |||
|
LGTM |
plan/planbuilder.go
Outdated
| func (b *planBuilder) buildAnalyze(as *ast.AnalyzeTableStmt) (Plan, error) { | ||
| if as.MaxNumBuckets == 0 { | ||
| as.MaxNumBuckets = defaultMaxNumBuckets |
There was a problem hiding this comment.
should we set an upper limit for the MaxNumBuckets
|
/run-all-tests |
What problem does this PR solve?
Make analyze buckets number configurable.
What is changed and how it works?
Extend the syntax of analyze, so we can use statement like
analyze table t with 4 bucketsto configure the bucket number.Check List
Tests
Code changes
Side effects
Related changes
PTAL @coocood @zz-jason @winoros