Merge pull request #3057 from dolthub/fulghum/join-planning

Changing how `NO_MERGE_JOIN` hint is applied to fix a panic

Author: fulghum

sql/analyzer/indexed_joins.go

@@ -158,6 +158,9 @@ func replanJoin(ctx *sql.Context, n *plan.JoinNode, a *Analyzer, scope *plan.Sco
 
 	qFlags.Set(sql.QFlagInnerJoin)
 
+	hints := m.SessionHints()
+	hints = append(hints, memo.ExtractJoinHint(n)...)
+
 	err = addIndexScans(ctx, m)
 	if err != nil {
 		return nil, err
@@ -180,9 +183,11 @@ func replanJoin(ctx *sql.Context, n *plan.JoinNode, a *Analyzer, scope *plan.Sco
 		return nil, err
 	}
 
-	err = addMergeJoins(ctx, m)
-	if err != nil {
-		return nil, err
+	if !mergeJoinsDisabled(hints) {
+		err = addMergeJoins(ctx, m)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	memo.CardMemoGroups(ctx, m.Root())
@@ -200,11 +205,9 @@ func replanJoin(ctx *sql.Context, n *plan.JoinNode, a *Analyzer, scope *plan.Sco
 		return nil, err
 	}
 
-	m.SetDefaultHints()
-	hints := memo.ExtractJoinHint(n)
+	// Once we've enumerated all expression groups, we can apply hints. This must be done after expression
+	// groups have been identified, so that the applied hints use the correct metadata.
 	for _, h := range hints {
-		// this should probably happen earlier, but the root is not
-		// populated before reordering
 		m.ApplyHint(h)
 	}
 
@@ -223,6 +226,16 @@ func replanJoin(ctx *sql.Context, n *plan.JoinNode, a *Analyzer, scope *plan.Sco
 	return m.BestRootPlan(ctx)
 }
 
+// mergeJoinsDisabled returns true if merge joins have been disabled in the specified |hints|.
+func mergeJoinsDisabled(hints []memo.Hint) bool {
+	for _, hint := range hints {
+		if hint.Typ == memo.HintTypeNoMergeJoin {
+			return true
+		}
+	}
+	return false
+}
+
 // addLookupJoins prefixes memo join group expressions with indexed join
 // alternatives to join plans added by joinOrderBuilder. We can assume that a
 // join with a non-nil join filter is not degenerate, and we can apply indexed

sql/memo/join_order_builder.go

@@ -154,7 +154,7 @@ var ErrUnsupportedReorderNode = errors.New("unsupported join reorder node")
 
 // useFastReorder determines whether to skip the current brute force join planning and use an alternate
 // planning algorithm that analyzes the join tree to find a sequence that can be implemented purely as lookup joins.
-// Currently we only use it for large joins (20+ tables) with no join hints.
+// Currently, we only use it for large joins (15+ tables) with no join hints.
 func (j *joinOrderBuilder) useFastReorder() bool {
 	if j.forceFastDFSLookupForTest {
 		return true
@@ -180,7 +180,7 @@ func (j *joinOrderBuilder) ReorderJoin(n sql.Node) {
 	// from ensureClosure in buildSingleLookupPlan, but the equivalence sets could create multiple possible join orders
 	// for the single-lookup plan, which would complicate things.
 	j.ensureClosure(j.m.root)
-	j.dbSube()
+	j.dpEnumerateSubsets()
 	return
 }
 
@@ -627,10 +627,10 @@ func (j *joinOrderBuilder) checkSize() {
 	}
 }
 
-// dpSube iterates all disjoint combinations of table sets,
+// dpEnumerateSubsets iterates all disjoint combinations of table sets,
 // adding plans to the tree when we find two sets that can
 // be joined
-func (j *joinOrderBuilder) dbSube() {
+func (j *joinOrderBuilder) dpEnumerateSubsets() {
 	all := j.allVertices()
 	for subset := vertexSet(1); subset <= all; subset++ {
 		if subset.isSingleton() {

sql/memo/memo.go

@@ -82,10 +82,13 @@ func (m *Memo) StatsProvider() sql.StatsProvider {
 	return m.statsProv
 }
 
-func (m *Memo) SetDefaultHints() {
+// SessionHints returns any hints that have been enabled in the session for join planning,
+// such as the @@disable_merge_join SQL system variable.
+func (m *Memo) SessionHints() (hints []Hint) {
 	if val, _ := m.Ctx.GetSessionVariable(m.Ctx, sql.DisableMergeJoin); val.(int8) != 0 {
-		m.ApplyHint(Hint{Typ: HintTypeNoMergeJoin})
+		hints = append(hints, Hint{Typ: HintTypeNoMergeJoin})
 	}
+	return hints
 }
 
 // newExprGroup creates a new logical expression group to encapsulate the
@@ -465,11 +468,6 @@ func (m *Memo) optimizeMemoGroup(grp *ExprGroup) error {
 // rather than a local property.
 func (m *Memo) updateBest(grp *ExprGroup, n RelExpr, cost float64) {
 	if !m.hints.isEmpty() {
-		for _, block := range m.hints.block {
-			if !block.isOk(n) {
-				return
-			}
-		}
 		if m.hints.satisfiedBy(n) {
 			if !grp.HintOk {
 				grp.Best = n
@@ -522,31 +520,20 @@ func getProjectColset(p *Project) sql.ColSet {
 	return colset
 }
 
+// ApplyHint applies |hint| to this memo, converting the parsed hint into an internal representation and updating
+// the internal data to match the memo metadata. Note that this function MUST be called only after memo groups have
+// been fully built out, otherwise the group information set in the internal join hint structures will be incomplete.
 func (m *Memo) ApplyHint(hint Hint) {
 	switch hint.Typ {
 	case HintTypeJoinOrder:
 		m.SetJoinOrder(hint.Args)
 	case HintTypeJoinFixedOrder:
 	case HintTypeNoMergeJoin:
-		m.SetBlockOp(func(n RelExpr) bool {
-			switch n := n.(type) {
-			case JoinRel:
-				jp := n.JoinPrivate()
-				if !jp.Left.Best.Group().HintOk || !jp.Right.Best.Group().HintOk {
-					// equiv closures can generate child plans that bypass hints
-					return false
-				}
-				if jp.Op.IsMerge() {
-					return false
-				}
-			}
-			return true
-		})
+		m.hints.disableMergeJoin = true
 	case HintTypeInnerJoin, HintTypeMergeJoin, HintTypeLookupJoin, HintTypeHashJoin, HintTypeSemiJoin, HintTypeAntiJoin, HintTypeLeftOuterLookupJoin:
 		m.SetJoinOp(hint.Typ, hint.Args[0], hint.Args[1])
 	case HintTypeLeftDeep:
 		m.hints.leftDeep = true
-	default:
 	}
 }
 
@@ -568,10 +555,6 @@ func (m *Memo) SetJoinOrder(tables []string) {
 	}
 }
 
-func (m *Memo) SetBlockOp(cb func(n RelExpr) bool) {
-	m.hints.block = append(m.hints.block, joinBlockHint{cb: cb})
-}
-
 func (m *Memo) SetJoinOp(op HintType, left, right string) {
 	var lTab, rTab sql.TableId
 	for _, n := range m.root.RelProps.TableIdNodes() {

sql/memo/select_hints.go

@@ -372,25 +372,18 @@ func (o joinOpHint) typeMatches(n RelExpr) bool {
 	return true
 }
 
-type joinBlockHint struct {
-	cb func(n RelExpr) bool
-}
-
-func (o joinBlockHint) isOk(n RelExpr) bool {
-	return o.cb(n)
-}
-
 // joinHints wraps a collection of join hints. The memo
 // interfaces with this object during costing.
 type joinHints struct {
-	ops      []joinOpHint
-	order    *joinOrderHint
-	block    []joinBlockHint
-	leftDeep bool
+	ops              []joinOpHint
+	order            *joinOrderHint
+	leftDeep         bool
+	disableMergeJoin bool
 }
 
+// isEmpty returns true if no hints that affect join planning have been set.
 func (h joinHints) isEmpty() bool {
-	return len(h.ops) == 0 && h.order == nil && !h.leftDeep && len(h.block) == 0
+	return len(h.ops) == 0 && h.order == nil && !h.leftDeep && !h.disableMergeJoin
 }
 
 // satisfiedBy returns whether a RelExpr satisfies every join hint. This