Skip to content

invertedidx: harden geo code to avoid panicking #154917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 7, 2025
Merged

invertedidx: harden geo code to avoid panicking #154917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/inverted_index
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2119,3 +2119,25 @@ c" STRING
statement ok
CREATE INVERTED INDEX ON t126444 ("b
c" gin_trgm_ops)

# Regression test for panicking with some GEOS errors (#151995).
statement ok
CREATE TABLE t151995 (
c0 INT PRIMARY KEY,
c1 GEOMETRY,
INVERTED INDEX (c1 ASC)
);

statement ok
INSERT INTO t151995 (c0, c1) VALUES (
0,
'010300004001000000040000004ED667271F45EEC180F702131A41A7C138EDC3641D46F041E9E58A67A768F0C1CBB3A399C439FEC1189C38E3BBDDEEC16074F4D5F0C7FA414CCB652363EC01429029CED0E787E1C14ED667271F45EEC180F702131A41A7C138EDC3641D46F041':::GEOMETRY
);

statement ok
ANALYZE t151995;

statement error geos error: IllegalArgumentException: BufferOp::getResultGeometry distance must be a finite value
SELECT *
FROM t151995 AS t1
JOIN t151995 AS t2 ON st_dfullywithinexclusive(t2.c1, t1.c1, '+Inf');
74 changes: 41 additions & 33 deletions pkg/sql/opt/invertedidx/geo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,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, geopb.Config,
) inverted.Expression
) (inverted.Expression, error)

// getSpanExprForGeographyIndex gets a SpanExpression that constrains the given
// geography index according to the given constant and geospatial relationship.
Expand All @@ -71,67 +71,67 @@ func getSpanExprForGeographyIndex(
additionalParams []tree.Datum,
relationship geoindex.RelationshipType,
indexConfig geopb.Config,
) inverted.Expression {
) (inverted.Expression, error) {
geogIdx := geoindex.NewS2GeographyIndex(*indexConfig.S2Geography)
geog := d.(*tree.DGeography).Geography

switch relationship {
case geoindex.Covers:
unionKeySpans, err := geogIdx.Covers(ctx, geog)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

case geoindex.CoveredBy:
rpKeyExpr, err := geogIdx.CoveredBy(ctx, geog)
if err != nil {
panic(err)
return nil, err
}
spanExpr, err := invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr)
if err != nil {
panic(err)
return nil, err
}
return spanExpr
return spanExpr, nil

case geoindex.DWithin:
// Parameters are type checked earlier. Keep this consistent with the definition
// in geo_builtins.go.
if len(additionalParams) != 1 && len(additionalParams) != 2 {
panic(errors.AssertionFailedf("unexpected param length %d", len(additionalParams)))
return nil, errors.AssertionFailedf("unexpected param length %d", len(additionalParams))
}
d, ok := additionalParams[0].(*tree.DFloat)
if !ok {
panic(errors.AssertionFailedf(
"parameter %s is not float", additionalParams[0].ResolvedType()))
return nil, errors.AssertionFailedf(
"parameter %s is not float", additionalParams[0].ResolvedType())
}
distanceMeters := float64(*d)
useSpheroid := geogfn.UseSpheroid
if len(additionalParams) == 2 {
b, ok := additionalParams[1].(*tree.DBool)
if !ok {
panic(errors.AssertionFailedf(
"parameter %s is not bool", additionalParams[1].ResolvedType()))
return nil, errors.AssertionFailedf(
"parameter %s is not bool", additionalParams[1].ResolvedType())
}
if !*b {
useSpheroid = geogfn.UseSphere
}
}
unionKeySpans, err := geogIdx.DWithin(ctx, geog, distanceMeters, useSpheroid)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

case geoindex.Intersects:
unionKeySpans, err := geogIdx.Intersects(ctx, geog)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

default:
panic(errors.AssertionFailedf("unhandled relationship: %v", relationship))
return nil, errors.AssertionFailedf("unhandled relationship: %v", relationship)
}
}

Expand All @@ -158,54 +158,54 @@ func getSpanExprForGeometryIndex(
additionalParams []tree.Datum,
relationship geoindex.RelationshipType,
indexConfig geopb.Config,
) inverted.Expression {
) (inverted.Expression, error) {
geomIdx := geoindex.NewS2GeometryIndex(*indexConfig.S2Geometry)
geom := d.(*tree.DGeometry).Geometry

switch relationship {
case geoindex.Covers:
unionKeySpans, err := geomIdx.Covers(ctx, geom)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

case geoindex.CoveredBy:
rpKeyExpr, err := geomIdx.CoveredBy(ctx, geom)
if err != nil {
panic(err)
return nil, err
}
spanExpr, err := invertedexpr.GeoRPKeyExprToSpanExpr(rpKeyExpr)
if err != nil {
panic(err)
return nil, err
}
return spanExpr
return spanExpr, nil

case geoindex.DFullyWithin:
distance := getDistanceParam(additionalParams)
unionKeySpans, err := geomIdx.DFullyWithin(ctx, geom, distance)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

case geoindex.DWithin:
distance := getDistanceParam(additionalParams)
unionKeySpans, err := geomIdx.DWithin(ctx, geom, distance)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

case geoindex.Intersects:
unionKeySpans, err := geomIdx.Intersects(ctx, geom)
if err != nil {
panic(err)
return nil, err
}
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans)
return invertedexpr.GeoUnionKeySpansToSpanExpr(unionKeySpans), nil

default:
panic(errors.AssertionFailedf("unhandled relationship: %v", relationship))
return nil, errors.AssertionFailedf("unhandled relationship: %v", relationship)
}
}

Expand Down Expand Up @@ -630,7 +630,11 @@ func extractGeoFilterCondition(
preFilterExpr :=
makeExprFromRelationshipAndParams(factory, expr, args, commuteArgs, relationship)

return getSpanExpr(ctx, d, additionalParams, relationship, index.GeoConfig()),
invExpr, err := getSpanExpr(ctx, d, additionalParams, relationship, index.GeoConfig())
if err != nil {
return inverted.NonInvertedColExpression{}, nil
}
return invExpr,
&invertedexpr.PreFiltererStateForInvertedFilterer{
Expr: preFilterExpr,
Col: arg2.Col,
Expand Down Expand Up @@ -1022,7 +1026,11 @@ func NewGeoDatumsToInvertedExpr(
// it for every row.
var invertedExpr inverted.Expression
if d, ok := nonIndexParam.(tree.Datum); ok {
invertedExpr = g.getSpanExpr(ctx, d, additionalParams, relationship, g.indexConfig)
var err error
invertedExpr, err = g.getSpanExpr(ctx, d, additionalParams, relationship, g.indexConfig)
if err != nil {
return nil, err
}
} else if funcExprCount == 1 {
// Currently pre-filtering is limited to a single FuncExpr.
preFilterRelationship = relationship
Expand Down Expand Up @@ -1079,7 +1087,7 @@ func (g *geoDatumsToInvertedExpr) Convert(
if g.filterer != nil {
preFilterState = g.filterer.Bind(d)
}
return g.getSpanExpr(ctx, d, t.additionalParams, t.relationship, g.indexConfig), nil
return g.getSpanExpr(ctx, d, t.additionalParams, t.relationship, g.indexConfig)

default:
return nil, fmt.Errorf("unsupported expression %v", t)
Expand Down