planner: respect `tidb_analyze_column_options` when build analyze plan #54240

Rustin170506 · 2024-06-26T08:32:32Z

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 to PREDICATE, we should only analyze predicate columns.

What changed and how does it work?

Checked tidb_analyze_default_column_choice when building the analyze plan.
Only analyze indexes and the columns composing the indexes if there are no predicate columns.
Added some tests to cover the analyze plan building code.
Removed all use of tidb_enable_column_tracking.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

codecov · 2024-06-26T09:04:40Z

Codecov Report

Attention: Patch coverage is 76.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 56.1611%. Comparing base (184b010) to head (5a30f87).
Report is 15 commits behind head on master.

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

Flag	Coverage Δ
integration	`36.3109% <10.0000%> (?)`
unit	`71.7921% <76.0000%> (-1.8609%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`52.9656% <ø> (-2.2339%)`	⬇️
parser	`∅ <ø> (∅)`
br	`52.6314% <ø> (+4.6677%)`	⬆️

Rustin170506

🔢 Self-check (PR reviewed by myself and ready for feedback.)

Rustin170506 · 2024-06-27T07:44:11Z

pkg/statistics/handle/usage/predicate_column_test.go

+
+	// Analyze table.
+	tk.MustExec("analyze table t")
+	// FIXME: We should correct the job info or skip this kind of job.


I will fix this problem in my next PR.

Rustin170506 · 2024-06-27T07:44:29Z

pkg/statistics/handle/usage/predicate_column_test.go

+			"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.


I will improve it in my next PR.

…yze job Signed-off-by: hi-rustin <rustin.liu@gmail.com> fix Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

🔢 Self-check (PR reviewed by myself and ready for feedback.)

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506

🔢 Self-check (PR reviewed by myself and ready for feedback.)

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

pkg/planner/core/planbuilder.go

pkg/statistics/handle/usage/predicate_column_test.go

AilinKid

Rest LGTM

AilinKid · 2024-06-28T08:21:28Z

pkg/statistics/handle/usage/predicate_column_test.go

+	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"),


is that means all columns in L271's msg?

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 <ailinsilence4@gmail.com>

pkg/statistics/handle/usage/predicate_column_test.go

ti-chi-bot · 2024-06-28T09:04:24Z

[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:

~~OWNERS~~ [AilinKid,qw4990]
~~pkg/planner/OWNERS~~ [AilinKid,qw4990]
~~pkg/statistics/OWNERS~~ [AilinKid,qw4990]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-06-28T09:04:26Z

[LGTM Timeline notifier]

Timeline:

2024-06-27 09:42:25.474203643 +0000 UTC m=+885471.959692476: ☑️ agreed by qw4990.
2024-06-28 09:04:25.952711678 +0000 UTC m=+969592.438200509: ☑️ agreed by AilinKid.

Rustin170506 · 2024-06-28T09:51:58Z

/retest

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. sig/planner SIG: Planner size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2024

Rustin170506 force-pushed the rustin-patch-sessionctx-stack1-analyze branch from 64d7b81 to 7bdc3c7 Compare June 26, 2024 08:36

Rustin170506 force-pushed the rustin-patch-sessionctx-stack1-analyze branch from 58d2dbd to 990c0d2 Compare June 26, 2024 09:38

Rustin170506 changed the title ~~planner: respect tidb_analyze_default_column_choice when build analyze plan~~ planner: respect tidb_analyze_column_options when build analyze plan Jun 26, 2024

Rustin170506 force-pushed the rustin-patch-sessionctx-stack1-analyze branch 3 times, most recently from d4b15f6 to b11f780 Compare June 26, 2024 09:58

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 26, 2024

Rustin170506 force-pushed the rustin-patch-sessionctx-stack1-analyze branch from e5cd628 to 8efe30a Compare June 26, 2024 11:17

Rustin170506 commented Jun 27, 2024

View reviewed changes

Rustin170506 added 13 commits June 27, 2024 15:53

planner: respect tidb_analyze_default_column_choice when build anal…

0a1c213

…yze job Signed-off-by: hi-rustin <rustin.liu@gmail.com> fix Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add case for analyzing with predicate

9e1c200

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add TestAnalyzeTableWithTiDBPersistAnalyzeOptionsEnabled

b29e96d

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add TestAnalyzeTableWithTiDBPersistAnalyzeOptionsDisabled

b3fd184

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add TestAnalyzeTableWhenV1StatsExists

bdd9398

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

chore: prepare bazel

5718b08

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add TestAutoAnalyzeWithPredicateColumns

84c048a

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: add TestAnalyzeWhenNoPredicateColumns

14d47ea

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix: do not analyze all columns

f60ff65

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

chore: add TODO

407687a

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: fix broken test

323172a

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

chore: remove useless code

be79b93

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

test: fix typo

d31c2d4

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 force-pushed the rustin-patch-sessionctx-stack1-analyze branch from e8f1d72 to d31c2d4 Compare June 27, 2024 07:53

Rustin170506 commented Jun 27, 2024

View reviewed changes

test: add TestAnalyzeNoPredicateColumnsWithPrimaryKey

6ae1a3e

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

fix: make bazel happy

04ccb5f

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

Rustin170506 commented Jun 27, 2024

View reviewed changes

Rustin170506 requested review from AilinKid and qw4990 June 27, 2024 08:39

test: fix broken test

a1950b8

Signed-off-by: hi-rustin <rustin.liu@gmail.com>

qw4990 approved these changes Jun 27, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 27, 2024

AilinKid reviewed Jun 28, 2024

View reviewed changes

pkg/planner/core/planbuilder.go Outdated Show resolved Hide resolved

AilinKid reviewed Jun 28, 2024

View reviewed changes

pkg/statistics/handle/usage/predicate_column_test.go Outdated Show resolved Hide resolved

AilinKid reviewed Jun 28, 2024

View reviewed changes

Update pkg/planner/core/planbuilder.go

fcee8d6

Co-authored-by: Arenatlx <ailinsilence4@gmail.com>

Rustin170506 commented Jun 28, 2024

View reviewed changes

pkg/statistics/handle/usage/predicate_column_test.go Outdated Show resolved Hide resolved

Update pkg/statistics/handle/usage/predicate_column_test.go

5a30f87

Rustin170506 requested a review from AilinKid June 28, 2024 08:30

AilinKid approved these changes Jun 28, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 28, 2024

ti-chi-bot bot merged commit 0d0fb4d into pingcap:master Jun 28, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: respect `tidb_analyze_column_options` when build analyze plan #54240

planner: respect `tidb_analyze_column_options` when build analyze plan #54240

Rustin170506 commented Jun 26, 2024 •

edited

Loading

codecov bot commented Jun 26, 2024 •

edited

Loading

Rustin170506 left a comment

Rustin170506 Jun 27, 2024

Rustin170506 Jun 27, 2024

Rustin170506 left a comment

Rustin170506 left a comment

AilinKid left a comment

AilinKid Jun 28, 2024

Rustin170506 Jun 28, 2024 •

edited

Loading

ti-chi-bot bot commented Jun 28, 2024

ti-chi-bot bot commented Jun 28, 2024

Rustin170506 commented Jun 28, 2024

planner: respect tidb_analyze_column_options when build analyze plan #54240

planner: respect tidb_analyze_column_options when build analyze plan #54240

Conversation

Rustin170506 commented Jun 26, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

codecov bot commented Jun 26, 2024 • edited Loading

Codecov Report

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 Jun 27, 2024

Choose a reason for hiding this comment

Rustin170506 Jun 27, 2024

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

Rustin170506 left a comment

Choose a reason for hiding this comment

AilinKid left a comment

Choose a reason for hiding this comment

AilinKid Jun 28, 2024

Choose a reason for hiding this comment

Rustin170506 Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot bot commented Jun 28, 2024

ti-chi-bot bot commented Jun 28, 2024

[LGTM Timeline notifier]

Rustin170506 commented Jun 28, 2024

planner: respect `tidb_analyze_column_options` when build analyze plan #54240

planner: respect `tidb_analyze_column_options` when build analyze plan #54240

Rustin170506 commented Jun 26, 2024 •

edited

Loading

codecov bot commented Jun 26, 2024 •

edited

Loading

Rustin170506 Jun 28, 2024 •

edited

Loading