builtins: fix panic for ST_MinimumBoundingCircle with num_segments

Release note (bug fix): Fix a bug where ST_MinimumBoundingCircle would panic with infinite coordinates and a num_segments argument.
cockroachdb · Apr 19, 2022 · b856781 · b856781
1 parent 21738e1
commit b856781
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 6 deletions.
diff --git a/pkg/geo/geomfn/linestring.go b/pkg/geo/geomfn/linestring.go
@@ -60,6 +60,9 @@ func LineMerge(g geo.Geometry) (geo.Geometry, error) {
 	if g.Empty() {
 		return g, nil
 	}
+	if BoundingBoxHasInfiniteCoordinates(g) {
+		return geo.Geometry{}, pgerror.Newf(pgcode.InvalidParameterValue, "value out of range: overflow")
+	}
 	ret, err := geos.LineMerge(g.EWKB())
 	if err != nil {
 		return geo.Geometry{}, err

diff --git a/pkg/geo/geomfn/topology_operations.go b/pkg/geo/geomfn/topology_operations.go
@@ -16,6 +16,8 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/geo"
 	"github.com/cockroachdb/cockroach/pkg/geo/geopb"
 	"github.com/cockroachdb/cockroach/pkg/geo/geos"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
 )
 
 // Boundary returns the boundary of a given Geometry.
@@ -42,6 +44,10 @@ func Centroid(g geo.Geometry) (geo.Geometry, error) {
 
 // MinimumBoundingCircle returns minimum bounding circle of an EWKB
 func MinimumBoundingCircle(g geo.Geometry) (geo.Geometry, geo.Geometry, float64, error) {
+	if BoundingBoxHasInfiniteCoordinates(g) {
+		return geo.Geometry{}, geo.Geometry{}, 0, pgerror.Newf(pgcode.InvalidParameterValue, "value out of range: overflow")
+	}
+
 	polygonEWKB, centroidEWKB, radius, err := geos.MinimumBoundingCircle(g.EWKB())
 	if err != nil {
 		return geo.Geometry{}, geo.Geometry{}, 0, err

diff --git a/pkg/sql/logictest/testdata/logic_test/geospatial b/pkg/sql/logictest/testdata/logic_test/geospatial
@@ -5670,6 +5670,9 @@ true
 statement error st_minimumboundingcircle\(\): value out of range: overflow
 select st_minimumboundingcircle(st_makepoint(((-0.27013513189303495):::FLOAT8::FLOAT8 // 5e-324:::FLOAT8::FLOAT8)::FLOAT8::FLOAT8, (-0.4968052087960828):::FLOAT8::FLOAT8)::GEOMETRY::GEOMETRY)::GEOMETRY
 
+statement error st_minimumboundingcircle\(\): value out of range: overflow
+select st_minimumboundingcircle(st_makepoint(((-0.27013513189303495):::FLOAT8::FLOAT8 // 5e-324:::FLOAT8::FLOAT8)::FLOAT8::FLOAT8, (-0.4968052087960828):::FLOAT8::FLOAT8)::GEOMETRY::GEOMETRY, 10)::GEOMETRY
+
 subtest st_unaryunion
 
 query T

diff --git a/pkg/sql/sem/builtins/geo_builtins.go b/pkg/sql/sem/builtins/geo_builtins.go
@@ -3181,9 +3181,6 @@ Note If the result has zero or one points, it will be returned as a POINT. If it
 		defProps(),
 		geometryOverload1(
 			func(ctx *tree.EvalContext, g *tree.DGeometry) (tree.Datum, error) {
-				if geomfn.BoundingBoxHasInfiniteCoordinates(g.Geometry) {
-					return nil, pgerror.Newf(pgcode.InvalidParameterValue, "value out of range: overflow")
-				}
 				line, err := geomfn.LineMerge(g.Geometry)
 				if err != nil {
 					return nil, err
@@ -6471,9 +6468,6 @@ The parent_only boolean is always ignored.`,
 	"st_minimumboundingcircle": makeBuiltin(defProps(),
 		geometryOverload1(
 			func(evalContext *tree.EvalContext, g *tree.DGeometry) (tree.Datum, error) {
-				if geomfn.BoundingBoxHasInfiniteCoordinates(g.Geometry) {
-					return nil, pgerror.Newf(pgcode.InvalidParameterValue, "value out of range: overflow")
-				}
 				polygon, _, _, err := geomfn.MinimumBoundingCircle(g.Geometry)
 				if err != nil {
 					return nil, err