-
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: update some UTs from cost model1 to model2 #39065
Conversation
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
executor/explain_test.go
Outdated
@@ -363,7 +364,7 @@ func TestCheckActRowsWithUnistore(t *testing.T) { | |||
}, | |||
{ | |||
sql: "select count(*) from t_unistore_act_rows group by b", | |||
expected: []string{"2", "2", "2", "4"}, | |||
expected: []string{"2", "4", "4"}, |
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.
Expected, the Agg is not pushed down in model2 since there are only 4 rows.
@@ -990,8 +991,8 @@ func TestSPM4PlanCache(t *testing.T) { | |||
tk.MustExec("admin reload bindings;") | |||
|
|||
res := tk.MustQuery("explain format = 'brief' select * from t;") | |||
require.Regexp(t, ".*TableReader.*", res.Rows()[0][0]) |
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.
Expected, IndexScan is always better than TableScan.
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")) |
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.
Expected, model2 prefers to use Scan instead of Lookup to avoid triggering too many double-read requests.
@@ -3571,22 +3571,20 @@ func TestPartitionTableExplain(t *testing.T) { | |||
"└─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)", |
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.
Expected, model2 prefers to avoid using double-read IndexJoin.
@@ -88,16 +89,14 @@ func TestRangeColumnPartitionPruningForIn(t *testing.T) { | |||
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", |
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.
Expected, it will resolved by #38874.
@@ -46,15 +46,15 @@ | |||
"children": [ | |||
{ | |||
"name": "TableFullScan_4", | |||
"cost": 570000, |
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.
Expected, no plan change, just update cost values.
@@ -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)]", |
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.
Expected, just update HashJoin build and probe sides since model2 takes row-size into consideration when calculating HashJoin costs.
/run-build |
/run-check_dev |
/run-all-tests |
/run-unit-test |
@@ -9,10 +9,10 @@ | |||
"└─Apply 2.00 root CARTESIAN left outer semi join, other cond:eq(test.t.e, Column#26)", | |||
" ├─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", |
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.
Expected, HashAgg is better than StreamAgg, IndexScan is better than TableScan.
" └─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", |
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.
Expected, pushing it down can reduce the risks of under-estimation.
"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", |
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.
Expected, HashAgg is better.
"│ └─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", |
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.
Expected, pushing the Agg down is safer.
/merge |
This pull request has been accepted and is ready to merge. Commit hash: 8a398a4
|
TiDB MergeCI notify🔴 Bad News! [4] CI still failing after this pr merged.
|
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
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.