MB-53372 properly formalize select in UDF with variables as non corre…

…lated, avoid caching for UDF select Change-Id: I95e1942592abd20e15dd64d9c0ea375fa63b2922 Reviewed-on: https://review.couchbase.org/c/query/+/178826 Reviewed-by: Bingjie Miao <bingjie.miao@couchbase.com> Tested-by: Marco Greco <marco.greco@couchbase.com>
couchbase · Aug 15, 2022 · 6a6d16c · 6a6d16c
1 parent 0faaadf
commit 6a6d16c
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 15 deletions.
diff --git a/algebra/select.go b/algebra/select.go
@@ -28,13 +28,14 @@ clause.
 type Select struct {
 	statementBase
 
-	subresult  Subresult             `json:"subresult"`
-	with       expression.Bindings   `json:"with"`
-	order      *Order                `json:"order"`
-	offset     expression.Expression `json:"offset"`
-	limit      expression.Expression `json:"limit"`
-	correlated bool                  `json:"correlated"`
-	setop      bool                  `json:"setop"`
+	subresult    Subresult             `json:"subresult"`
+	with         expression.Bindings   `json:"with"`
+	order        *Order                `json:"order"`
+	offset       expression.Expression `json:"offset"`
+	limit        expression.Expression `json:"limit"`
+	correlated   bool                  `json:"correlated"`
+	hasVariables bool                  `json:"hasVariables"` // not actually propagated
+	setop        bool                  `json:"setop"`
 }
 
 /*
@@ -214,6 +215,9 @@ For the subresult of the subquery, call Formalize, for the order
 by clause call MapExpressions, for limit and offset call Accept.
 */
 func (this *Select) FormalizeSubquery(parent *expression.Formalizer) (err error) {
+	if parent != nil && parent.InFunction() {
+		this.hasVariables = true
+	}
 	if parent != nil && !this.setop {
 		withs := parent.SaveWiths()
 		defer parent.RestoreWiths(withs)
@@ -338,6 +342,10 @@ func (this *Select) SetCorrelated() {
 	this.correlated = true
 }
 
+func (this *Select) HasVariables() bool {
+	return this.hasVariables
+}
+
 /*
 this.setop indicates whether the Select is under a set operation (UNION/INTERSECT/EXCEPT)
 */

diff --git a/execution/context.go b/execution/context.go
@@ -886,10 +886,13 @@ func (this *Context) EvaluateSubquery(query *algebra.Select, parent value.Value)
 	var subplan, subplanIsks interface{}
 	planFound := false
 
-	subresults := this.getSubresults()
-	subresult, _, ok := subresults.get(query)
-	if ok {
-		return subresult.(value.Value), nil
+	useCache := !query.IsCorrelated() && !query.HasVariables()
+	if useCache {
+		subresults := this.getSubresults()
+		subresult, _, ok := subresults.get(query)
+		if ok {
+			return subresult.(value.Value), nil
+		}
 	}
 
 	subplans := this.getSubplans()
@@ -1013,7 +1016,8 @@ func (this *Context) EvaluateSubquery(query *algebra.Select, parent value.Value)
 	}
 
 	// Cache results
-	if !query.IsCorrelated() {
+	if useCache {
+		subresults := this.getSubresults()
 		subresults.set(query, results, nil)
 	}
 

diff --git a/expression/formalizer.go b/expression/formalizer.go
@@ -22,6 +22,7 @@ const (
 	FORM_MAP_SELF     = 1 << iota // Map SELF to keyspace: used in sarging index
 	FORM_MAP_KEYSPACE             // Map keyspace to SELF: used in creating index
 	FORM_INDEX_SCOPE              // formalizing index key or index condition
+	FORM_IN_FUNCTION              // We are setting variables for function invocation
 )
 
 const (
@@ -54,9 +55,17 @@ func NewKeyspaceFormalizer(keyspace string, parent *Formalizer) *Formalizer {
 	return newFormalizer(keyspace, parent, false, true)
 }
 
+func NewFunctionFormalizer(keyspace string, parent *Formalizer) *Formalizer {
+	rv := newFormalizer(keyspace, parent, false, false)
+	rv.flags |= FORM_IN_FUNCTION
+	return rv
+}
+
 func newFormalizer(keyspace string, parent *Formalizer, mapSelf, mapKeyspace bool) *Formalizer {
 	var pv, av value.Value
 	var withs map[string]bool
+
+	flags := uint32(0)
 	if parent != nil {
 		pv = parent.allowed
 		av = parent.aliases
@@ -70,7 +79,6 @@ func newFormalizer(keyspace string, parent *Formalizer, mapSelf, mapKeyspace boo
 		}
 	}
 
-	flags := uint32(0)
 	if mapSelf {
 		flags |= FORM_MAP_SELF
 	}
@@ -103,6 +111,10 @@ func (this *Formalizer) mapKeyspace() bool {
 	return (this.flags & FORM_MAP_KEYSPACE) != 0
 }
 
+func (this *Formalizer) InFunction() bool {
+	return (this.flags & FORM_IN_FUNCTION) != 0
+}
+
 func (this *Formalizer) indexScope() bool {
 	return (this.flags & FORM_INDEX_SCOPE) != 0
 }
@@ -562,6 +574,15 @@ func (this *Formalizer) SetWiths(withs Bindings) {
 	}
 }
 
+func (this *Formalizer) SetPermanentWiths(withs Bindings) {
+	if this.withs == nil {
+		this.withs = make(map[string]bool, len(withs))
+	}
+	for _, b := range withs {
+		this.withs[b.Variable()] = true
+	}
+}
+
 func (this *Formalizer) SaveWiths() map[string]bool {
 	withs := this.withs
 	this.withs = make(map[string]bool, len(withs))

diff --git a/functions/inline/inline.go b/functions/inline/inline.go
@@ -71,6 +71,7 @@ func NewInlineBody(expr expression.Expression, text string) (functions.FunctionB
 
 func (this *inlineBody) SetVarNames(vars []string) errors.Error {
 	var bindings expression.Bindings
+	var f *expression.Formalizer
 
 	this.varNames = vars
 
@@ -87,6 +88,7 @@ func (this *inlineBody) SetVarNames(vars []string) errors.Error {
 		args := expression.NewSimpleBinding("args", c)
 		args.SetStatic(true)
 		bindings = expression.Bindings{args}
+		f = expression.NewFormalizer("", nil)
 	} else {
 		bindings = make(expression.Bindings, len(vars))
 		i := 0
@@ -95,10 +97,10 @@ func (this *inlineBody) SetVarNames(vars []string) errors.Error {
 			bindings[i].SetStatic(true)
 			i++
 		}
+		f = expression.NewFunctionFormalizer("", nil)
 	}
 
-	f := expression.NewFormalizer("", nil)
-	f.SetWiths(bindings)
+	f.SetPermanentWiths(bindings)
 	f.PushBindings(bindings, true)
 	_, err := this.expr.Accept(f)
 	if err != nil {