opt: fix panic in GenerateInvertedIndexScans

This commit fixes a nil-pointer exception caused when empty index spans
were generated from a filter on an inverted column.

Fixes #59702

Release note (bug fix): A bug has been fixed that caused errors for some
queries on tables with GEOMETRY or GEOGRAPHY inverted indexes with
filters containing shapes with zero area.

pkg/sql/opt/invertedexpr/geo_expression.go

@@ -61,9 +61,9 @@ func geoToSpan(span geoindex.KeySpan, b []byte) (inverted.Span, []byte) {
 
 // GeoUnionKeySpansToSpanExpr converts geoindex.UnionKeySpans to a
 // SpanExpression.
-func GeoUnionKeySpansToSpanExpr(ukSpans geoindex.UnionKeySpans) *inverted.SpanExpression {
+func GeoUnionKeySpansToSpanExpr(ukSpans geoindex.UnionKeySpans) inverted.Expression {
 	if len(ukSpans) == 0 {
-		return nil
+		return inverted.NonInvertedColExpression{}
 	}
 	// Avoid per-span heap allocations. Each of the 2 keys in a span is the
 	// geoInvertedIndexMarker (1 byte) followed by a varint.
@@ -79,9 +79,9 @@ func GeoUnionKeySpansToSpanExpr(ukSpans geoindex.UnionKeySpans) *inverted.SpanEx
 }
 
 // GeoRPKeyExprToSpanExpr converts geoindex.RPKeyExpr to SpanExpression.
-func GeoRPKeyExprToSpanExpr(rpExpr geoindex.RPKeyExpr) (*inverted.SpanExpression, error) {
+func GeoRPKeyExprToSpanExpr(rpExpr geoindex.RPKeyExpr) (inverted.Expression, error) {
 	if len(rpExpr) == 0 {
-		return nil, nil
+		return inverted.NonInvertedColExpression{}, nil
 	}
 	spansToRead := make([]inverted.Span, 0, len(rpExpr))
 	var b []byte // avoid per-expr heap allocations

pkg/sql/opt/invertedidx/geo.go

@@ -63,7 +63,7 @@ func GetGeoIndexRelationship(expr opt.ScalarExpr) (_ geoindex.RelationshipType,
 // and getSpanExprForGeometryIndex and used in extractGeoFilterCondition.
 type getSpanExprForGeoIndexFn func(
 	context.Context, tree.Datum, []tree.Datum, geoindex.RelationshipType, *geoindex.Config,
-) *inverted.SpanExpression
+) inverted.Expression
 
 // getSpanExprForGeographyIndex gets a SpanExpression that constrains the given
 // geography index according to the given constant and geospatial relationship.
@@ -73,27 +73,28 @@ func getSpanExprForGeographyIndex(
 	additionalParams []tree.Datum,
 	relationship geoindex.RelationshipType,
 	indexConfig *geoindex.Config,
-) *inverted.SpanExpression {
+) inverted.Expression {
 	geogIdx := geoindex.NewS2GeographyIndex(*indexConfig.S2Geography)
 	geog := d.(*tree.DGeography).Geography
-	var spanExpr *inverted.SpanExpression
 
 	switch relationship {
 	case geoindex.Covers:
 		unionKeySpans, err := geogIdx.Covers(ctx, geog)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	case geoindex.CoveredBy:
 		rpKeyExpr, err := geogIdx.CoveredBy(ctx, geog)
 		if err != nil {
 			panic(err)
 		}
-		if spanExpr, err = invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr); err != nil {
+		spanExpr, err := invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr)
+		if err != nil {
 			panic(err)
 		}
+		return spanExpr
 
 	case geoindex.DWithin:
 		// Parameters are type checked earlier. Keep this consistent with the definition
@@ -122,20 +123,18 @@ func getSpanExprForGeographyIndex(
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	case geoindex.Intersects:
 		unionKeySpans, err := geogIdx.Intersects(ctx, geog)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	default:
 		panic(errors.AssertionFailedf("unhandled relationship: %v", relationship))
 	}
-
-	return spanExpr
 }
 
 // Helper for DWithin and DFullyWithin.
@@ -160,56 +159,55 @@ func getSpanExprForGeometryIndex(
 	additionalParams []tree.Datum,
 	relationship geoindex.RelationshipType,
 	indexConfig *geoindex.Config,
-) *inverted.SpanExpression {
+) inverted.Expression {
 	geomIdx := geoindex.NewS2GeometryIndex(*indexConfig.S2Geometry)
 	geom := d.(*tree.DGeometry).Geometry
-	var spanExpr *inverted.SpanExpression
 
 	switch relationship {
 	case geoindex.Covers:
 		unionKeySpans, err := geomIdx.Covers(ctx, geom)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	case geoindex.CoveredBy:
 		rpKeyExpr, err := geomIdx.CoveredBy(ctx, geom)
 		if err != nil {
 			panic(err)
 		}
-		if spanExpr, err = invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr); err != nil {
+		spanExpr, err := invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr)
+		if err != nil {
 			panic(err)
 		}
+		return spanExpr
 
 	case geoindex.DFullyWithin:
 		distance := getDistanceParam(additionalParams)
 		unionKeySpans, err := geomIdx.DFullyWithin(ctx, geom, distance)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	case geoindex.DWithin:
 		distance := getDistanceParam(additionalParams)
 		unionKeySpans, err := geomIdx.DWithin(ctx, geom, distance)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	case geoindex.Intersects:
 		unionKeySpans, err := geomIdx.Intersects(ctx, geom)
 		if err != nil {
 			panic(err)
 		}
-		spanExpr = invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
+		return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
 
 	default:
 		panic(errors.AssertionFailedf("unhandled relationship: %v", relationship))
 	}
-
-	return spanExpr
 }
 
 type geoJoinPlanner struct {
@@ -527,7 +525,7 @@ type geoInvertedExpr struct {
 
 	// spanExpr is the result of evaluating the geospatial relationship
 	// represented by this geoInvertedExpr. It is nil prior to evaluation.
-	spanExpr *inverted.SpanExpression
+	spanExpr inverted.Expression
 }
 
 var _ tree.TypedExpr = &geoInvertedExpr{}
@@ -803,9 +801,10 @@ func NewGeoDatumsToInvertedExpr(
 
 			// If possible, get the span expression now so we don't need to recompute
 			// it for every row.
-			var spanExpr *inverted.SpanExpression
+			var spanExpr inverted.Expression
 			if d, ok := nonIndexParam.(tree.Datum); ok {
 				spanExpr = g.getSpanExpr(evalCtx.Ctx(), d, additionalParams, relationship, g.indexConfig)
+
 			} else if funcExprCount == 1 {
 				// Currently pre-filtering is limited to a single FuncExpr.
 				preFilterRelationship = relationship

pkg/sql/opt/xform/testdata/rules/select

@@ -4067,6 +4067,31 @@ exec-ddl
 DROP INDEX mc_idx
 ----
 
+# Regression test for #59702. Do not panic when empty index spans are generated
+# from a filter on an inverted column.
+exec-ddl
+CREATE TABLE t59702 (
+  k INT PRIMARY KEY,
+  g GEOGRAPHY,
+  INVERTED INDEX (g)
+)
+----
+
+opt
+SELECT * FROM t59702 WHERE st_intersects(g, st_buffer(st_makepoint(1, 1)::GEOGRAPHY, 0))
+----
+select
+ ├── columns: k:1!null g:2!null
+ ├── immutable
+ ├── key: (1)
+ ├── fd: (1)-->(2)
+ ├── scan t59702
+ │    ├── columns: k:1!null g:2
+ │    ├── key: (1)
+ │    └── fd: (1)-->(2)
+ └── filters
+      └── st_intersects(g:2, '0103000020E610000000000000') [outer=(2), immutable, constraints=(/2: (/NULL - ])]
+
 # --------------------------------------------------
 # GenerateZigzagJoins
 # --------------------------------------------------