planner: update some UTs from cost model1 to model2 by qw4990 · Pull Request #39065 · pingcap/tidb

qw4990 · 2022-11-10T07:20:05Z

What problem does this PR solve?

Issue Number: ref #35240

Problem Summary: planner: update some UTs from cost model1 to model2

What is changed and how it works?

planner: update some UTs from cost model1 to model2

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

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

ti-chi-bot · 2022-11-10T07:20:06Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Reminiscent
fzzf678

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

qw4990 · 2022-11-10T07:23:07Z

executor/explain_test.go

 		{
 			sql:      "select count(*) from t_unistore_act_rows group by b",
-			expected: []string{"2", "2", "2", "4"},
+			expected: []string{"2", "4", "4"},


Expected, the Agg is not pushed down in model2 since there are only 4 rows.

qw4990 · 2022-11-10T07:23:29Z

executor/explainfor_test.go

 	tk.MustExec("admin reload bindings;")

 	res := tk.MustQuery("explain format = 'brief' select * from t;")
-	require.Regexp(t, ".*TableReader.*", res.Rows()[0][0])


Expected, IndexScan is always better than TableScan.

qw4990 · 2022-11-10T07:24:20Z

executor/partition_table_test.go

-	tk.MustIndexLookup("select * from thash where a<100")
-	tk.MustIndexLookup("select * from trange where a<100")
-	tk.MustIndexLookup("select * from tlist where a<1")
+	require.True(t, tk.HasPlan("select * from thash where a<100", "TableFullScan"))


Expected, model2 prefers to use Scan instead of Lookup to avoid triggering too many double-read requests.

qw4990 · 2022-11-10T07:25:06Z

executor/partition_table_test.go

 		"└─IndexRangeScan 2.00 cop[tikv] table:t, index:b(b) range:[2,2], [3,3], keep order:false"))
 	tk.MustQuery(`explain format = 'brief' select * from t,t2 where t2.a = 1 and t2.b = t.b`).Check(testkit.Rows(
 		"Projection 1.00 root  testpartitiontableexplain.t.a, testpartitiontableexplain.t.b, testpartitiontableexplain.t2.a, testpartitiontableexplain.t2.b",
-		"└─IndexJoin 1.00 root  inner join, inner:IndexReader, outer key:testpartitiontableexplain.t2.b, inner key:testpartitiontableexplain.t.b, equal cond:eq(testpartitiontableexplain.t2.b, testpartitiontableexplain.t.b)",


Expected, model2 prefers to avoid using double-read IndexJoin.

qw4990 · 2022-11-10T07:25:42Z

planner/core/partition_pruner_test.go

 	tk.MustQuery(`explain format='brief' select /*+ HASH_AGG() */ count(1) from t1 where dt in ('2020-11-27','2020-11-28')`).Check(
 		testkit.Rows("HashAgg 1.00 root  funcs:count(Column#5)->Column#4",
 			"└─PartitionUnion 2.00 root  ",
-			"  ├─HashAgg 1.00 root  funcs:count(Column#7)->Column#5",


Expected, it will resolved by #38874.

qw4990 · 2022-11-10T07:26:15Z

planner/core/testdata/binary_plan_suite_out.json

            "children": [
              {
                "name": "TableFullScan_4",
-                "cost": 570000,


Expected, no plan change, just update cost values.

qw4990 · 2022-11-10T07:27:00Z

planner/core/testdata/join_reorder_suite_out.json

@@ -2533,60 +2533,60 @@
          "  │   └─Selection 9990.00 cop[tikv]  not(isnull(test.t3.b))",
          "  │     └─TableFullScan 10000.00 cop[tikv] table:t3, partition:p2 keep order:false, stats:pseudo",
          "  └─HashJoin(Probe) 58476.62 root  inner join, equal:[eq(test.t.b, test.t2.b)]",
-          "    ├─HashJoin(Build) 46781.30 root  left outer join, equal:[eq(test.t.a, test.t1.a)]",


Expected, just update HashJoin build and probe sides since model2 takes row-size into consideration when calculating HashJoin costs.

qw4990 · 2022-11-10T07:57:25Z

/run-build

qw4990 · 2022-11-10T08:04:51Z

/run-check_dev

qw4990 · 2022-11-10T08:27:28Z

/run-all-tests

…2-ut

qw4990 · 2022-11-10T09:34:23Z

/run-unit-test
/run-mysql-test

…2-ut

qw4990 · 2022-11-11T04:56:56Z

util/ranger/testdata/ranger_suite_out.json

          "  ├─TableReader(Build) 2.00 root  data:TableFullScan",
          "  │ └─TableFullScan 2.00 cop[tikv] table:t keep order:false",
-          "  └─StreamAgg(Probe) 2.00 root  funcs:count(1)->Column#26",
+          "  └─HashAgg(Probe) 2.00 root  funcs:count(1)->Column#26",


Expected, HashAgg is better than StreamAgg, IndexScan is better than TableScan.

qw4990 · 2022-11-11T04:57:56Z

planner/core/testdata/analyze_suite_out.json

-          "  └─TableFullScan 8.00 cop[tikv] table:t keep order:false"
+          "HashAgg 2.00 root  group by:test.t.a, funcs:count(Column#4)->Column#3",
+          "└─TableReader 2.00 root  data:HashAgg",
+          "  └─HashAgg 2.00 cop[tikv]  group by:test.t.a, funcs:count(1)->Column#4",


Expected, pushing it down can reduce the risks of under-estimation.

qw4990 · 2022-11-11T04:58:11Z

planner/core/testdata/analyze_suite_out.json

-      "TableReader(Table(t)->Sel([le(test.t.b, 40)])->StreamAgg)->StreamAgg",
-      "TableReader(Table(t)->Sel([le(test.t.b, 50)])->StreamAgg)->StreamAgg",
-      "TableReader(Table(t)->Sel([le(test.t.b, 100000000000)])->StreamAgg)->StreamAgg",
+      "TableReader(Table(t)->Sel([le(test.t.b, 20)])->HashAgg)->HashAgg",


Expected, HashAgg is better.

qw4990 · 2022-11-11T04:58:56Z

planner/core/testdata/plan_suite_out.json

-          "│       └─TableFullScan 10000.00 cop[tikv] table:td keep order:false, stats:pseudo",
+          "│ └─HashAgg 199.80 root  group by:test.td.id, funcs:max(Column#32)->Column#19, funcs:firstrow(test.td.id)->test.td.id",
+          "│   └─TableReader 199.80 root  data:HashAgg",
+          "│     └─HashAgg 199.80 cop[tikv]  group by:test.td.id, funcs:max(test.td.id)->Column#32",


Expected, pushing the Agg down is safer.

qw4990 · 2022-11-11T05:51:33Z

/merge

ti-chi-bot · 2022-11-11T05:51:37Z

This pull request has been accepted and is ready to merge.

Details

Commit hash: 8a398a4

sre-bot · 2022-11-11T06:41:45Z

TiDB MergeCI notify

🔴 Bad News! [4] CI still failing after this pr merged.
These failed integration tests don't seem to be introduced by the current PR.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-ddl-test	🔴 failed 1, success 5, total 6	43 min	Existing failure
idc-jenkins-ci/integration-cdc-test	🔴 failed 1, success 38, total 39	21 min	Existing failure
idc-jenkins-ci-tidb/integration-common-test	🔴 failed 1, success 16, total 17	11 min	Existing failure
idc-jenkins-ci-tidb/mybatis-test	🔴 failed 1, success 0, total 1	10 min	Existing failure
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	9 min 10 sec	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	7 min 25 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	6 min 4 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	4 min 54 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 53 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

fixup

2b3a37e

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 10, 2022

qw4990 added sig/planner SIG: Planner epic/cost-model the optimizer cost model and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 10, 2022

fixup

198f94d

ti-chi-bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 10, 2022

qw4990 commented Nov 10, 2022

View reviewed changes

Merge branch 'master' into model2-ut

a2afbd1

qw4990 added 3 commits November 10, 2022 16:43

Merge remote-tracking branch 'upstream/master' into model2-ut

d3a1bfd

Merge branch 'model2-ut' of https://github.com/qw4990/tidb into model…

11e0f33

…2-ut

Merge branch 'master' into model2-ut

871577a

qw4990 added 3 commits November 10, 2022 17:39

fixup

208d4a9

Merge branch 'model2-ut' of https://github.com/qw4990/tidb into model…

725de71

…2-ut

fixup

8f0e0ae

qw4990 requested review from Reminiscent and fzzf678 November 10, 2022 09:41

Merge branch 'master' into model2-ut

593b955

fixup

7fd5e52

Reminiscent approved these changes Nov 10, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 10, 2022

fzzf678 approved these changes Nov 10, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 10, 2022

qw4990 added 13 commits November 11, 2022 10:29

Merge branch 'master' into model2-ut

1f506c9

fixup

404ebb7

Merge branch 'master' into model2-ut

5cd3f91

fixup

a4e5599

fixup

64baf2e

fixup

8835e52

fixup

517b2fe

fixup

cdd04eb

fixup

f3e1c73

fixup

91f518a

fixup

41b5071

fixup

90cbff3

fixup

60335af

qw4990 commented Nov 11, 2022

View reviewed changes

qw4990 added 2 commits November 11, 2022 13:16

fixup

c0a60cc

Merge branch 'master' into model2-ut

8a398a4

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 11, 2022

ti-chi-bot merged commit 3bbffdf into pingcap:master Nov 11, 2022

Conversation

qw4990 commented Nov 10, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

Uh oh!

ti-chi-bot commented Nov 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qw4990 commented Nov 10, 2022

Uh oh!

qw4990 commented Nov 10, 2022

Uh oh!

qw4990 commented Nov 10, 2022

Uh oh!

qw4990 commented Nov 10, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qw4990 commented Nov 11, 2022

Uh oh!

ti-chi-bot commented Nov 11, 2022

Uh oh!

sre-bot commented Nov 11, 2022

TiDB MergeCI notify

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ti-chi-bot commented Nov 10, 2022 •

edited

Loading