From f2a51937ecb91cd92b6f1f9054dfd7a65a6c6021 Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Thu, 31 Aug 2023 09:48:10 -0700 Subject: [PATCH 01/13] init commit --- .../casetest/rule/rule_join_reorder_test.go | 13 ++ .../rule/testdata/join_reorder_suite_in.json | 16 ++ .../rule/testdata/join_reorder_suite_out.json | 161 ++++++++++++++++++ planner/core/exhaust_physical_plans.go | 14 +- sessionctx/variable/session.go | 3 + sessionctx/variable/sysvar.go | 4 + sessionctx/variable/tidb_vars.go | 4 + 7 files changed, 209 insertions(+), 6 deletions(-) diff --git a/planner/core/casetest/rule/rule_join_reorder_test.go b/planner/core/casetest/rule/rule_join_reorder_test.go index 5701fcae99279..985a22896f016 100644 --- a/planner/core/casetest/rule/rule_join_reorder_test.go +++ b/planner/core/casetest/rule/rule_join_reorder_test.go @@ -72,6 +72,19 @@ func TestNoHashJoinHint(t *testing.T) { runJoinReorderTestData(t, tk, "TestNoHashJoinHint") } +// test the global/session variable tidb_opt_enable_hash_join being set to no +func TestOptEnableHashJoin(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("set tidb_opt_enable_hash_join=no") + tk.MustExec("create table t1(a int, b int, key(a));") + tk.MustExec("create table t2(a int, b int, key(a));") + tk.MustExec("create table t3(a int, b int, key(a));") + tk.MustExec("create table t4(a int, b int, key(a));") + runJoinReorderTestData(t, tk, "TestOptEnableHashJoin") +} + func TestNoMergeJoinHint(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) diff --git a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json index 1201331e17a43..3bcae1a99cba0 100644 --- a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json +++ b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json @@ -49,6 +49,22 @@ "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), no_hash_join(t3), hash_join(t4) */ * from t1, t2, t3, t4" ] }, + { + "name": "TestOptEnableHashJoin", + "cases": [ + "select /*+ hash_join(t1) */ * from t1, t2", + "select * from t1, t2", + "select * from t1, t2 where t1.a=t2.a", + "select * from t1, t2 where t1.b=t2.b", + "select * from t1, t2 where t1.a=t2.a and t1.b=t2.b", + "select * from t1 left join t2 on t1.b=t2.b", + "select * from t1 left join t2 on t1.a=t2.a", + "select * from t1 right join t2 on t1.b=t2.b", + "select * from t1 right join t2 on t1.a=t2.a", + "select /*+ leading(t4, t3, t2, t1) */ * from t1, t2, t3, t4", + "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), hash_join(t4) */ * from t1, t2, t3, t4" + ] + }, { "name": "TestNoIndexJoinHint", "cases": [ diff --git a/planner/core/casetest/rule/testdata/join_reorder_suite_out.json b/planner/core/casetest/rule/testdata/join_reorder_suite_out.json index fd4c581d478ee..83f9f9cc0d88c 100644 --- a/planner/core/casetest/rule/testdata/join_reorder_suite_out.json +++ b/planner/core/casetest/rule/testdata/join_reorder_suite_out.json @@ -782,6 +782,167 @@ } ] }, + { + "Name": "TestOptEnableHashJoin", + "Cases": [ + { + "SQL": "select * from t1, t2", + "Plan": [ + "MergeJoin 100000000.00 root inner join", + "├─TableReader(Build) 10000.00 root data:TableFullScan", + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─TableReader(Probe) 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1, t2 where t1.a=t2.a", + "Plan": [ + "IndexHashJoin 12487.50 root inner join, inner:IndexLookUp, outer key:test.t1.a, inner key:test.t2.a, equal cond:eq(test.t1.a, test.t2.a)", + "├─TableReader(Build) 9990.00 root data:Selection", + "│ └─Selection 9990.00 cop[tikv] not(isnull(test.t1.a))", + "│ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", + "└─IndexLookUp(Probe) 12487.50 root ", + " ├─Selection(Build) 12487.50 cop[tikv] not(isnull(test.t2.a))", + " │ └─IndexRangeScan 12500.00 cop[tikv] table:t2, index:a(a) range: decided by [eq(test.t2.a, test.t1.a)], keep order:false, stats:pseudo", + " └─TableRowIDScan(Probe) 12487.50 cop[tikv] table:t2 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1, t2 where t1.b=t2.b", + "Plan": [ + "MergeJoin 12487.50 root inner join, left key:test.t1.b, right key:test.t2.b", + "├─Sort(Build) 9990.00 root test.t2.b", + "│ └─TableReader 9990.00 root data:Selection", + "│ └─Selection 9990.00 cop[tikv] not(isnull(test.t2.b))", + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─Sort(Probe) 9990.00 root test.t1.b", + " └─TableReader 9990.00 root data:Selection", + " └─Selection 9990.00 cop[tikv] not(isnull(test.t1.b))", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1, t2 where t1.a=t2.a and t1.b=t2.b", + "Plan": [ + "IndexHashJoin 12475.01 root inner join, inner:IndexLookUp, outer key:test.t1.a, inner key:test.t2.a, equal cond:eq(test.t1.a, test.t2.a), eq(test.t1.b, test.t2.b)", + "├─TableReader(Build) 9980.01 root data:Selection", + "│ └─Selection 9980.01 cop[tikv] not(isnull(test.t1.a)), not(isnull(test.t1.b))", + "│ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", + "└─IndexLookUp(Probe) 12475.01 root ", + " ├─Selection(Build) 12487.50 cop[tikv] not(isnull(test.t2.a))", + " │ └─IndexRangeScan 12500.00 cop[tikv] table:t2, index:a(a) range: decided by [eq(test.t2.a, test.t1.a)], keep order:false, stats:pseudo", + " └─Selection(Probe) 12475.01 cop[tikv] not(isnull(test.t2.b))", + " └─TableRowIDScan 12487.50 cop[tikv] table:t2 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1 left join t2 on t1.b=t2.b", + "Plan": [ + "MergeJoin 12487.50 root left outer join, left key:test.t1.b, right key:test.t2.b", + "├─Sort(Build) 9990.00 root test.t2.b", + "│ └─TableReader 9990.00 root data:Selection", + "│ └─Selection 9990.00 cop[tikv] not(isnull(test.t2.b))", + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─Sort(Probe) 10000.00 root test.t1.b", + " └─TableReader 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1 left join t2 on t1.a=t2.a", + "Plan": [ + "IndexHashJoin 12487.50 root left outer join, inner:IndexLookUp, outer key:test.t1.a, inner key:test.t2.a, equal cond:eq(test.t1.a, test.t2.a)", + "├─TableReader(Build) 10000.00 root data:TableFullScan", + "│ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", + "└─IndexLookUp(Probe) 12487.50 root ", + " ├─Selection(Build) 12487.50 cop[tikv] not(isnull(test.t2.a))", + " │ └─IndexRangeScan 12500.00 cop[tikv] table:t2, index:a(a) range: decided by [eq(test.t2.a, test.t1.a)], keep order:false, stats:pseudo", + " └─TableRowIDScan(Probe) 12487.50 cop[tikv] table:t2 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1 right join t2 on t1.b=t2.b", + "Plan": [ + "MergeJoin 12487.50 root right outer join, left key:test.t1.b, right key:test.t2.b", + "├─Sort(Build) 9990.00 root test.t1.b", + "│ └─TableReader 9990.00 root data:Selection", + "│ └─Selection 9990.00 cop[tikv] not(isnull(test.t1.b))", + "│ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", + "└─Sort(Probe) 10000.00 root test.t2.b", + " └─TableReader 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select * from t1 right join t2 on t1.a=t2.a", + "Plan": [ + "IndexHashJoin 12487.50 root right outer join, inner:IndexLookUp, outer key:test.t2.a, inner key:test.t1.a, equal cond:eq(test.t2.a, test.t1.a)", + "├─TableReader(Build) 10000.00 root data:TableFullScan", + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─IndexLookUp(Probe) 12487.50 root ", + " ├─Selection(Build) 12487.50 cop[tikv] not(isnull(test.t1.a))", + " │ └─IndexRangeScan 12500.00 cop[tikv] table:t1, index:a(a) range: decided by [eq(test.t1.a, test.t2.a)], keep order:false, stats:pseudo", + " └─TableRowIDScan(Probe) 12487.50 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select /*+ hash_join(t1) */ * from t1, t2", + "Plan": [ + "HashJoin 100000000.00 root CARTESIAN inner join", + "├─TableReader(Build) 10000.00 root data:TableFullScan", + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─TableReader(Probe) 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select /*+ leading(t4, t3, t2, t1) */ * from t1, t2, t3, t4", + "Plan": [ + "Projection 10000000000000000.00 root test.t1.a, test.t1.b, test.t2.a, test.t2.b, test.t3.a, test.t3.b, test.t4.a, test.t4.b", + "└─HashJoin 10000000000000000.00 root CARTESIAN inner join", + " ├─TableReader(Build) 10000.00 root data:TableFullScan", + " │ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", + " └─MergeJoin(Probe) 1000000000000.00 root inner join", + " ├─TableReader(Build) 10000.00 root data:TableFullScan", + " │ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + " └─MergeJoin(Probe) 100000000.00 root inner join", + " ├─TableReader(Build) 10000.00 root data:TableFullScan", + " │ └─TableFullScan 10000.00 cop[tikv] table:t3 keep order:false, stats:pseudo", + " └─TableReader(Probe) 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t4 keep order:false, stats:pseudo" + ], + "Warning": null + }, + { + "SQL": "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), hash_join(t4) */ * from t1, t2, t3, t4", + "Plan": [ + "HashJoin 10000000000000000.00 root CARTESIAN inner join", + "├─TableReader(Build) 10000.00 root data:TableFullScan", + "│ └─TableFullScan 10000.00 cop[tikv] table:t4 keep order:false, stats:pseudo", + "└─MergeJoin(Probe) 1000000000000.00 root inner join", + " ├─TableReader(Build) 10000.00 root data:TableFullScan", + " │ └─TableFullScan 10000.00 cop[tikv] table:t3 keep order:false, stats:pseudo", + " └─HashJoin(Probe) 100000000.00 root CARTESIAN inner join", + " ├─TableReader(Build) 10000.00 root data:TableFullScan", + " │ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + " └─TableReader(Probe) 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + ], + "Warning": null + } + ] + }, + { "Name": "TestNoIndexJoinHint", "Cases": [ diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 750d9e60e359d..0fda7bc93e14c 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -45,7 +45,6 @@ import ( "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/set" "github.com/pingcap/tidb/util/size" - "github.com/pingcap/tipb/go-tipb" "go.uber.org/atomic" "go.uber.org/zap" ) @@ -244,7 +243,7 @@ func (p *LogicalJoin) GetMergeJoin(prop *property.PhysicalProperty, schema *expr // If TiDB_SMJ hint is existed, it should consider enforce merge join, // because we can't trust lhsChildProperty completely. if (p.preferJoinType&preferMergeJoin) > 0 || - (p.preferJoinType&preferNoHashJoin) > 0 { // if hash join is not allowed, generate as many other types of join as possible to avoid 'cant-find-plan' error. + p.shouldSkipHashJoin() { // if hash join is not allowed, generate as many other types of join as possible to avoid 'cant-find-plan' error. joins = append(joins, p.getEnforcedMergeJoin(prop, schema, statsInfo)...) } @@ -391,6 +390,10 @@ var ForceUseOuterBuild4Test = atomic.NewBool(false) // TODO: use hint and remove this variable var ForcedHashLeftJoin4Test = atomic.NewBool(false) +func (p *LogicalJoin) shouldSkipHashJoin() bool { + return (p.preferJoinType&preferNoHashJoin) > 0 || (p.SCtx().GetSessionVars().EnableHashJoin == false) +} + func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []PhysicalPlan, forced bool) { if !prop.IsSortItemEmpty() { // hash join doesn't promise any orders return @@ -450,11 +453,10 @@ func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []Phy } } - forced = (p.preferJoinType&preferHashJoin > 0) || forceLeftToBuild || forceRightToBuild - noHashJoin := (p.preferJoinType & preferNoHashJoin) > 0 - if !forced && noHashJoin { + forced = forceLeftToBuild || forceRightToBuild + if !forced && p.shouldSkipHashJoin() { return nil, false - } else if forced && noHashJoin { + } else if forced && p.shouldSkipHashJoin() { p.SCtx().GetSessionVars().StmtCtx.AppendWarning(ErrInternal.GenWithStack( "Some HASH_JOIN and NO_HASH_JOIN hints conflict, NO_HASH_JOIN is ignored")) } diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index f14cb85b4f919..6fe2b54ce6661 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -1238,6 +1238,9 @@ type SessionVars struct { // AnalyzeVersion indicates how TiDB collect and use analyzed statistics. AnalyzeVersion int + // EnableHashjoin indicates whether to enable hash join. + EnableHashJoin bool + // EnableIndexMergeJoin indicates whether to enable index merge join. EnableIndexMergeJoin bool diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 5292d6badbfdb..4408cbd4b4409 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -2021,6 +2021,10 @@ var defaultSysVars = []*SysVar{ s.AnalyzeVersion = tidbOptPositiveInt32(val, DefTiDBAnalyzeVersion) return nil }}, + {Scope: ScopeGlobal | ScopeSession, Name: TiDBOptEnableHashJoin, Value: BoolToOnOff(DefTiDBOptEnableHashJoin), Type: TypeBool, SetSession: func(s *SessionVars, val string) error { + s.EnableHashJoin = TiDBOptOn(val) + return nil + }}, {Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMergeJoin, Value: BoolToOnOff(DefTiDBEnableIndexMergeJoin), Hidden: true, Type: TypeBool, SetSession: func(s *SessionVars, val string) error { s.EnableIndexMergeJoin = TiDBOptOn(val) return nil diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index 381c86c39daa6..6591a5d058d0c 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/sessionctx/variable/tidb_vars.go @@ -904,6 +904,9 @@ const ( // TiDBEnableCheckConstraint indicates whether to enable check constraint feature. TiDBEnableCheckConstraint = "tidb_enable_check_constraint" + + // TiDBOptEnableHashJoin indicates whether to enable hash join. + TiDBOptEnableHashJoin = "tidb_opt_enable_hash_join" ) // TiDB vars that have only global scope @@ -1389,6 +1392,7 @@ const ( DefTiDBLockUnchangedKeys = true DefTiDBEnableCheckConstraint = false DefTiDBSkipMissingPartitionStats = true + DefTiDBOptEnableHashJoin = true ) // Process global variables. From e48b5653e645e9eb00c006741734ca734bd99916 Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Tue, 5 Sep 2023 09:25:04 -0700 Subject: [PATCH 02/13] revert the library importing --- planner/core/exhaust_physical_plans.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index ce7cc7e1ecea6..cb48006880133 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -45,6 +45,7 @@ import ( "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/set" "github.com/pingcap/tidb/util/size" + "github.com/pingcap/tipb/go-tipb" "go.uber.org/atomic" "go.uber.org/zap" ) From ac6748e3284d34942cb8434834d24cd6f3cd3260 Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Tue, 5 Sep 2023 09:35:29 -0700 Subject: [PATCH 03/13] revert the library importing --- planner/core/exhaust_physical_plans.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index cb48006880133..7f266e127ffa5 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -45,7 +45,7 @@ import ( "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/set" "github.com/pingcap/tidb/util/size" - "github.com/pingcap/tipb/go-tipb" + "github.com/pingcap/tipb/go-tipb" "go.uber.org/atomic" "go.uber.org/zap" ) From 8b3abc50d24786d707f9c9aa5aa4ac3a81622024 Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Tue, 12 Sep 2023 14:32:13 -0700 Subject: [PATCH 04/13] Address feedbacks --- planner/core/casetest/rule/rule_join_reorder_test.go | 2 +- planner/core/casetest/rule/testdata/join_reorder_suite_in.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/planner/core/casetest/rule/rule_join_reorder_test.go b/planner/core/casetest/rule/rule_join_reorder_test.go index 985a22896f016..07138b9220230 100644 --- a/planner/core/casetest/rule/rule_join_reorder_test.go +++ b/planner/core/casetest/rule/rule_join_reorder_test.go @@ -77,7 +77,7 @@ func TestOptEnableHashJoin(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) tk.MustExec("use test") - tk.MustExec("set tidb_opt_enable_hash_join=no") + tk.MustExec("set tidb_opt_enable_hash_join=off") tk.MustExec("create table t1(a int, b int, key(a));") tk.MustExec("create table t2(a int, b int, key(a));") tk.MustExec("create table t3(a int, b int, key(a));") diff --git a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json index 3bcae1a99cba0..460f92fbcdf98 100644 --- a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json +++ b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json @@ -52,7 +52,6 @@ { "name": "TestOptEnableHashJoin", "cases": [ - "select /*+ hash_join(t1) */ * from t1, t2", "select * from t1, t2", "select * from t1, t2 where t1.a=t2.a", "select * from t1, t2 where t1.b=t2.b", @@ -61,6 +60,7 @@ "select * from t1 left join t2 on t1.a=t2.a", "select * from t1 right join t2 on t1.b=t2.b", "select * from t1 right join t2 on t1.a=t2.a", + "select /*+ hash_join(t1) */ * from t1, t2", "select /*+ leading(t4, t3, t2, t1) */ * from t1, t2, t3, t4", "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), hash_join(t4) */ * from t1, t2, t3, t4" ] From cdf1010cc33efcc5d38b364270ad07663e83a113 Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Thu, 21 Sep 2023 19:47:26 -0700 Subject: [PATCH 05/13] address the issue after merge --- .../rule/testdata/join_reorder_suite_in.json | 3 +- .../rule/testdata/join_reorder_suite_out.json | 46 ++++++------------- planner/core/exhaust_physical_plans.go | 3 +- 3 files changed, 16 insertions(+), 36 deletions(-) diff --git a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json index 460f92fbcdf98..c3c3ece36dba5 100644 --- a/planner/core/casetest/rule/testdata/join_reorder_suite_in.json +++ b/planner/core/casetest/rule/testdata/join_reorder_suite_in.json @@ -61,8 +61,7 @@ "select * from t1 right join t2 on t1.b=t2.b", "select * from t1 right join t2 on t1.a=t2.a", "select /*+ hash_join(t1) */ * from t1, t2", - "select /*+ leading(t4, t3, t2, t1) */ * from t1, t2, t3, t4", - "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), hash_join(t4) */ * from t1, t2, t3, t4" + "select /*+ hash_join(t2) */ * from t1, t2" ] }, { diff --git a/planner/core/casetest/rule/testdata/join_reorder_suite_out.json b/planner/core/casetest/rule/testdata/join_reorder_suite_out.json index 83f9f9cc0d88c..9a5eb9881759f 100644 --- a/planner/core/casetest/rule/testdata/join_reorder_suite_out.json +++ b/planner/core/casetest/rule/testdata/join_reorder_suite_out.json @@ -609,7 +609,7 @@ " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" ], "Warning": [ - "Warning 1815 Some HASH_JOIN and NO_HASH_JOIN hints conflict, NO_HASH_JOIN is ignored" + "Warning 1815 A conflict between the HASH_JOIN hint and the NO_HASH_JOIN hint, or the tidb_opt_enable_hash_join system variable, the HASH_JOIN hint will take precedence." ] }, { @@ -622,7 +622,7 @@ " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" ], "Warning": [ - "Warning 1815 Some HASH_JOIN and NO_HASH_JOIN hints conflict, NO_HASH_JOIN is ignored" + "Warning 1815 A conflict between the HASH_JOIN hint and the NO_HASH_JOIN hint, or the tidb_opt_enable_hash_join system variable, the HASH_JOIN hint will take precedence." ] }, { @@ -903,42 +903,22 @@ "└─TableReader(Probe) 10000.00 root data:TableFullScan", " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" ], - "Warning": null - }, - { - "SQL": "select /*+ leading(t4, t3, t2, t1) */ * from t1, t2, t3, t4", - "Plan": [ - "Projection 10000000000000000.00 root test.t1.a, test.t1.b, test.t2.a, test.t2.b, test.t3.a, test.t3.b, test.t4.a, test.t4.b", - "└─HashJoin 10000000000000000.00 root CARTESIAN inner join", - " ├─TableReader(Build) 10000.00 root data:TableFullScan", - " │ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo", - " └─MergeJoin(Probe) 1000000000000.00 root inner join", - " ├─TableReader(Build) 10000.00 root data:TableFullScan", - " │ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", - " └─MergeJoin(Probe) 100000000.00 root inner join", - " ├─TableReader(Build) 10000.00 root data:TableFullScan", - " │ └─TableFullScan 10000.00 cop[tikv] table:t3 keep order:false, stats:pseudo", - " └─TableReader(Probe) 10000.00 root data:TableFullScan", - " └─TableFullScan 10000.00 cop[tikv] table:t4 keep order:false, stats:pseudo" - ], - "Warning": null + "Warning": [ + "Warning 1815 A conflict between the HASH_JOIN hint and the NO_HASH_JOIN hint, or the tidb_opt_enable_hash_join system variable, the HASH_JOIN hint will take precedence." + ] }, { - "SQL": "select /*+ leading(t1, t2, t3, t4), hash_join(t1, t2), hash_join(t4) */ * from t1, t2, t3, t4", + "SQL": "select /*+ hash_join(t2) */ * from t1, t2", "Plan": [ - "HashJoin 10000000000000000.00 root CARTESIAN inner join", + "HashJoin 100000000.00 root CARTESIAN inner join", "├─TableReader(Build) 10000.00 root data:TableFullScan", - "│ └─TableFullScan 10000.00 cop[tikv] table:t4 keep order:false, stats:pseudo", - "└─MergeJoin(Probe) 1000000000000.00 root inner join", - " ├─TableReader(Build) 10000.00 root data:TableFullScan", - " │ └─TableFullScan 10000.00 cop[tikv] table:t3 keep order:false, stats:pseudo", - " └─HashJoin(Probe) 100000000.00 root CARTESIAN inner join", - " ├─TableReader(Build) 10000.00 root data:TableFullScan", - " │ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", - " └─TableReader(Probe) 10000.00 root data:TableFullScan", - " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" + "│ └─TableFullScan 10000.00 cop[tikv] table:t2 keep order:false, stats:pseudo", + "└─TableReader(Probe) 10000.00 root data:TableFullScan", + " └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo" ], - "Warning": null + "Warning": [ + "Warning 1815 A conflict between the HASH_JOIN hint and the NO_HASH_JOIN hint, or the tidb_opt_enable_hash_join system variable, the HASH_JOIN hint will take precedence." + ] } ] }, diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 7f266e127ffa5..bb742cb7e061a 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -459,7 +459,8 @@ func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []Phy return nil, false } else if forced && p.shouldSkipHashJoin() { p.SCtx().GetSessionVars().StmtCtx.AppendWarning(ErrInternal.GenWithStack( - "Some HASH_JOIN and NO_HASH_JOIN hints conflict, NO_HASH_JOIN is ignored")) + "A conflict between the HASH_JOIN hint and the NO_HASH_JOIN hint, " + + "or the tidb_opt_enable_hash_join system variable, the HASH_JOIN hint will take precedence.")) } return } From eb9850aec790f1d06eb5ca9598aa3e52b557a43c Mon Sep 17 00:00:00 2001 From: Min Zhou Date: Thu, 21 Sep 2023 20:03:42 -0700 Subject: [PATCH 06/13] fix a bug --- planner/core/exhaust_physical_plans.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index bb742cb7e061a..454099f199d68 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -454,7 +454,7 @@ func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []Phy } } - forced = forceLeftToBuild || forceRightToBuild + forced = (p.preferJoinType&preferHashJoin > 0) || forceLeftToBuild || forceRightToBuild if !forced && p.shouldSkipHashJoin() { return nil, false } else if forced && p.shouldSkipHashJoin() { From cae12dce64816cfcbff239cb2078445fa3cd2852 Mon Sep 17 00:00:00 2001 From: Yuanjia Zhang Date: Mon, 25 Sep 2023 12:52:03 +0800 Subject: [PATCH 07/13] Make linter happy --- planner/core/exhaust_physical_plans.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 454099f199d68..35e2c9e7af5c4 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -392,7 +392,7 @@ var ForceUseOuterBuild4Test = atomic.NewBool(false) var ForcedHashLeftJoin4Test = atomic.NewBool(false) func (p *LogicalJoin) shouldSkipHashJoin() bool { - return (p.preferJoinType&preferNoHashJoin) > 0 || (p.SCtx().GetSessionVars().EnableHashJoin == false) + return (p.preferJoinType&preferNoHashJoin) > 0 || (!p.SCtx().GetSessionVars().EnableHashJoin) } func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []PhysicalPlan, forced bool) { From cef90b0b8c2a6fdd59600ec5e1d94ef57e4c7143 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 14:30:18 +0800 Subject: [PATCH 08/13] fix --- planner/core/casetest/rule/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/casetest/rule/BUILD.bazel b/planner/core/casetest/rule/BUILD.bazel index 7949277ddcb3b..c0fc070d0951a 100644 --- a/planner/core/casetest/rule/BUILD.bazel +++ b/planner/core/casetest/rule/BUILD.bazel @@ -12,7 +12,7 @@ go_test( ], data = glob(["testdata/**"]), flaky = True, - shard_count = 23, + shard_count = 24, deps = [ "//domain", "//expression", From 3bbd2df3b56ff91e94119a76605bef06fa919a51 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 15:01:46 +0800 Subject: [PATCH 09/13] fix --- planner/core/logical_plans_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/logical_plans_test.go b/planner/core/logical_plans_test.go index 32e0f0d370328..e9a76538fb859 100644 --- a/planner/core/logical_plans_test.go +++ b/planner/core/logical_plans_test.go @@ -563,6 +563,7 @@ func TestPlanBuilder(t *testing.T) { s := createPlannerSuite() defer s.Close() s.ctx.GetSessionVars().CostModelVersion = modelVer1 + s.ctx.GetSessionVars().EnableHashJoin = true ctx := context.Background() for i, ca := range input { comment := fmt.Sprintf("for %s", ca) From cdb93a721b2e05f7c6a5b212d67ebf375523f376 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 15:16:35 +0800 Subject: [PATCH 10/13] fix ci --- planner/core/logical_plans_test.go | 1 - planner/core/optimizer.go | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/planner/core/logical_plans_test.go b/planner/core/logical_plans_test.go index e9a76538fb859..32e0f0d370328 100644 --- a/planner/core/logical_plans_test.go +++ b/planner/core/logical_plans_test.go @@ -563,7 +563,6 @@ func TestPlanBuilder(t *testing.T) { s := createPlannerSuite() defer s.Close() s.ctx.GetSessionVars().CostModelVersion = modelVer1 - s.ctx.GetSessionVars().EnableHashJoin = true ctx := context.Background() for i, ca := range input { comment := fmt.Sprintf("for %s", ca) diff --git a/planner/core/optimizer.go b/planner/core/optimizer.go index df38bb17ca41c..08ad8829a9095 100644 --- a/planner/core/optimizer.go +++ b/planner/core/optimizer.go @@ -162,6 +162,7 @@ type logicalOptRule interface { func BuildLogicalPlanForTest(ctx context.Context, sctx sessionctx.Context, node ast.Node, infoSchema infoschema.InfoSchema) (Plan, types.NameSlice, error) { sctx.GetSessionVars().PlanID.Store(0) sctx.GetSessionVars().PlanColumnID.Store(0) + sctx.GetSessionVars().EnableHashJoin = true builder, _ := NewPlanBuilder().Init(sctx, infoSchema, &utilhint.BlockHintProcessor{}) p, err := builder.Build(ctx, node) if err != nil { From c6dc9b3beeefb6db58706f8adae4455f31d09bbf Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 17:16:17 +0800 Subject: [PATCH 11/13] fix ci --- session/session.go | 1 + 1 file changed, 1 insertion(+) diff --git a/session/session.go b/session/session.go index d838d79c18902..9296de5dd7529 100644 --- a/session/session.go +++ b/session/session.go @@ -3039,6 +3039,7 @@ func CreateSession4TestWithOpt(store kv.Storage, opt *Opt) (Session, error) { s.GetSessionVars().MaxChunkSize = 32 s.GetSessionVars().MinPagingSize = variable.DefMinPagingSize s.GetSessionVars().EnablePaging = variable.DefTiDBEnablePaging + s.GetSessionVars().EnableHashJoin = true err = s.GetSessionVars().SetSystemVarWithoutValidation(variable.CharacterSetConnection, "utf8mb4") } return s, err From e4dd5d71cb7b2f28a43e4a4f1b2d5adbcdc74d38 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 17:46:31 +0800 Subject: [PATCH 12/13] fix ci --- util/mock/context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/util/mock/context.go b/util/mock/context.go index 3ecf16466b3d6..f2aeeeb24ff1c 100644 --- a/util/mock/context.go +++ b/util/mock/context.go @@ -463,6 +463,7 @@ func NewContext() *Context { } vars := variable.NewSessionVars(sctx) sctx.sessionVars = vars + vars.EnableHashJoin = true vars.InitChunkSize = 2 vars.MaxChunkSize = 32 vars.StmtCtx.TimeZone = time.UTC From a6893ef0208b3f1c550b0a0f84a0dc5cca1121d2 Mon Sep 17 00:00:00 2001 From: qw4990 Date: Mon, 25 Sep 2023 18:34:13 +0800 Subject: [PATCH 13/13] fix ci --- planner/core/exhaust_physical_plans.go | 2 +- planner/core/optimizer.go | 1 - session/session.go | 1 - sessionctx/variable/session.go | 4 ++-- sessionctx/variable/sysvar.go | 2 +- util/mock/context.go | 1 - 6 files changed, 4 insertions(+), 7 deletions(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 35e2c9e7af5c4..1098844ac05fe 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -392,7 +392,7 @@ var ForceUseOuterBuild4Test = atomic.NewBool(false) var ForcedHashLeftJoin4Test = atomic.NewBool(false) func (p *LogicalJoin) shouldSkipHashJoin() bool { - return (p.preferJoinType&preferNoHashJoin) > 0 || (!p.SCtx().GetSessionVars().EnableHashJoin) + return (p.preferJoinType&preferNoHashJoin) > 0 || (p.SCtx().GetSessionVars().DisableHashJoin) } func (p *LogicalJoin) getHashJoins(prop *property.PhysicalProperty) (joins []PhysicalPlan, forced bool) { diff --git a/planner/core/optimizer.go b/planner/core/optimizer.go index 08ad8829a9095..df38bb17ca41c 100644 --- a/planner/core/optimizer.go +++ b/planner/core/optimizer.go @@ -162,7 +162,6 @@ type logicalOptRule interface { func BuildLogicalPlanForTest(ctx context.Context, sctx sessionctx.Context, node ast.Node, infoSchema infoschema.InfoSchema) (Plan, types.NameSlice, error) { sctx.GetSessionVars().PlanID.Store(0) sctx.GetSessionVars().PlanColumnID.Store(0) - sctx.GetSessionVars().EnableHashJoin = true builder, _ := NewPlanBuilder().Init(sctx, infoSchema, &utilhint.BlockHintProcessor{}) p, err := builder.Build(ctx, node) if err != nil { diff --git a/session/session.go b/session/session.go index 9296de5dd7529..d838d79c18902 100644 --- a/session/session.go +++ b/session/session.go @@ -3039,7 +3039,6 @@ func CreateSession4TestWithOpt(store kv.Storage, opt *Opt) (Session, error) { s.GetSessionVars().MaxChunkSize = 32 s.GetSessionVars().MinPagingSize = variable.DefMinPagingSize s.GetSessionVars().EnablePaging = variable.DefTiDBEnablePaging - s.GetSessionVars().EnableHashJoin = true err = s.GetSessionVars().SetSystemVarWithoutValidation(variable.CharacterSetConnection, "utf8mb4") } return s, err diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 20f97c851c835..398f7dc52c6f2 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -1244,8 +1244,8 @@ type SessionVars struct { // AnalyzeVersion indicates how TiDB collect and use analyzed statistics. AnalyzeVersion int - // EnableHashjoin indicates whether to enable hash join. - EnableHashJoin bool + // DisableHashJoin indicates whether to disable hash join. + DisableHashJoin bool // EnableHistoricalStats indicates whether to enable historical statistics. EnableHistoricalStats bool diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 7ecc9595dbff0..f8870466509a2 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -2057,7 +2057,7 @@ var defaultSysVars = []*SysVar{ return nil }}, {Scope: ScopeGlobal | ScopeSession, Name: TiDBOptEnableHashJoin, Value: BoolToOnOff(DefTiDBOptEnableHashJoin), Type: TypeBool, SetSession: func(s *SessionVars, val string) error { - s.EnableHashJoin = TiDBOptOn(val) + s.DisableHashJoin = !TiDBOptOn(val) return nil }}, {Scope: ScopeGlobal | ScopeSession, Name: TiDBEnableIndexMergeJoin, Value: BoolToOnOff(DefTiDBEnableIndexMergeJoin), Hidden: true, Type: TypeBool, SetSession: func(s *SessionVars, val string) error { diff --git a/util/mock/context.go b/util/mock/context.go index f2aeeeb24ff1c..3ecf16466b3d6 100644 --- a/util/mock/context.go +++ b/util/mock/context.go @@ -463,7 +463,6 @@ func NewContext() *Context { } vars := variable.NewSessionVars(sctx) sctx.sessionVars = vars - vars.EnableHashJoin = true vars.InitChunkSize = 2 vars.MaxChunkSize = 32 vars.StmtCtx.TimeZone = time.UTC