address comments

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/golang/protobuf v1.2.0
 	github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db // indirect
 	github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c
+	github.com/gorilla/context v1.1.1 // indirect
 	github.com/gorilla/mux v1.6.2
 	github.com/grpc-ecosystem/go-grpc-middleware v1.0.0
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0

planner/core/logical_plan_test.go

@@ -988,7 +988,7 @@ func (s *testPlanSuite) TestJoinReOrder(c *C) {
 		},
 		{
 			sql:  "select * from t t1, t t2, t t3, t t4, t t5 where t1.a = t5.a and t5.a = t4.a and t4.a = t3.a and t3.a = t2.a and t2.a = t1.a and t1.a = t3.a and t2.a = t4.a and t5.b < 8",
-			best: "Join{Join{Join{Join{DataScan(t5)->DataScan(t1)}(t5.a,t1.a)->DataScan(t2)}(t1.a,t2.a)->DataScan(t3)}(t2.a,t3.a)(t1.a,t3.a)->DataScan(t4)}(t3.a,t4.a)(t2.a,t4.a)(t5.a,t4.a)->Projection",
+			best: "Join{Join{Join{Join{DataScan(t5)->DataScan(t1)}(t5.a,t1.a)->DataScan(t2)}(t1.a,t2.a)->DataScan(t3)}(t2.a,t3.a)(t1.a,t3.a)->DataScan(t4)}(t5.a,t4.a)(t3.a,t4.a)(t2.a,t4.a)->Projection",
 		},
 		{
 			sql:  "select * from t t1, t t2, t t3, t t4, t t5 where t1.a = t5.a and t5.a = t4.a and t4.a = t3.a and t3.a = t2.a and t2.a = t1.a and t1.a = t3.a and t2.a = t4.a and t3.b = 1 and t4.a = 1",

planner/core/rule_join_reorder_dp.go

@@ -19,7 +19,6 @@ import (
 	"github.com/pingcap/parser/ast"
 	"github.com/pingcap/tidb/expression"
 	"github.com/pingcap/tidb/sessionctx"
-	log "github.com/sirupsen/logrus"
 )
 
 type joinReorderDPSolver struct {
@@ -48,8 +47,14 @@ func (s *joinReorderDPSolver) solve(joinGroup []LogicalPlan, conds []expression.
 		sf := cond.(*expression.ScalarFunction)
 		lCol := sf.GetArgs()[0].(*expression.Column)
 		rCol := sf.GetArgs()[1].(*expression.Column)
-		lIdx := findNodeIndexInGroup(joinGroup, lCol)
-		rIdx := findNodeIndexInGroup(joinGroup, rCol)
+		lIdx, err := findNodeIndexInGroup(joinGroup, lCol)
+		if err != nil {
+			return nil, err
+		}
+		rIdx, err := findNodeIndexInGroup(joinGroup, rCol)
+		if err != nil {
+			return nil, err
+		}
 		addEdge(lIdx, rIdx, sf)
 	}
 	visited := make([]bool, len(joinGroup))
@@ -181,12 +186,11 @@ func (s *joinReorderDPSolver) makeBushyJoin(cartesianJoinGroup []LogicalPlan) Lo
 	return cartesianJoinGroup[0]
 }
 
-func findNodeIndexInGroup(groups []LogicalPlan, col *expression.Column) int {
-	for i, plan := range groups {
+func findNodeIndexInGroup(group []LogicalPlan, col *expression.Column) (int, error) {
+	for i, plan := range group {
 		if plan.Schema().Contains(col) {
-			return i
+			return i, nil
 		}
 	}
-	log.Errorf("Unknown columns %s, position %d", col, col.UniqueID)
-	return -1
+	return -1, ErrUnknownColumn.GenWithStackByArgs(col, "JOIN REORDER RULE")
 }

planner/core/rule_join_reorder_greedy.go

@@ -24,50 +24,24 @@ import (
 
 // extractJoinGroup will extract the nodes that connected by join tree.
 // A join tree is a tree that all node is join except the leaves.
-func extractJoinGroup(p *LogicalJoin) (group []LogicalPlan, eqEdges []*expression.ScalarFunction, otherConds []expression.Expression) {
-	if p.reordered || p.preferJoinType > uint(0) || p.JoinType != InnerJoin || p.StraightJoin {
-		return nil, nil, nil
-	}
-	lChild := p.children[0]
-	rChild := p.children[1]
-	lJoin, lOk := lChild.(*LogicalJoin)
-	rJoin, rOk := rChild.(*LogicalJoin)
-	if !lOk && !rOk {
-		eqEdges = make([]*expression.ScalarFunction, len(p.EqualConditions))
-		otherConds = make([]expression.Expression, len(p.OtherConditions))
-		copy(eqEdges, p.EqualConditions)
-		copy(otherConds, p.OtherConditions)
-		return []LogicalPlan{lChild, rChild}, eqEdges, otherConds
-	}
-	if lOk {
-		lhsJoinGroup, lEqEdges, lOtherConds := extractJoinGroup(lJoin)
-		// If left side is
-		if lhsJoinGroup == nil {
-			group = append(group, lJoin)
-		} else {
-			group = append(group, lhsJoinGroup...)
-			eqEdges = append(eqEdges, lEqEdges...)
-			otherConds = append(otherConds, lOtherConds...)
-		}
-	} else {
-		group = append(group, lChild)
-	}
-
-	if rOk {
-		rhsJoinGroup, rEqEdges, rOtherConds := extractJoinGroup(rJoin)
-		if rhsJoinGroup == nil {
-			group = append(group, rJoin)
-		} else {
-			group = append(group, rhsJoinGroup...)
-			eqEdges = append(eqEdges, rEqEdges...)
-			otherConds = append(otherConds, rOtherConds...)
-
-		}
-	} else {
-		group = append(group, rChild)
+func extractJoinGroup(p LogicalPlan) (group []LogicalPlan, eqEdges []*expression.ScalarFunction, otherConds []expression.Expression) {
+	join, isJoin := p.(*LogicalJoin)
+	if !isJoin || join.preferJoinType > uint(0) || join.JoinType != InnerJoin || join.StraightJoin {
+		return []LogicalPlan{p}, nil, nil
 	}
 
-	return group, append(eqEdges, p.EqualConditions...), append(otherConds, p.OtherConditions...)
+	lhsGroup, lhsEqualConds, lhsOtherConds := extractJoinGroup(join.children[0])
+	rhsGroup, rhsEqualConds, rhsOtherConds := extractJoinGroup(join.children[1])
+
+	group = append(group, lhsGroup...)
+	group = append(group, rhsGroup...)
+	eqEdges = append(eqEdges, join.EqualConditions...)
+	eqEdges = append(eqEdges, lhsEqualConds...)
+	eqEdges = append(eqEdges, rhsEqualConds...)
+	otherConds = append(otherConds, join.OtherConditions...)
+	otherConds = append(otherConds, lhsOtherConds...)
+	otherConds = append(otherConds, rhsOtherConds...)
+	return group, eqEdges, otherConds
 }
 
 type joinReOrderGreedySolver struct {
@@ -127,6 +101,7 @@ func (s *joinReOrderGreedySolver) enlargeJoinTree() (LogicalPlan, error) {
 				finalRemainOthers = remainOthers
 			}
 		}
+		// If we could find more join node, meaning that the sub connected graph have been totally explored.
 		if bestJoin == nil {
 			break
 		}
@@ -210,15 +185,21 @@ func (s *joinReOrderGreedySolver) optimize(p LogicalPlan) (LogicalPlan, error) {
 }
 
 func (s *joinReOrderGreedySolver) optimizeRecursive(p LogicalPlan) (LogicalPlan, error) {
-	if join, ok := p.(*LogicalJoin); ok {
-		s.curJoinGroup, s.eqEdges, s.otherConds = extractJoinGroup(join)
-		if len(s.curJoinGroup) != 0 {
-			var err error
-			p, err = s.solve()
+	var err error
+	curJoinGroup, eqEdges, otherConds := extractJoinGroup(p)
+	if len(curJoinGroup) > 1 {
+		for i := range curJoinGroup {
+			curJoinGroup[i], err = s.optimizeRecursive(curJoinGroup[i])
 			if err != nil {
 				return nil, err
 			}
 		}
+		s.curJoinGroup, s.eqEdges, s.otherConds = curJoinGroup, eqEdges, otherConds
+		p, err = s.solve()
+		if err != nil {
+			return nil, err
+		}
+		return p, nil
 	}
 	newChildren := make([]LogicalPlan, 0, len(p.Children()))
 	for _, child := range p.Children() {