From da906a7e085c867a35f5623b509665b7a9323701 Mon Sep 17 00:00:00 2001
From: Rebecca Taft <becca@cockroachlabs.com>
Date: Tue, 13 Apr 2021 13:55:48 -0500
Subject: [PATCH] sql: fix index out of range error in inverted filterer

Prior to this commit, it was possible that the SpansToRead in
an inverted.SpanExpression were a superset of the spans that were
needed to evaluate the span expression. This could result in an index
out of range error in the invertedFilterer when a span from the input
didn't exist in its list of needed spans. This commit fixes the
problem by ensuring that SpansToRead does not contain spans that are
not needed to evaluate the span expression.

Fixes #63180

Release note (bug fix): Fixed an internal error that could occur
when executing queries using an inverted index. The error was an index
out of range error, and could occur in rare cases when a filter or
join predicate contained at least two JSON, Array, Geometry or
Geography expressions that were combined with AND. This has now been
fixed.
---
 pkg/sql/opt/invertedexpr/expression.go       | 26 +++++++++++++++++---
 pkg/sql/opt/invertedexpr/testdata/expression |  4 +--
 pkg/sql/opt/xform/testdata/rules/select      |  3 ++-
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/pkg/sql/opt/invertedexpr/expression.go b/pkg/sql/opt/invertedexpr/expression.go
index faa63e3a0f37..c81d813da4b3 100644
--- a/pkg/sql/opt/invertedexpr/expression.go
+++ b/pkg/sql/opt/invertedexpr/expression.go
@@ -617,11 +617,16 @@ func opSpanExpressionAndDefault(
 
 // Intersects two SpanExpressions.
 func intersectSpanExpressions(left, right *SpanExpression) *SpanExpression {
-	// Since we simply union into SpansToRead, we can end up with
-	// FactoredUnionSpans as a subset of SpanToRead *and* both children pruned.
-	// TODO(sumeer): tighten the SpansToRead for this case.
 	expr := &SpanExpression{
-		Tight:              left.Tight && right.Tight,
+		Tight: left.Tight && right.Tight,
+
+		// We calculate SpansToRead as the union of the left and right sides as a
+		// first approximation, but this may result in too many spans if either of
+		// the children are pruned below. SpansToRead will be recomputed in
+		// tryPruneChildren if needed. (It is important that SpansToRead be exactly
+		// what would be computed if a caller traversed the tree and explicitly
+		// unioned all the FactoredUnionSpans, and no looser, since the execution
+		// code path relies on this property.)
 		SpansToRead:        unionSpans(left.SpansToRead, right.SpansToRead),
 		FactoredUnionSpans: intersectSpans(left.FactoredUnionSpans, right.FactoredUnionSpans),
 		Operator:           SetIntersection,
@@ -675,6 +680,19 @@ func tryPruneChildren(expr *SpanExpression, op SetOperator) {
 		expr.Operator = child.Operator
 		expr.Left = child.Left
 		expr.Right = child.Right
+
+		// If child.FactoredUnionSpans is non-empty, we need to recalculate
+		// SpansToRead since it may have contained some spans that were removed by
+		// discarding child.FactoredUnionSpans.
+		if child.FactoredUnionSpans != nil {
+			expr.SpansToRead = expr.FactoredUnionSpans
+			if expr.Left != nil {
+				expr.SpansToRead = unionSpans(expr.SpansToRead, expr.Left.(*SpanExpression).SpansToRead)
+			}
+			if expr.Right != nil {
+				expr.SpansToRead = unionSpans(expr.SpansToRead, expr.Right.(*SpanExpression).SpansToRead)
+			}
+		}
 	}
 	promoteLeft := expr.Left != nil && expr.Right == nil
 	promoteRight := expr.Left == nil && expr.Right != nil
diff --git a/pkg/sql/opt/invertedexpr/testdata/expression b/pkg/sql/opt/invertedexpr/testdata/expression
index 03ed544485e8..948f94a8d3dd 100644
--- a/pkg/sql/opt/invertedexpr/testdata/expression
+++ b/pkg/sql/opt/invertedexpr/testdata/expression
@@ -242,7 +242,7 @@ and result=_ left=b right=ac
 ----
 span expression
  ├── tight: true
- ├── to read: ["a", "c")
+ ├── to read: ["b", "b"]
  └── union spans: ["b", "b"]
 
 new-span-leaf name=bj tight=true span=b,j
@@ -265,7 +265,7 @@ and result=_ left=b right=bj
 ----
 span expression
  ├── tight: true
- ├── to read: ["b", "j")
+ ├── to read: ["b", "b"]
  └── union spans: ["b", "b"]
 
 # [b, j) or [a, c) = [a, j)
diff --git a/pkg/sql/opt/xform/testdata/rules/select b/pkg/sql/opt/xform/testdata/rules/select
index 30cd50adbd8a..fe29333609f0 100644
--- a/pkg/sql/opt/xform/testdata/rules/select
+++ b/pkg/sql/opt/xform/testdata/rules/select
@@ -2009,7 +2009,8 @@ project
       │              ├── inverted constraint: /7/1
       │              │    └── spans
       │              │         ├── ["B\xfdL\x00\x00\x00\x00\x00\x00\x00", "B\xfdL\x00\x00\x00\x00\x00\x00\x00"]
-      │              │         └── ["B\xfdN\x00\x00\x00\x00\x00\x00\x01", "B\xfdP\x00\x00\x00\x00\x00\x00\x01")
+      │              │         ├── ["B\xfdO\x00\x00\x00\x00\x00\x00\x00", "B\xfdO\x00\x00\x00\x00\x00\x00\x00"]
+      │              │         └── ["B\xfdP\x00\x00\x00\x00\x00\x00\x00", "B\xfdP\x00\x00\x00\x00\x00\x00\x00"]
       │              ├── key: (1)
       │              └── fd: (1)-->(7)
       └── filters