planner: update some UTs from cost model1 to model2 #38959

qw4990 · 2022-11-08T05:18:28Z

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-08T05:18:30Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

Reminiscent
time-and-fate

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.

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

qw4990 · 2022-11-08T05:19:25Z

executor/explainfor_test.go

@@ -461,12 +462,12 @@ func TestPointGetUserVarPlanCache(t *testing.T) {
 	tk.Session().SetSessionManager(&testkit.MockSessionManager{PS: ps})
 	tk.MustQuery(fmt.Sprintf("explain for connection %d", tkProcess.ID)).Check(testkit.Rows( // can use idx_a
 		`Projection_9 1.00 root  test.t1.a, test.t1.b, test.t2.a, test.t2.b`,
-		`└─IndexJoin_17 1.00 root  inner join, inner:TableReader_13, outer key:test.t2.a, inner key:test.t1.a, equal cond:eq(test.t2.a, test.t1.a)`,


Expected, 1) model2 prefers to use Scan instead of Lookup, 2) model2 prefers to use MJ if no much data to process.

qw4990 · 2022-11-08T05:19:51Z

planner/core/testdata/analyze_suite_out.json

@@ -424,7 +424,7 @@
    "Name": "TestEmptyTable",
    "Cases": [
      "TableReader(Table(t)->Sel([le(test.t.c1, 50)]))",
-      "LeftHashJoin{TableReader(Table(t)->Sel([not(isnull(test.t.c1))]))->TableReader(Table(t1)->Sel([not(isnull(test.t1.c1))]))->HashAgg}(test.t.c1,test.t1.c1)",


Expected, pushing the Agg down is safer.

qw4990 · 2022-11-08T05:20:26Z

planner/core/testdata/expression_rewriter_suite_out.json

@@ -7,10 +7,9 @@
        "Plan": [
          "HashJoin 2.25 root  inner join, equal:[eq(test.t1.a, test.t2.a) eq(test.t1.b, test.t2.b)]",
          "├─HashAgg(Build) 1.69 root  group by:test.t2.a, test.t2.b, funcs:firstrow(test.t2.a)->test.t2.a, funcs:firstrow(test.t2.b)->test.t2.b",
-          "│ └─TableReader 1.69 root  data:HashAgg",


We can ignore this case since it will be solved by #38874.

qw4990 · 2022-11-08T05:21:37Z

planner/core/testdata/integration_suite_out.json

@@ -3196,47 +3196,44 @@
      {
        "SQL": "explain format = 'brief' select count(*) from fact_t join d1_t on fact_t.d1_k > d1_t.d1_k",
        "Plan": [
-          "HashAgg 1.00 root  funcs:count(Column#12)->Column#11",


Expected, model2 prefers to use StreamAgg instead of HashAgg if no much data to process.

qw4990 · 2022-11-08T05:22:00Z

planner/core/testdata/integration_suite_out.json

@@ -7080,10 +7077,11 @@
          "  └─TableReader 8000.00 root  data:ExchangeSender",
          "    └─ExchangeSender 8000.00 mpp[tiflash]  ExchangeType: PassThrough",
          "      └─Projection 8000.00 mpp[tiflash]  Column#5, test.t.id",
-          "        └─HashAgg 8000.00 mpp[tiflash]  group by:test.t.id, test.t.name, funcs:count(1)->Column#5, funcs:firstrow(test.t.id)->test.t.id",
-          "          └─ExchangeReceiver 10000.00 mpp[tiflash]  ",
-          "            └─ExchangeSender 10000.00 mpp[tiflash]  ExchangeType: HashPartition, Hash Cols: [name: test.t.name, collate: utf8mb4_bin], [name: test.t.id, collate: binary]",


Expected, 2PhaseAgg is safer than 1PhaseAgg.

qw4990 · 2022-11-08T05:25:13Z

planner/core/testdata/plan_suite_out.json

@@ -477,7 +477,7 @@
      },
      {
        "SQL": "select c from t where t.c = 1 and t.d = 1 order by t.a limit 1",
-        "Best": "IndexReader(Index(t.c_d_e)[[1 1,1 1]])->TopN([test.t.a],0,1)->Projection"


Expected, pushing it down is safer.

qw4990 · 2022-11-08T05:26:00Z

planner/core/testdata/plan_suite_out.json

@@ -1588,7 +1588,7 @@
    "Cases": [
      {
        "SQL": "select t1.a, (select count(t2.a) from t t2 where t2.g in (select t3.d from t t3 where t3.c = t1.a)) as agg_col from t t1;",
-        "Best": "Apply{IndexReader(Index(t.f)[[NULL,+inf]])->IndexHashJoin{IndexReader(Index(t.c_d_e)[[NULL,+inf]]->HashAgg)->HashAgg->IndexReader(Index(t.g)[[NULL,NULL]])}(test.t.d,test.t.g)}->HashAgg"


Expected, IndexJoin and IndexHashJoin have no difference in model2, and whether to push the Agg down will be solved by #38874

qw4990 · 2022-11-08T07:41:16Z

/merge

ti-chi-bot · 2022-11-08T07:41:20Z

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

Commit hash: 2ddd74d

sre-bot · 2022-11-08T08:53:38Z

TiDB MergeCI notify

🔴 Bad News! New failing [3] after this pr merged.
These new failed integration tests seem to be caused by the current PR, please try to fix these new failed integration tests, thanks!

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/integration-common-test	🟥 failed 1, success 16, total 17	14 min	New failing
idc-jenkins-ci-tidb/sqllogic-test-1	🟥 failed 3, success 23, total 26	5 min 33 sec	New failing
idc-jenkins-ci-tidb/sqllogic-test-2	🟥 failed 2, success 26, total 28	5 min 22 sec	New failing
idc-jenkins-ci-tidb/integration-ddl-test	🔴 failed 1, success 5, total 6	43 min	Existing failure
idc-jenkins-ci-tidb/mybatis-test	🔴 failed 1, success 0, total 1	11 min	Existing failure
idc-jenkins-ci/integration-cdc-test	✅ all 39 tests passed	22 min	Fixed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	18 min	Existing passed
idc-jenkins-ci-tidb/common-test	🟢 all 11 tests passed	10 min	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	9 min 33 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

fixup

ecedb0e

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 8, 2022

qw4990 added sig/planner SIG: Planner epic/cost-model the optimizer cost model and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 8, 2022

qw4990 commented Nov 8, 2022

View reviewed changes

fixup

ecbb043

ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 8, 2022

qw4990 commented Nov 8, 2022

View reviewed changes

qw4990 requested review from time-and-fate and Reminiscent November 8, 2022 05:58

Reminiscent approved these changes Nov 8, 2022

View reviewed changes

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

qw4990 added 2 commits November 8, 2022 14:10

fixup

611d317

fixup

66481bf

ti-chi-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 Nov 8, 2022

qw4990 added 2 commits November 8, 2022 14:47

Merge branch 'master' into model2-ut

8decf84

fixup

aabddd0

time-and-fate approved these changes Nov 8, 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 8, 2022

Merge branch 'master' into model2-ut

2ddd74d

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

chrysan approved these changes Nov 8, 2022

View reviewed changes

ti-chi-bot merged commit 3208719 into pingcap:master Nov 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

planner: update some UTs from cost model1 to model2 #38959

planner: update some UTs from cost model1 to model2 #38959

qw4990 commented Nov 8, 2022

ti-chi-bot commented Nov 8, 2022 •

edited

Loading

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 Nov 8, 2022

qw4990 commented Nov 8, 2022

ti-chi-bot commented Nov 8, 2022

sre-bot commented Nov 8, 2022

planner: update some UTs from cost model1 to model2 #38959

planner: update some UTs from cost model1 to model2 #38959

Conversation

qw4990 commented Nov 8, 2022

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 8, 2022 • edited Loading

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 Nov 8, 2022

Choose a reason for hiding this comment

qw4990 commented Nov 8, 2022

ti-chi-bot commented Nov 8, 2022

sre-bot commented Nov 8, 2022

TiDB MergeCI notify

ti-chi-bot commented Nov 8, 2022 •

edited

Loading