diff --git a/pkg/geo/geodist/geodist.go b/pkg/geo/geodist/geodist.go index a5f35b68d831..9aaf9f52fdd6 100644 --- a/pkg/geo/geodist/geodist.go +++ b/pkg/geo/geodist/geodist.go @@ -11,14 +11,24 @@ // Package geodist finds distances between two geospatial shapes. package geodist -import "github.com/cockroachdb/errors" +import ( + "github.com/cockroachdb/errors" + "github.com/golang/geo/s2" + "github.com/twpayne/go-geom" +) -// Point is an interface that represents a geospatial Point. -type Point interface { - IsShape() - IsPoint() +// Point is a union of the point types used in geometry and geography representation. +// The interfaces for distance calculation defined below are shared for both representations, +// and this union helps us avoid heap allocations by doing cheap copy-by-value of points. The +// code that peers inside a Point knows which of the two fields is populated. +type Point struct { + GeomPoint geom.Coord + GeogPoint s2.Point } +// IsShape implements the geodist.Shape interface. +func (p *Point) IsShape() {} + // Edge is a struct that represents a connection between two points. type Edge struct { V0, V1 Point @@ -63,7 +73,7 @@ type Shape interface { IsShape() } -var _ Shape = (Point)(nil) +var _ Shape = (*Point)(nil) var _ Shape = (LineString)(nil) var _ Shape = (LinearRing)(nil) var _ Shape = (Polygon)(nil) @@ -122,24 +132,24 @@ type DistanceCalculator interface { // It returns whether the function above should return early. func ShapeDistance(c DistanceCalculator, a Shape, b Shape) (bool, error) { switch a := a.(type) { - case Point: + case *Point: switch b := b.(type) { - case Point: - return c.DistanceUpdater().Update(a, b), nil + case *Point: + return c.DistanceUpdater().Update(*a, *b), nil case LineString: - return onPointToLineString(c, a, b), nil + return onPointToLineString(c, *a, b), nil case Polygon: - return onPointToPolygon(c, a, b), nil + return onPointToPolygon(c, *a, b), nil default: return false, errors.Newf("unknown shape: %T", b) } case LineString: switch b := b.(type) { - case Point: + case *Point: c.DistanceUpdater().FlipGeometries() // defer to restore the order of geometries at the end of the function call. defer c.DistanceUpdater().FlipGeometries() - return onPointToLineString(c, b, a), nil + return onPointToLineString(c, *b, a), nil case LineString: return onShapeEdgesToShapeEdges(c, a, b), nil case Polygon: @@ -149,11 +159,11 @@ func ShapeDistance(c DistanceCalculator, a Shape, b Shape) (bool, error) { } case Polygon: switch b := b.(type) { - case Point: + case *Point: c.DistanceUpdater().FlipGeometries() // defer to restore the order of geometries at the end of the function call. defer c.DistanceUpdater().FlipGeometries() - return onPointToPolygon(c, b, a), nil + return onPointToPolygon(c, *b, a), nil case LineString: c.DistanceUpdater().FlipGeometries() // defer to restore the order of geometries at the end of the function call. diff --git a/pkg/geo/geogfn/distance.go b/pkg/geo/geogfn/distance.go index 990559caf4b6..39c350c637bb 100644 --- a/pkg/geo/geogfn/distance.go +++ b/pkg/geo/geogfn/distance.go @@ -55,19 +55,6 @@ func Distance( // Spheroids // -// s2GeodistPoint implements geodist.Point. -type s2GeodistPoint struct { - s2.Point -} - -var _ geodist.Point = (*s2GeodistPoint)(nil) - -// IsShape implements the geodist.Point interface. -func (*s2GeodistPoint) IsShape() {} - -// Point implements the geodist.Point interface. -func (*s2GeodistPoint) IsPoint() {} - // s2GeodistLineString implements geodist.LineString. type s2GeodistLineString struct { *s2.Polyline @@ -83,7 +70,10 @@ func (*s2GeodistLineString) IsLineString() {} // Edge implements the geodist.LineString interface. func (g *s2GeodistLineString) Edge(i int) geodist.Edge { - return geodist.Edge{V0: &s2GeodistPoint{Point: (*g.Polyline)[i]}, V1: &s2GeodistPoint{Point: (*g.Polyline)[i+1]}} + return geodist.Edge{ + V0: geodist.Point{GeogPoint: (*g.Polyline)[i]}, + V1: geodist.Point{GeogPoint: (*g.Polyline)[i+1]}, + } } // NumEdges implements the geodist.LineString interface. @@ -93,7 +83,9 @@ func (g *s2GeodistLineString) NumEdges() int { // Vertex implements the geodist.LineString interface. func (g *s2GeodistLineString) Vertex(i int) geodist.Point { - return &s2GeodistPoint{Point: (*g.Polyline)[i]} + return geodist.Point{ + GeogPoint: (*g.Polyline)[i], + } } // NumVertexes implements the geodist.LineString interface. @@ -116,7 +108,10 @@ func (*s2GeodistLinearRing) IsLinearRing() {} // Edge implements the geodist.LinearRing interface. func (g *s2GeodistLinearRing) Edge(i int) geodist.Edge { - return geodist.Edge{V0: &s2GeodistPoint{Point: g.Loop.Vertex(i)}, V1: &s2GeodistPoint{Point: g.Loop.Vertex(i + 1)}} + return geodist.Edge{ + V0: geodist.Point{GeogPoint: g.Loop.Vertex(i)}, + V1: geodist.Point{GeogPoint: g.Loop.Vertex(i + 1)}, + } } // NumEdges implements the geodist.LinearRing interface. @@ -126,7 +121,9 @@ func (g *s2GeodistLinearRing) NumEdges() int { // Vertex implements the geodist.LinearRing interface. func (g *s2GeodistLinearRing) Vertex(i int) geodist.Point { - return &s2GeodistPoint{Point: g.Loop.Vertex(i)} + return geodist.Point{ + GeogPoint: g.Loop.Vertex(i), + } } // NumVertexes implements the geodist.LinearRing interface. @@ -168,7 +165,7 @@ var _ geodist.EdgeCrosser = (*s2GeodistEdgeCrosser)(nil) func (c *s2GeodistEdgeCrosser) ChainCrossing(p geodist.Point) (bool, geodist.Point) { // Returns nil for the intersection point as we don't require the intersection // point as we do not have to implement ShortestLine in geography. - return c.EdgeCrosser.ChainCrossingSign(p.(*s2GeodistPoint).Point) != s2.DoNotCross, nil + return c.EdgeCrosser.ChainCrossingSign(p.GeogPoint) != s2.DoNotCross, geodist.Point{} } // distanceGeographyRegions calculates the distance between two sets of regions. @@ -269,11 +266,9 @@ func (u *geographyMinDistanceUpdater) Distance() float64 { } // Update implements the geodist.DistanceUpdater interface. -func (u *geographyMinDistanceUpdater) Update( - aInterface geodist.Point, bInterface geodist.Point, -) bool { - a := aInterface.(*s2GeodistPoint).Point - b := bInterface.(*s2GeodistPoint).Point +func (u *geographyMinDistanceUpdater) Update(aPoint geodist.Point, bPoint geodist.Point) bool { + a := aPoint.GeogPoint + b := bPoint.GeogPoint sphereDistance := s2.ChordAngleBetweenPoints(a, b) if sphereDistance < u.minD { @@ -331,9 +326,9 @@ func (c *geographyDistanceCalculator) NewEdgeCrosser( ) geodist.EdgeCrosser { return &s2GeodistEdgeCrosser{ EdgeCrosser: s2.NewChainEdgeCrosser( - edge.V0.(*s2GeodistPoint).Point, - edge.V1.(*s2GeodistPoint).Point, - startPoint.(*s2GeodistPoint).Point, + edge.V0.GeogPoint, + edge.V1.GeogPoint, + startPoint.GeogPoint, ), } } @@ -342,7 +337,7 @@ func (c *geographyDistanceCalculator) NewEdgeCrosser( func (c *geographyDistanceCalculator) PointInLinearRing( point geodist.Point, polygon geodist.LinearRing, ) bool { - return polygon.(*s2GeodistLinearRing).ContainsPoint(point.(*s2GeodistPoint).Point) + return polygon.(*s2GeodistLinearRing).ContainsPoint(point.GeogPoint) } // ClosestPointToEdge implements geodist.DistanceCalculator. @@ -355,11 +350,10 @@ func (c *geographyDistanceCalculator) PointInLinearRing( // For visualization and more, see: Section 6 / Figure 4 of // "Projective configuration theorems: old wine into new wineskins", Tabachnikov, Serge, 2016/07/16 func (c *geographyDistanceCalculator) ClosestPointToEdge( - edge geodist.Edge, pointInterface geodist.Point, + edge geodist.Edge, point geodist.Point, ) (geodist.Point, bool) { - eV0 := edge.V0.(*s2GeodistPoint).Point - eV1 := edge.V1.(*s2GeodistPoint).Point - point := pointInterface.(*s2GeodistPoint).Point + eV0 := edge.V0.GeogPoint + eV1 := edge.V1.GeogPoint // Project the point onto the normal of the edge. A great circle passing through // the normal and the point will intersect with the great circle represented @@ -368,21 +362,21 @@ func (c *geographyDistanceCalculator) ClosestPointToEdge( // To find the point where the great circle represented by the edge and the // great circle represented by (normal, point), we project the point // onto the normal. - normalScaledToPoint := normal.Mul(normal.Dot(point.Vector)) + normalScaledToPoint := normal.Mul(normal.Dot(point.GeogPoint.Vector)) // The difference between the point and the projection of the normal when normalized // should give us a point on the great circle which contains the vertexes of the edge. - closestPoint := s2.Point{Vector: point.Vector.Sub(normalScaledToPoint).Normalize()} + closestPoint := s2.Point{Vector: point.GeogPoint.Vector.Sub(normalScaledToPoint).Normalize()} // We then check whether the given point lies on the geodesic of the edge, // as the above algorithm only generates a point on the great circle // represented by the edge. - return &s2GeodistPoint{Point: closestPoint}, (&s2.Polyline{eV0, eV1}).IntersectsCell(s2.CellFromPoint(closestPoint)) + return geodist.Point{GeogPoint: closestPoint}, (&s2.Polyline{eV0, eV1}).IntersectsCell(s2.CellFromPoint(closestPoint)) } // regionToGeodistShape converts the s2 Region to a geodist object. func regionToGeodistShape(r s2.Region) (geodist.Shape, error) { switch r := r.(type) { case s2.Point: - return &s2GeodistPoint{Point: r}, nil + return &geodist.Point{GeogPoint: r}, nil case *s2.Polyline: return &s2GeodistLineString{Polyline: r}, nil case *s2.Polygon: diff --git a/pkg/geo/geomfn/distance.go b/pkg/geo/geomfn/distance.go index ed7ad8be826d..8963329a0846 100644 --- a/pkg/geo/geomfn/distance.go +++ b/pkg/geo/geomfn/distance.go @@ -212,7 +212,7 @@ func distanceInternal( func geomToGeodist(g geom.T) (geodist.Shape, error) { switch g := g.(type) { case *geom.Point: - return &geomGeodistPoint{Coord: g.Coords()}, nil + return &geodist.Point{GeomPoint: g.Coords()}, nil case *geom.LineString: return &geomGeodistLineString{LineString: g}, nil case *geom.Polygon: @@ -221,19 +221,6 @@ func geomToGeodist(g geom.T) (geodist.Shape, error) { return nil, errors.Newf("could not find shape: %T", g) } -// geomGeodistPoint implements geodist.Point. -type geomGeodistPoint struct { - geom.Coord -} - -var _ geodist.Point = (*geomGeodistPoint)(nil) - -// IsShape implements the geodist.Point interface. -func (*geomGeodistPoint) IsShape() {} - -// Point implements the geodist.Point interface. -func (*geomGeodistPoint) IsPoint() {} - // geomGeodistLineString implements geodist.LineString. type geomGeodistLineString struct { *geom.LineString @@ -250,8 +237,8 @@ func (*geomGeodistLineString) IsLineString() {} // Edge implements the geodist.LineString interface. func (g *geomGeodistLineString) Edge(i int) geodist.Edge { return geodist.Edge{ - V0: &geomGeodistPoint{Coord: g.LineString.Coord(i)}, - V1: &geomGeodistPoint{Coord: g.LineString.Coord(i + 1)}, + V0: geodist.Point{GeomPoint: g.LineString.Coord(i)}, + V1: geodist.Point{GeomPoint: g.LineString.Coord(i + 1)}, } } @@ -262,7 +249,7 @@ func (g *geomGeodistLineString) NumEdges() int { // Vertex implements the geodist.LineString interface. func (g *geomGeodistLineString) Vertex(i int) geodist.Point { - return &geomGeodistPoint{Coord: g.LineString.Coord(i)} + return geodist.Point{GeomPoint: g.LineString.Coord(i)} } // NumVertexes implements the geodist.LineString interface. @@ -286,8 +273,8 @@ func (*geomGeodistLinearRing) IsLinearRing() {} // Edge implements the geodist.LinearRing interface. func (g *geomGeodistLinearRing) Edge(i int) geodist.Edge { return geodist.Edge{ - V0: &geomGeodistPoint{Coord: g.LinearRing.Coord(i)}, - V1: &geomGeodistPoint{Coord: g.LinearRing.Coord(i + 1)}, + V0: geodist.Point{GeomPoint: g.LinearRing.Coord(i)}, + V1: geodist.Point{GeomPoint: g.LinearRing.Coord(i + 1)}, } } @@ -298,7 +285,7 @@ func (g *geomGeodistLinearRing) NumEdges() int { // Vertex implements the geodist.LinearRing interface. func (g *geomGeodistLinearRing) Vertex(i int) geodist.Point { - return &geomGeodistPoint{Coord: g.LinearRing.Coord(i)} + return geodist.Point{GeomPoint: g.LinearRing.Coord(i)} } // NumVertexes implements the geodist.LinearRing interface. @@ -341,7 +328,7 @@ var _ geodist.EdgeCrosser = (*geomGeodistEdgeCrosser)(nil) // ChainCrossing implements geodist.EdgeCrosser. func (c *geomGeodistEdgeCrosser) ChainCrossing(p geodist.Point) (bool, geodist.Point) { - nextEdgeV1 := p.(*geomGeodistPoint).Coord + nextEdgeV1 := p.GeomPoint result := lineintersector.LineIntersectsLine( c.strategy, c.edgeV0, @@ -351,9 +338,9 @@ func (c *geomGeodistEdgeCrosser) ChainCrossing(p geodist.Point) (bool, geodist.P ) c.nextEdgeV0 = nextEdgeV1 if result.HasIntersection() { - return true, &geomGeodistPoint{result.Intersection()[0]} + return true, geodist.Point{GeomPoint: result.Intersection()[0]} } - return false, nil + return false, geodist.Point{} } // geomMinDistanceUpdater finds the minimum distance using geom calculations. @@ -390,9 +377,9 @@ func (u *geomMinDistanceUpdater) Distance() float64 { } // Update implements the geodist.DistanceUpdater interface. -func (u *geomMinDistanceUpdater) Update(aInterface geodist.Point, bInterface geodist.Point) bool { - a := aInterface.(*geomGeodistPoint).Coord - b := bInterface.(*geomGeodistPoint).Coord +func (u *geomMinDistanceUpdater) Update(aPoint geodist.Point, bPoint geodist.Point) bool { + a := aPoint.GeomPoint + b := bPoint.GeomPoint dist := coordNorm(coordSub(a, b)) if dist < u.currentValue { @@ -411,8 +398,8 @@ func (u *geomMinDistanceUpdater) Update(aInterface geodist.Point, bInterface geo // OnIntersects implements the geodist.DistanceUpdater interface. func (u *geomMinDistanceUpdater) OnIntersects(p geodist.Point) bool { - u.coordA = p.(*geomGeodistPoint).Coord - u.coordB = p.(*geomGeodistPoint).Coord + u.coordA = p.GeomPoint + u.coordB = p.GeomPoint u.currentValue = 0 return true } @@ -464,9 +451,9 @@ func (u *geomMaxDistanceUpdater) Distance() float64 { } // Update implements the geodist.DistanceUpdater interface. -func (u *geomMaxDistanceUpdater) Update(aInterface geodist.Point, bInterface geodist.Point) bool { - a := aInterface.(*geomGeodistPoint).Coord - b := bInterface.(*geomGeodistPoint).Coord +func (u *geomMaxDistanceUpdater) Update(aPoint geodist.Point, bPoint geodist.Point) bool { + a := aPoint.GeomPoint + b := bPoint.GeomPoint dist := coordNorm(coordSub(a, b)) if dist > u.currentValue { @@ -522,9 +509,9 @@ func (c *geomDistanceCalculator) NewEdgeCrosser( ) geodist.EdgeCrosser { return &geomGeodistEdgeCrosser{ strategy: &lineintersector.NonRobustLineIntersector{}, - edgeV0: edge.V0.(*geomGeodistPoint).Coord, - edgeV1: edge.V1.(*geomGeodistPoint).Coord, - nextEdgeV0: startPoint.(*geomGeodistPoint).Coord, + edgeV0: edge.V0.GeomPoint, + edgeV1: edge.V1.GeomPoint, + nextEdgeV0: startPoint.GeomPoint, } } @@ -567,11 +554,11 @@ func (c *geomDistanceCalculator) PointInLinearRing( // See also: https://en.wikipedia.org/wiki/Winding_number // See also: https://en.wikipedia.org/wiki/Nonzero-rule windingNumber := 0 - p := point.(*geomGeodistPoint).Coord + p := point.GeomPoint for edgeIdx := 0; edgeIdx < polygon.NumEdges(); edgeIdx++ { e := polygon.Edge(edgeIdx) - eV0 := e.V0.(*geomGeodistPoint).Coord - eV1 := e.V1.(*geomGeodistPoint).Coord + eV0 := e.V0.GeomPoint + eV1 := e.V1.GeomPoint // Same vertex; none of these checks will pass. if coordEqual(eV0, eV1) { continue @@ -610,15 +597,11 @@ func (c *geomDistanceCalculator) PointInLinearRing( // ClosestPointToEdge implements geodist.DistanceCalculator. func (c *geomDistanceCalculator) ClosestPointToEdge( - edge geodist.Edge, pointInterface geodist.Point, + e geodist.Edge, p geodist.Point, ) (geodist.Point, bool) { - eV0 := edge.V0.(*geomGeodistPoint).Coord - eV1 := edge.V1.(*geomGeodistPoint).Coord - p := pointInterface.(*geomGeodistPoint).Coord - // Edge is a single point. Closest point must be any edge vertex. - if coordEqual(eV0, eV1) { - return edge.V0, coordEqual(eV0, p) + if coordEqual(e.V0.GeomPoint, e.V1.GeomPoint) { + return e.V0, coordEqual(e.V0.GeomPoint, p.GeomPoint) } // From http://www.faqs.org/faqs/graphics/algorithms-faq/, section 1.02 @@ -639,19 +622,19 @@ func (c *geomDistanceCalculator) ClosestPointToEdge( // r<0 P is on the backward extension of AB // r>1 P is on the forward extension of AB // 0 1 { - return pointInterface, false + return p, false } - return &geomGeodistPoint{Coord: coordAdd(eV0, coordMul(ab, r))}, true + return geodist.Point{GeomPoint: coordAdd(e.V0.GeomPoint, coordMul(ab, r))}, true } diff --git a/pkg/sql/opt/exec/execbuilder/testdata/sql_fn b/pkg/sql/opt/exec/execbuilder/testdata/sql_fn index 4fed1985d067..a303661672aa 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/sql_fn +++ b/pkg/sql/opt/exec/execbuilder/testdata/sql_fn @@ -241,3 +241,10 @@ CREATE TABLE my_spatial_table ( CONSTRAINT "primary" PRIMARY KEY (k ASC), FAMILY "primary" (k, geom1, geom2, geom3, geom4, geom5, geom6, geom8, geom9, geom10) ) + +# Regression test for #50296. Using AddGeometryColumn with NULL arguments must +# not panic. +query T +SELECT addgeometrycolumn('a', 'b', 3, NULL, 2); +---- +NULL diff --git a/pkg/sql/opt/optbuilder/scope.go b/pkg/sql/opt/optbuilder/scope.go index 09dd5d180880..226a54c4ab2e 100644 --- a/pkg/sql/opt/optbuilder/scope.go +++ b/pkg/sql/opt/optbuilder/scope.go @@ -1255,6 +1255,9 @@ func (s *scope) replaceSQLFn(f *tree.FuncExpr, def *tree.FunctionDefinition) tre if err != nil { panic(err) } + if typedFunc == tree.DNull { + return tree.DNull + } f = typedFunc.(*tree.FuncExpr) args := make(memo.ScalarListExpr, len(f.Exprs)) diff --git a/pkg/util/mon/bytes_usage.go b/pkg/util/mon/bytes_usage.go index 45a41708b66b..906c5b446c37 100644 --- a/pkg/util/mon/bytes_usage.go +++ b/pkg/util/mon/bytes_usage.go @@ -571,8 +571,10 @@ func (b *BoundAccount) Grow(ctx context.Context, x int64) error { // Shrink releases part of the cumulated allocations by the specified size. func (b *BoundAccount) Shrink(ctx context.Context, delta int64) { if b.used < delta { - panic(fmt.Sprintf("%s: no bytes in account to release, current %d, free %d", - b.mon.name, b.used, delta)) + log.ReportOrPanic(ctx, &b.mon.settings.SV, + "%s: no bytes in account to release, current %d, free %d", + b.mon.name, b.used, delta) + delta = b.used } b.used -= delta b.reserved += delta