-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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: respect tidb_analyze_column_options
when build analyze plan
#54240
planner: respect tidb_analyze_column_options
when build analyze plan
#54240
Conversation
64d7b81
to
7bdc3c7
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #54240 +/- ##
=================================================
- Coverage 74.7619% 56.1611% -18.6008%
=================================================
Files 1523 1645 +122
Lines 361410 613324 +251914
=================================================
+ Hits 270197 344450 +74253
- Misses 71584 245648 +174064
- Partials 19629 23226 +3597
Flags with carried forward coverage won't be shown. Click here to find out more.
|
58d2dbd
to
990c0d2
Compare
tidb_analyze_default_column_choice
when build analyze plantidb_analyze_column_options
when build analyze plan
d4b15f6
to
b11f780
Compare
e5cd628
to
8efe30a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 Self-check (PR reviewed by myself and ready for feedback.)
|
||
// Analyze table. | ||
tk.MustExec("analyze table t") | ||
// FIXME: We should correct the job info or skip this kind of job. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will fix this problem in my next PR.
"Note 1105 Analyze use auto adjusted sample rate 1.000000 for table test.t, reason to use this rate is \"use min(1, 110000/10000) as the sample-rate=1\"", | ||
), | ||
) | ||
// TODO: we should also include indexes in the job info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will improve it in my next PR.
…yze job Signed-off-by: hi-rustin <[email protected]> fix Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
e8f1d72
to
d31c2d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 Self-check (PR reviewed by myself and ready for feedback.)
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔢 Self-check (PR reviewed by myself and ready for feedback.)
Signed-off-by: hi-rustin <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rest LGTM
tk.MustExec("analyze table t") | ||
// FIXME: We should correct the job info or skip this kind of job. | ||
tk.MustQuery("select table_name, job_info from mysql.analyze_jobs order by id desc limit 1").Check( | ||
testkit.Rows("t analyze table columns with 256 buckets, 100 topn, 1 samplerate"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that means all columns in L271's msg?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means we only analyzed the _row_id and updated the stats meta. I left a FIXME here, I will fix it in my next PR.
Co-authored-by: Arenatlx <[email protected]>
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AilinKid, qw4990 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What problem does this PR solve?
Issue Number: ref #53567
This PR is based on #54200.
Problem Summary:
When
tidb_analyze_column_options
is set toPREDICATE
, we should only analyze predicate columns.What changed and how does it work?
tidb_analyze_default_column_choice
when building the analyze plan.tidb_enable_column_tracking
.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.