From a942997c8d8049672ec3fefa7f226f080af09c66 Mon Sep 17 00:00:00 2001 From: foreyes Date: Mon, 15 Jul 2019 13:00:28 +0800 Subject: [PATCH 1/8] fix logic of extractTableName and tryToGetIndexJoin --- planner/core/exhaust_physical_plans.go | 12 +++++++++--- planner/core/logical_plan_builder.go | 6 ++++++ 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 53e255fa5f75a..5a86232bc1402 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -696,15 +696,21 @@ func (p *LogicalJoin) tryToGetIndexJoin(prop *property.PhysicalProperty) (indexJ } if leftJoins != nil && lhsCardinality < rhsCardinality { - return leftJoins, hasIndexJoinHint + return leftJoins, leftOuter } if rightJoins != nil && rhsCardinality < lhsCardinality { - return rightJoins, hasIndexJoinHint + return rightJoins, rightOuter } + // At this position, at least one of leftJoins and rightJoins is nil + // Unless (leftOuter == rightOuter && lhsCardinality == rhsCardinality) + canForceLeft := leftJoins != nil && leftOuter + canForceRight := rightJoins != nil && rightOuter + joins := append(leftJoins, rightJoins...) - return joins, hasIndexJoinHint && len(joins) != 0 + forced = canForceLeft || canForceRight + return joins, forced } return nil, false diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index aa95086bbbb24..d504c9fcfef86 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -283,6 +283,12 @@ func (p *LogicalJoin) extractOnCondition(conditions []expression.Expression, der func extractTableAlias(p LogicalPlan) *model.CIStr { if p.Schema().Len() > 0 && p.Schema().Columns[0].TblName.L != "" { + tblName := p.Schema().Columns[0].TblName.L + for _, column := range p.Schema().Columns { + if column.TblName.L != tblName { + return nil + } + } return &(p.Schema().Columns[0].TblName) } return nil From d3fdc4a7fe18d56ab39a3f37aea88931e25122e3 Mon Sep 17 00:00:00 2001 From: foreyes Date: Mon, 15 Jul 2019 16:05:47 +0800 Subject: [PATCH 2/8] add unit test --- planner/core/physical_plan_test.go | 49 ++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/planner/core/physical_plan_test.go b/planner/core/physical_plan_test.go index 3d1bce80e281d..1039df4a213d2 100644 --- a/planner/core/physical_plan_test.go +++ b/planner/core/physical_plan_test.go @@ -1393,3 +1393,52 @@ func (s *testPlanSuite) TestUnmatchedTableInHint(c *C) { } } } + +func (s *testPlanSuite) TestIndexJoinHint(c *C) { + defer testleak.AfterTest(c)() + store, dom, err := newStoreWithBootstrap() + c.Assert(err, IsNil) + defer func() { + dom.Close() + store.Close() + }() + se, err := session.CreateSession4Test(store) + c.Assert(err, IsNil) + _, err = se.Execute(context.Background(), "use test") + c.Assert(err, IsNil) + + tests := []struct { + sql string + best string + warning string + }{ + { + sql: "select /*+ TIDB_INLJ(t1) */ t1.a, t2.a, t3.a from t t1, t t2, t t3 where t1.a = t2.a and t2.a = t3.a;", + best: "MergeInnerJoin{TableReader(Table(t))->IndexJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t2.a,test.t1.a)}(test.t3.a,test.t2.a)->Projection", + warning: "", + }, + { + sql: "select /*+ TIDB_INLJ(t1) */ t1.b, t2.b from t t1, t t2 where t1.b = t2.b;", + best: "LeftHashJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t1.b,test.t2.b)", + warning: "[planner:1815]Optimizer Hint /*+ TIDB_INLJ(t1) */ is inapplicable", + }, + } + for i, test := range tests { + comment := Commentf("case:%v sql:%s", i, test) + stmt, err := s.ParseOneStmt(test.sql, "", "") + c.Assert(err, IsNil, comment) + + p, err := planner.Optimize(se, stmt, s.is) + c.Assert(err, IsNil) + c.Assert(core.ToString(p), Equals, test.best) + + warnings := se.GetSessionVars().StmtCtx.GetWarnings() + if test.warning == "" { + c.Assert(len(warnings), Equals, 0) + } else { + c.Assert(len(warnings), Equals, 1) + c.Assert(warnings[0].Level, Equals, stmtctx.WarnLevelWarning) + c.Assert(warnings[0].Err.Error(), Equals, test.warning) + } + } +} From 715e091791b9f0e32acac0c7842e442eb1310fdf Mon Sep 17 00:00:00 2001 From: foreyes Date: Mon, 15 Jul 2019 16:24:14 +0800 Subject: [PATCH 3/8] fix test case --- planner/core/physical_plan_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/planner/core/physical_plan_test.go b/planner/core/physical_plan_test.go index 1039df4a213d2..9fc17334e4eba 100644 --- a/planner/core/physical_plan_test.go +++ b/planner/core/physical_plan_test.go @@ -1418,8 +1418,8 @@ func (s *testPlanSuite) TestIndexJoinHint(c *C) { warning: "", }, { - sql: "select /*+ TIDB_INLJ(t1) */ t1.b, t2.b from t t1, t t2 where t1.b = t2.b;", - best: "LeftHashJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t1.b,test.t2.b)", + sql: "select /*+ TIDB_INLJ(t1) */ t1.b, t2.a from t t1, t t2 where t1.b = t2.a;", + best: "LeftHashJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t1.b,test.t2.a)", warning: "[planner:1815]Optimizer Hint /*+ TIDB_INLJ(t1) */ is inapplicable", }, } From fcd90ab50d2af8803c79d6090490e2c11265440c Mon Sep 17 00:00:00 2001 From: foreyes Date: Tue, 16 Jul 2019 11:33:21 +0800 Subject: [PATCH 4/8] add comment --- planner/core/logical_plan_builder.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index d504c9fcfef86..f3e1a008d2417 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -281,6 +281,9 @@ func (p *LogicalJoin) extractOnCondition(conditions []expression.Expression, der return } +// This function will return the table alias of a given logical plan. +// If there're multiple tables, it will return nil. +// This function will be further developed in the future. func extractTableAlias(p LogicalPlan) *model.CIStr { if p.Schema().Len() > 0 && p.Schema().Columns[0].TblName.L != "" { tblName := p.Schema().Columns[0].TblName.L From 4bb32f8d765f0b780b12b8ff53497f9c738fe58d Mon Sep 17 00:00:00 2001 From: foreyes Date: Wed, 17 Jul 2019 15:45:06 +0800 Subject: [PATCH 5/8] rewrite comments --- planner/core/logical_plan_builder.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index f3e1a008d2417..b33ef716553c3 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -281,9 +281,9 @@ func (p *LogicalJoin) extractOnCondition(conditions []expression.Expression, der return } -// This function will return the table alias of a given logical plan. -// If there're multiple tables, it will return nil. -// This function will be further developed in the future. +// The table alias is only used to check if the logicalPlan match some +// optimizer hints. It will return nil when there are multiple table alias, +// because hints are not expected to take effect in this case. func extractTableAlias(p LogicalPlan) *model.CIStr { if p.Schema().Len() > 0 && p.Schema().Columns[0].TblName.L != "" { tblName := p.Schema().Columns[0].TblName.L From 226466836bee6a059ba6a8a7817376a8b1de4f1a Mon Sep 17 00:00:00 2001 From: foreyes Date: Thu, 18 Jul 2019 15:22:10 +0800 Subject: [PATCH 6/8] rewrite comment --- planner/core/logical_plan_builder.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index b33ef716553c3..62170b782809c 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -281,9 +281,10 @@ func (p *LogicalJoin) extractOnCondition(conditions []expression.Expression, der return } -// The table alias is only used to check if the logicalPlan match some -// optimizer hints. It will return nil when there are multiple table alias, -// because hints are not expected to take effect in this case. +// extractTableAlias returns table alias of the LogicalPlan's columns. +// It will return nil when there are multiple table alias, because the alias +// is only used to check if the logicalPlan match some optimizer hints, +// and hints are not expected to take effect in this case. func extractTableAlias(p LogicalPlan) *model.CIStr { if p.Schema().Len() > 0 && p.Schema().Columns[0].TblName.L != "" { tblName := p.Schema().Columns[0].TblName.L From 9279bf4026db414754ddf34ac08988dcf2006dff Mon Sep 17 00:00:00 2001 From: foreyes Date: Mon, 22 Jul 2019 13:12:44 +0800 Subject: [PATCH 7/8] rewrite comments & clear code --- planner/core/exhaust_physical_plans.go | 4 +--- planner/core/logical_plan_builder.go | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/planner/core/exhaust_physical_plans.go b/planner/core/exhaust_physical_plans.go index 5a86232bc1402..5447b8b15a02c 100644 --- a/planner/core/exhaust_physical_plans.go +++ b/planner/core/exhaust_physical_plans.go @@ -703,13 +703,11 @@ func (p *LogicalJoin) tryToGetIndexJoin(prop *property.PhysicalProperty) (indexJ return rightJoins, rightOuter } - // At this position, at least one of leftJoins and rightJoins is nil - // Unless (leftOuter == rightOuter && lhsCardinality == rhsCardinality) canForceLeft := leftJoins != nil && leftOuter canForceRight := rightJoins != nil && rightOuter + forced = canForceLeft || canForceRight joins := append(leftJoins, rightJoins...) - forced = canForceLeft || canForceRight return joins, forced } diff --git a/planner/core/logical_plan_builder.go b/planner/core/logical_plan_builder.go index 62170b782809c..b37e3ce58eaa0 100644 --- a/planner/core/logical_plan_builder.go +++ b/planner/core/logical_plan_builder.go @@ -282,9 +282,8 @@ func (p *LogicalJoin) extractOnCondition(conditions []expression.Expression, der } // extractTableAlias returns table alias of the LogicalPlan's columns. -// It will return nil when there are multiple table alias, because the alias -// is only used to check if the logicalPlan match some optimizer hints, -// and hints are not expected to take effect in this case. +// It will return nil when there are multiple table alias, because the alias is only used to check if +// the logicalPlan match some optimizer hints, and hints are not expected to take effect in this case. func extractTableAlias(p LogicalPlan) *model.CIStr { if p.Schema().Len() > 0 && p.Schema().Columns[0].TblName.L != "" { tblName := p.Schema().Columns[0].TblName.L From b824034334bad49190dee60e99190354afbce91f Mon Sep 17 00:00:00 2001 From: foreyes Date: Tue, 23 Jul 2019 13:21:38 +0800 Subject: [PATCH 8/8] fix unit test --- planner/core/physical_plan_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/planner/core/physical_plan_test.go b/planner/core/physical_plan_test.go index 9fc17334e4eba..3e69977e21754 100644 --- a/planner/core/physical_plan_test.go +++ b/planner/core/physical_plan_test.go @@ -1414,7 +1414,7 @@ func (s *testPlanSuite) TestIndexJoinHint(c *C) { }{ { sql: "select /*+ TIDB_INLJ(t1) */ t1.a, t2.a, t3.a from t t1, t t2, t t3 where t1.a = t2.a and t2.a = t3.a;", - best: "MergeInnerJoin{TableReader(Table(t))->IndexJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t2.a,test.t1.a)}(test.t3.a,test.t2.a)->Projection", + best: "MergeInnerJoin{IndexJoin{TableReader(Table(t))->TableReader(Table(t))}(test.t2.a,test.t1.a)->TableReader(Table(t))}(test.t2.a,test.t3.a)", warning: "", }, { @@ -1428,7 +1428,7 @@ func (s *testPlanSuite) TestIndexJoinHint(c *C) { stmt, err := s.ParseOneStmt(test.sql, "", "") c.Assert(err, IsNil, comment) - p, err := planner.Optimize(se, stmt, s.is) + p, err := core.Optimize(se, stmt, s.is) c.Assert(err, IsNil) c.Assert(core.ToString(p), Equals, test.best)