MB-52689 Save/restore of advisor structure for subquery

The main issue with this defect is that when we ADVISE for
subqueries, there are a couple of additional structures
in the builder that requires save/restore operation. This
is in addition to the queryInfos map that is currently
being saved/restored.

Another issue is when we run ADVISE on subquery, if
there is an error from the planner, do not simply return
the error (e.g. no INDEX available). Let the logic
continues such that we can advise indexes later.

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

planner/build.go

@@ -76,7 +76,9 @@ func (this *builder) chkBldSubqueries(stmt algebra.Statement, qp *plan.QueryPlan
 	if this.hasBuilderFlag(BUILDER_PLAN_SUBQUERY) {
 		err = this.buildSubqueries(stmt, qp)
 		// once plans for subqueries are built, unset builder flag
-		this.unsetBuilderFlag(BUILDER_PLAN_SUBQUERY)
+		if !this.subquery {
+			this.unsetBuilderFlag(BUILDER_PLAN_SUBQUERY)
+		}
 	}
 	return
 }
@@ -89,9 +91,16 @@ func (this *builder) buildSubqueries(stmt algebra.Statement, qp *plan.QueryPlan)
 	if len(subqueries) == 0 {
 		return nil
 	}
+	var saveQInfo *saveQueryInfo
 	subquery := this.subquery
+	if this.indexAdvisor {
+		saveQInfo = this.saveQueryInfo()
+	}
 	defer func() {
 		this.subquery = subquery
+		if this.indexAdvisor {
+			this.restoreQueryInfo(saveQInfo)
+		}
 	}()
 	this.subquery = true
 	this.makeSubqueryInfos(len(subqueries))
@@ -101,22 +110,27 @@ func (this *builder) buildSubqueries(stmt algebra.Statement, qp *plan.QueryPlan)
 			continue
 		}
 
-		this.startSubqIndexAdvisor()
-		this.initialIndexAdvisor(subq)
+		if this.indexAdvisor {
+			this.startSubqIndexAdvisor()
+			this.initialIndexAdvisor(subq)
+		}
 
 		// be warned, this amends the AST for the subqueries
 		p, err := subq.Accept(this)
-		if err != nil {
+		if err == nil {
+			qplan := p.(*plan.QueryPlan)
+			qp.AddSubquery(subq, qplan.PlanOp())
+			for s, o := range qplan.Subqueries() {
+				if !qp.HasSubquery(s) {
+					qp.AddSubquery(s, o)
+				}
+			}
+		} else if !this.indexAdvisor {
 			return err
 		}
-		qplan := p.(*plan.QueryPlan)
-		qp.AddSubquery(subq, qplan.PlanOp())
-		for s, o := range qplan.Subqueries() {
-			if !qp.HasSubquery(s) {
-				qp.AddSubquery(s, o)
-			}
+		if this.indexAdvisor {
+			this.endSubqIndexAdvisor(subq)
 		}
-		this.endSubqIndexAdvisor(subq)
 	}
 	return nil
 }

planner/build_advise_ce.go

@@ -23,13 +23,23 @@ import (
 type collectQueryInfo struct {
 }
 
+type saveQueryInfo struct {
+}
+
 func (this *builder) VisitAdvise(stmt *algebra.Advise) (interface{}, error) {
 	return nil, fmt.Errorf("Not supported in CE version. Use https://index-advisor.couchbase.com")
 }
 
 func (this *builder) setAdvisePhase(op int) {
 }
 
+func (this *builder) saveQueryInfo() *saveQueryInfo {
+	return nil
+}
+
+func (this *builder) restoreQueryInfo(saveQInfo *saveQueryInfo) {
+}
+
 func (this *builder) makeSubqueryInfos(l int) {
 }
 

planner/build_advise_ee.go

@@ -218,7 +218,6 @@ type collectQueryInfo struct {
 	keyspaceInfos       advisor.KeyspaceInfos
 	queryInfo           *advisor.QueryInfo
 	queryInfos          map[expression.HasExpressions]*advisor.QueryInfo
-	saveQueryInfos      map[expression.HasExpressions]*advisor.QueryInfo
 	subqueryInfos       map[*algebra.Select]map[expression.HasExpressions]*advisor.QueryInfo
 	indexCollector      *scanIdxCol
 	idxCandidates       []datastore.Index
@@ -228,24 +227,50 @@ type collectQueryInfo struct {
 	advisePhase         int
 }
 
+type saveQueryInfo struct {
+	keyspaceInfos advisor.KeyspaceInfos
+	queryInfo     *advisor.QueryInfo
+	queryInfos    map[expression.HasExpressions]*advisor.QueryInfo
+}
+
 func (this *builder) setAdvisePhase(op int) {
 	this.indexAdvisor = true
 	this.advisePhase = op
 }
 
+func (this *builder) saveQueryInfo() *saveQueryInfo {
+	return &saveQueryInfo{
+		keyspaceInfos: this.keyspaceInfos,
+		queryInfo:     this.queryInfo,
+		queryInfos:    this.queryInfos,
+	}
+}
+
+func (this *builder) restoreQueryInfo(saveQInfo *saveQueryInfo) {
+	if saveQInfo != nil {
+		this.queryInfos = saveQInfo.queryInfos
+		this.keyspaceInfos = saveQInfo.keyspaceInfos
+		this.queryInfo = saveQInfo.queryInfo
+	} else {
+		this.queryInfos = nil
+		this.keyspaceInfos = nil
+		this.queryInfo = nil
+	}
+}
+
 func (this *builder) makeSubqueryInfos(l int) {
 	this.subqueryInfos = make(map[*algebra.Select]map[expression.HasExpressions]*advisor.QueryInfo, l)
 }
 
 func (this *builder) startSubqIndexAdvisor() {
-	this.saveQueryInfos = this.queryInfos
 	this.queryInfos = make(map[expression.HasExpressions]*advisor.QueryInfo, 1)
+	this.keyspaceInfos = nil
+	this.queryInfo = nil
 }
 
 func (this *builder) endSubqIndexAdvisor(s *algebra.Select) {
 	this.subqueryInfos[s] = this.queryInfos
-	this.queryInfos = this.saveQueryInfos
-	this.saveQueryInfos = nil
+	this.queryInfos = nil
 }
 
 func (this *builder) initialIndexAdvisor(stmt algebra.Statement) {