MB-51928 Additional fixes for join filter

A couple of additional issues for join filter:

1. When a plan with join filter is considered, but not
   chosen as the final plan, due to shared operator
   sub-tree used during plan considerations, there
   could be left-over bit filter structures in some
   operators.

   The fix is to do a final cleanup after a best plan
   is chosen, to reset any left-over bit filter info.

2. When a JOIN_FILTER hint is specified, since the
   marking of JOIN_FILTER hint is done at the very
   end, plan comparison did not take into consideration
   of whether JOIN_FILTER hint is followed or not, and
   it's possible to choose a plan that has lower cost
   without any join filter.

   The fix is to check and mark for JOIN_FILTER hint
   at the last level during join enumeration, such that
   we mark for JOIN_FILTER hint before we compare plans
   for the last level, and plans without JOIN_FILTER
   hint error are preferred over those with hint errors.

Change-Id: I14cc27773a1ff6e80d5508f985abf1cb2e0e2cf4
Reviewed-on: https://review.couchbase.org/c/query/+/179119
Reviewed-by: Sitaram Vemulapalli <sitaram.vemulapalli@couchbase.com>
Tested-by: Bingjie Miao <bingjie.miao@couchbase.com>

plan/bit_filter.go

@@ -268,6 +268,28 @@ func (this *BuildBitFilterBase) GetBuildBitFilters() BitFilters {
 	return this.buildBitFilters
 }
 
+func (this *BuildBitFilterBase) RemoveBuildBitFilter(alias string) {
+	for i, bf := range this.buildBitFilters {
+		if bf.alias == alias {
+			this.buildBitFilters[i] = nil
+		}
+	}
+}
+
+func (this *BuildBitFilterBase) CompactBuildBitFilters() {
+	newBuildBitFilters := make(BitFilters, 0, len(this.buildBitFilters))
+	for _, bf := range this.buildBitFilters {
+		if bf != nil {
+			newBuildBitFilters = append(newBuildBitFilters, bf)
+		}
+	}
+	if len(newBuildBitFilters) > 0 {
+		this.buildBitFilters = newBuildBitFilters
+	} else {
+		this.buildBitFilters = nil
+	}
+}
+
 func (this *BuildBitFilterBase) marshalBuildBitFilters(r map[string]interface{}) {
 	buildBFs := make([]interface{}, 0, len(this.buildBitFilters))
 	for _, bbf := range this.buildBitFilters {
@@ -310,6 +332,28 @@ func (this *ProbeBitFilterBase) GetProbeBitFilters() BitFilters {
 	return this.probeBitFilters
 }
 
+func (this *ProbeBitFilterBase) RemoveProbeBitFilter(alias string) {
+	for i, bf := range this.probeBitFilters {
+		if bf.alias == alias {
+			this.probeBitFilters[i] = nil
+		}
+	}
+}
+
+func (this *ProbeBitFilterBase) CompactProbeBitFilters() {
+	newProbeBitFilters := make(BitFilters, 0, len(this.probeBitFilters))
+	for _, bf := range this.probeBitFilters {
+		if bf != nil {
+			newProbeBitFilters = append(newProbeBitFilters, bf)
+		}
+	}
+	if len(newProbeBitFilters) > 0 {
+		this.probeBitFilters = newProbeBitFilters
+	} else {
+		this.probeBitFilters = nil
+	}
+}
+
 func (this *ProbeBitFilterBase) marshalProbeBitFilters(r map[string]interface{}) {
 	probeBFs := make([]interface{}, 0, len(this.probeBitFilters))
 	for _, pbf := range this.probeBitFilters {

planner/build_join_ansi.go

@@ -2351,3 +2351,128 @@ func checkScanJoinFilterHint(baseKeyspaces map[string]*base.BaseKeyspace,
 	}
 	return
 }
+
+func checkProbeBFAliases(probeAliases map[string]map[string]bool, gather bool, ops ...plan.Operator) {
+	for i := len(ops) - 1; i >= 0; i-- {
+		switch op := ops[i].(type) {
+		case *plan.IndexScan3:
+			if !op.Covering() {
+				alias := op.Term().Alias()
+				buildAliases := probeAliases[alias]
+				if gather && buildAliases == nil {
+					buildAliases = make(map[string]bool, len(probeAliases)-1)
+					probeAliases[alias] = buildAliases
+				}
+				for _, probeBF := range op.GetProbeBitFilters() {
+					if gather {
+						buildAliases[probeBF.Alias()] = true
+					} else {
+						checkBaseProbeBFAliases(buildAliases, op.GetProbeFilterBase())
+					}
+				}
+			}
+		case *plan.DistinctScan:
+			checkScanProbeBFAliases(probeAliases, gather, op.Scan())
+		case *plan.IntersectScan:
+			checkScanProbeBFAliases(probeAliases, gather, op.Scans()...)
+		case *plan.OrderedIntersectScan:
+			checkScanProbeBFAliases(probeAliases, gather, op.Scans()...)
+		case *plan.UnionScan:
+			checkScanProbeBFAliases(probeAliases, gather, op.Scans()...)
+		case *plan.Parallel:
+			checkProbeBFAliases(probeAliases, gather, op.Child())
+		case *plan.Sequence:
+			checkProbeBFAliases(probeAliases, gather, op.Children()...)
+		case *plan.HashJoin:
+			if !op.Outer() {
+				checkProbeBFAliases(probeAliases, gather, op.Child())
+			}
+		case *plan.Alias:
+			// skip next term (SubqueryTerm)
+			i--
+		}
+	}
+}
+
+func checkScanProbeBFAliases(probeAliases map[string]map[string]bool, gather bool, scans ...plan.SecondaryScan) {
+	for _, scan := range scans {
+		checkProbeBFAliases(probeAliases, gather, scan)
+	}
+}
+
+func checkBaseProbeBFAliases(buildAliases map[string]bool, op *plan.ProbeBitFilterBase) {
+	compact := false
+	for _, probeBF := range op.GetProbeBitFilters() {
+		buildAlias := probeBF.Alias()
+		if _, ok := buildAliases[buildAlias]; ok {
+			op.RemoveProbeBitFilter(buildAlias)
+			compact = true
+		}
+	}
+	if compact {
+		op.CompactProbeBitFilters()
+	}
+}
+
+func checkBuildBFAliases(probeAliases map[string]map[string]bool, ops ...plan.Operator) {
+	for i := len(ops) - 1; i >= 0; i-- {
+		switch op := ops[i].(type) {
+		case *plan.IndexScan3:
+			if op.Covering() {
+				checkBaseBuildBFAliases(probeAliases, op.Term().Alias(), op.GetBuildFilterBase())
+			}
+		case *plan.DistinctScan:
+			checkScanBuildBFAliases(probeAliases, op.Scan())
+		case *plan.IntersectScan:
+			checkScanBuildBFAliases(probeAliases, op.Scans()...)
+		case *plan.OrderedIntersectScan:
+			checkScanBuildBFAliases(probeAliases, op.Scans()...)
+		case *plan.UnionScan:
+			checkScanBuildBFAliases(probeAliases, op.Scans()...)
+		case *plan.ExpressionScan:
+			checkBaseBuildBFAliases(probeAliases, op.Alias(), op.GetBuildFilterBase())
+		case *plan.Unnest:
+			checkBaseBuildBFAliases(probeAliases, op.Alias(), op.GetBuildFilterBase())
+		case *plan.Filter:
+			checkBaseBuildBFAliases(probeAliases, op.Alias(), op.GetBuildFilterBase())
+		case *plan.Parallel:
+			checkBuildBFAliases(probeAliases, op.Child())
+		case *plan.Sequence:
+			checkBuildBFAliases(probeAliases, op.Children()...)
+		case *plan.HashJoin:
+			if !op.Outer() {
+				checkBuildBFAliases(probeAliases, op.Child())
+			}
+		case *plan.Alias:
+			checkBaseBuildBFAliases(probeAliases, op.Alias(), op.GetBuildFilterBase())
+			// skip next term (SubqueryTerm)
+			i--
+		}
+	}
+}
+
+func checkScanBuildBFAliases(probeAliases map[string]map[string]bool, scans ...plan.SecondaryScan) {
+	for _, scan := range scans {
+		checkBuildBFAliases(probeAliases, scan)
+	}
+}
+
+func checkBaseBuildBFAliases(probeAliases map[string]map[string]bool, buildAlias string, op *plan.BuildBitFilterBase) {
+	compact := false
+	for _, buildBF := range op.GetBuildBitFilters() {
+		probeAlias := buildBF.Alias()
+		buildAliases := probeAliases[probeAlias]
+		if _, ok := buildAliases[buildAlias]; ok {
+			delete(buildAliases, buildAlias)
+			if len(buildAliases) == 0 {
+				delete(probeAliases, probeAlias)
+			}
+		} else {
+			op.RemoveBuildBitFilter(probeAlias)
+			compact = true
+		}
+	}
+	if compact {
+		op.CompactBuildBitFilters()
+	}
+}

planner/build_select_from.go

@@ -208,10 +208,11 @@ func (this *builder) visitFrom(node *algebra.Subselect, group *algebra.Group,
 				return err
 			}
 			// join filter hints are checked/marked after all join/nest/unnest are done
-			err = this.markJoinFilterHints()
+			err = this.MarkJoinFilterHints()
 			if err != nil {
 				return err
 			}
+			this.CheckBitFilters(this.children, this.subChildren)
 		}
 	} else {
 		// No FROM clause

planner/hints.go

@@ -429,7 +429,7 @@ func (this *builder) markOptimHints(alias string, includeJoin bool) (err error)
 	return nil
 }
 
-func (this *builder) markJoinFilterHints() (err error) {
+func (this *builder) MarkJoinFilterHints() (err error) {
 	if len(this.subChildren) > 0 {
 		err = checkJoinFilterHint(this.baseKeyspaces, this.subChildren...)
 		if err != nil {

planner/optimizer.go

@@ -53,7 +53,10 @@ type Builder interface {
 		outerCoveringScans []plan.CoveringOperator, outerFilter expression.Expression) (
 		[]plan.Operator, []plan.Operator, []plan.CoveringOperator, expression.Expression, error)
 
-	MarkOptimHints() error
+	MarkKeyspaceHints() error
+	MarkJoinFilterHints() error
+	CheckBitFilters(qPlan, subPlan []plan.Operator)
+	CheckJoinFilterHints(qPlan, subPlan []plan.Operator) (hintError bool, err error)
 }
 
 func (this *builder) GetBaseKeyspaces() map[string]*base.BaseKeyspace {
@@ -361,12 +364,48 @@ func (this *builder) BuildUnnest(unnest *algebra.Unnest, outerPlan, outerSubPlan
 	return this.children, this.subChildren, this.coveringScans, this.filter, nil
 }
 
-func (this *builder) MarkOptimHints() (err error) {
+func (this *builder) MarkKeyspaceHints() (err error) {
 	for alias, _ := range this.baseKeyspaces {
 		err = this.markOptimHints(alias, true)
 		if err != nil {
 			return err
 		}
 	}
-	return this.markJoinFilterHints()
+	return
+}
+
+func (this *builder) CheckJoinFilterHints(qPlan, subPlan []plan.Operator) (hintError bool, err error) {
+	err = checkJoinFilterHint(this.baseKeyspaces, subPlan...)
+	for _, baseKeyspace := range this.baseKeyspaces {
+		if baseKeyspace.HasJoinFltrHintError() {
+			hintError = true
+			baseKeyspace.UnsetJoinFltrHintError()
+		}
+	}
+	if err != nil || hintError {
+		return
+	}
+
+	err = checkJoinFilterHint(this.baseKeyspaces, qPlan...)
+	for _, baseKeyspace := range this.baseKeyspaces {
+		if baseKeyspace.HasJoinFltrHintError() {
+			hintError = true
+			baseKeyspace.UnsetJoinFltrHintError()
+		}
+	}
+	return
+}
+
+func (this *builder) CheckBitFilters(qPlan, subPlan []plan.Operator) {
+	probeAliases := make(map[string]map[string]bool, len(this.baseKeyspaces))
+	checkProbeBFAliases(probeAliases, true, subPlan...)
+	checkProbeBFAliases(probeAliases, true, qPlan...)
+
+	checkBuildBFAliases(probeAliases, subPlan...)
+	checkBuildBFAliases(probeAliases, qPlan...)
+
+	if len(probeAliases) > 0 {
+		checkProbeBFAliases(probeAliases, false, subPlan...)
+		checkProbeBFAliases(probeAliases, false, qPlan...)
+	}
 }