Skip to content
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.

Sign up for GitHub

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

Jump to bottom

planner, sessionctx: enable indexMerge by default #29617

Closed
wants to merge 2 commits into from
Closed

planner, sessionctx: enable indexMerge by default #29617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
34c07a8
enable tidb_enable_index_merge by default
Yui-Song Nov 9, 2021
1c6ddda
Update explain_indexmerge.result
Yui-Song Nov 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions cmd/explaintest/r/explain_indexmerge.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,23 @@ create index td on t (d);
load stats 's/explain_indexmerge_stats_t.json';
explain format = 'brief' select * from t where a < 50 or b < 50;
id estRows task access object operator info
TableReader 98.00 root data:Selection
└─Selection 98.00 cop[tikv] or(lt(test.t.a, 50), lt(test.t.b, 50))
└─TableFullScan 5000000.00 cop[tikv] table:t keep order:false
IndexMerge 98.00 root
├─TableRangeScan(Build) 49.00 cop[tikv] table:t range:[-inf,50), keep order:false
├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false
└─TableRowIDScan(Probe) 98.00 cop[tikv] table:t keep order:false
explain format = 'brief' select * from t where (a < 50 or b < 50) and f > 100;
id estRows task access object operator info
TableReader 98.00 root data:Selection
└─Selection 98.00 cop[tikv] gt(test.t.f, 100), or(lt(test.t.a, 50), lt(test.t.b, 50))
└─TableFullScan 5000000.00 cop[tikv] table:t keep order:false
IndexMerge 98.00 root
├─TableRangeScan(Build) 49.00 cop[tikv] table:t range:[-inf,50), keep order:false
├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false
└─Selection(Probe) 98.00 cop[tikv] gt(test.t.f, 100)
└─TableRowIDScan 98.00 cop[tikv] table:t keep order:false
explain format = 'brief' select * from t where b < 50 or c < 50;
id estRows task access object operator info
TableReader 98.00 root data:Selection
└─Selection 98.00 cop[tikv] or(lt(test.t.b, 50), lt(test.t.c, 50))
└─TableFullScan 5000000.00 cop[tikv] table:t keep order:false
IndexMerge 98.00 root
├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tb(b) range:[-inf,50), keep order:false
├─IndexRangeScan(Build) 49.00 cop[tikv] table:t, index:tc(c) range:[-inf,50), keep order:false
└─TableRowIDScan(Probe) 98.00 cop[tikv] table:t keep order:false
set session tidb_enable_index_merge = on;
explain format = 'brief' select * from t where a < 50 or b < 50;
id estRows task access object operator info
Expand Down
23 changes: 13 additions & 10 deletions planner/core/testdata/integration_suite_out.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -911,14 +911,16 @@
"PartitionUnion 59.97 root ",
"├─IndexMerge 19.99 root ",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p0, index:b(b) range:[1,1], keep order:false, stats:pseudo",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p0, index:c(c) range:[\"8\",\"8\"], keep order:false, stats:pseudo",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p0, index:c(c) range:[\"8\",\"8\"], keep order:false, stats:pseudo" ,
"│ └─TableRowIDScan(Probe) 19.99 cop[tikv] table:t, partition:p0 keep order:false, stats:pseudo",
"├─TableReader 19.99 root data:Selection",
"│ └─Selection 19.99 cop[tiflash] or(eq(test.t.b, 1), eq(test.t.c, \"8\"))",
"│ └─TableFullScan 10000.00 cop[tiflash] table:t, partition:p1 keep order:false, stats:pseudo",
"└─TableReader 19.99 root data:Selection",
" └─Selection 19.99 cop[tiflash] or(eq(test.t.b, 1), eq(test.t.c, \"8\"))",
" └─TableFullScan 10000.00 cop[tiflash] table:t, partition:p2 keep order:false, stats:pseudo"
"├─IndexMerge 19.99 root ",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p1, index:b(b) range:[1,1], keep order:false, stats:pseudo",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p1, index:c(c) range:[\"8\",\"8\"], keep order:false, stats:pseudo" ,
"│ └─TableRowIDScan(Probe) 19.99 cop[tikv] table:t, partition:p1 keep order:false, stats:pseudo",
"└─IndexMerge 19.99 root ",
" ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p2, index:b(b) range:[1,1], keep order:false, stats:pseudo",
" ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p2, index:c(c) range:[\"8\",\"8\"], keep order:false, stats:pseudo",
" └─TableRowIDScan(Probe) 19.99 cop[tikv] table:t, partition:p2 keep order:false, stats:pseudo"
],
"Warn": null
},
Expand All @@ -934,9 +936,10 @@
"│ ├─TableRangeScan(Build) 1.00 cop[tikv] table:t, partition:p1 range:[1,1], keep order:false, stats:pseudo",
"│ ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p1, index:b(b) range:[2,2], keep order:false, stats:pseudo",
"│ └─TableRowIDScan(Probe) 11.00 cop[tikv] table:t, partition:p1 keep order:false, stats:pseudo",
"└─TableReader 11.00 root data:Selection",
" └─Selection 11.00 cop[tiflash] or(eq(test.t.a, 1), eq(test.t.b, 2))",
" └─TableFullScan 10000.00 cop[tiflash] table:t, partition:p2 keep order:false, stats:pseudo"
"└─IndexMerge 11.00 root ",
" ├─TableRangeScan(Build) 1.00 cop[tikv] table:t, partition:p2 range:[1,1], keep order:false, stats:pseudo",
" ├─IndexRangeScan(Build) 10.00 cop[tikv] table:t, partition:p2, index:b(b) range:[2,2], keep order:false, stats:pseudo",
" └─TableRowIDScan(Probe) 11.00 cop[tikv] table:t, partition:p2 keep order:false, stats:pseudo"
],
"Warn": null
},
Expand Down
12 changes: 6 additions & 6 deletions planner/core/testdata/plan_suite_out.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,9 +286,9 @@
},
{
"SQL": "select /*+ USE_INDEX_MERGE(t1, c_d_e, f_g) */ * from t where c < 1 or f > 2",
"Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))",
"Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))",
"HasWarn": true,
"Hints": "use_index(@`sel_1` `test`.`t` )"
"Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)"
},
{
"SQL": "select /*+ NO_INDEX_MERGE(), USE_INDEX_MERGE(t, primary, f_g, c_d_e) */ * from t where a < 1 or f > 2",
Expand All @@ -304,15 +304,15 @@
},
{
"SQL": "select /*+ USE_INDEX_MERGE(db2.t) */ * from t where c < 1 or f > 2",
"Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))",
"Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))",
"HasWarn": true,
"Hints": "use_index(@`sel_1` `test`.`t` )"
"Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)"
},
{
"SQL": "select /*+ USE_INDEX_MERGE(db2.t, c_d_e, f_g) */ * from t where c < 1 or f > 2",
"Best": "TableReader(Table(t)->Sel([or(lt(test.t.c, 1), gt(test.t.f, 2))]))",
"Best": "IndexMergeReader(PartialPlans->[Index(t.c_d_e)[[-inf,1)], Index(t.f)[(2,+inf]]], TablePlan->Table(t))",
"HasWarn": true,
"Hints": "use_index(@`sel_1` `test`.`t` )"
"Hints": "use_index_merge(@`sel_1` `t` `c_d_e`, `f`)"
}
]
},
Expand Down
2 changes: 1 addition & 1 deletion sessionctx/variable/sysvar.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,7 @@ var defaultSysVars = []*SysVar{
s.SetEnableCascadesPlanner(TiDBOptOn(val))
return nil
}},
{Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMerge, Value: Off, Type: TypeBool, SetSession: func(s *SessionVars, val string) error {
{Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMerge, Value: On, Type: TypeBool, SetSession: func(s *SessionVars, val string) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only change the default for new installations. I believe this is fine, but I will wait for you to confirm (and update the release note).

If you'd like to change for existing installs, a bootstrap task is needed (session/bootstrap.go).

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(1) Please make sure this works for 4.0 to 5.0 upgrade too, since from analyze-2.0 feature we found it only works for intra-5.0 upgrade but does not work for the 4.0 to 5.0 upgrade which changes the default anyway no matter it is existing or new installs;
(2) Also @guo-shaoge is working on GA index merge in Sprint 6. I'd suggest to make this change together with index merge GA feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just change the default for new installations is enough. Change for existing installs will results in plan changes, which will give the user "surprise". That's not what I expected.
And I will talk to @guo-shaoge later.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is that approach we identify whether it's an existing or new install only works for 5.x but not 4.x, which means all the existing system from 4.x will be impacted and default will be changed after upgrading to 5.x.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. config, session: promise the compatibility of oom-action when upgrading #22102 changed the default oom-action, which works for all situations(3.0 to 4.0 and 4.0 to 5.0).
    But oom-action is a config item, I am not sure it works for sysvars.
  2. IMO, it's better to enable index merge after it's official GA. We are working on it.

s.SetEnableIndexMerge(TiDBOptOn(val))
return nil
}},
Expand Down