Skip to content
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

Jump to bottom

geo/geomfn: implement ST_MakePolygon({geometry,_geometry}) #50979

Merged
merged 1 commit into from
Jul 7, 2020
Merged

geo/geomfn: implement ST_MakePolygon({geometry,_geometry}) #50979

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
4 changes: 4 additions & 0 deletions docs/generated/sql/functions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,10 @@ given Geometry.</p>
</span></td></tr>
<tr><td><a name="st_makepoint"></a><code>st_makepoint(x: <a href="float.html">float</a>, y: <a href="float.html">float</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a new Point with the given X and Y coordinates.</p>
</span></td></tr>
<tr><td><a name="st_makepolygon"></a><code>st_makepolygon(geometry: geometry) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a new Polygon with the given outer LineString.</p>
</span></td></tr>
<tr><td><a name="st_makepolygon"></a><code>st_makepolygon(outer: geometry, interior: anyelement[]) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns a new Polygon with the given outer LineString and interior (hole) LineString(s).</p>
</span></td></tr>
<tr><td><a name="st_maxdistance"></a><code>st_maxdistance(geometry_a: geometry, geometry_b: geometry) &rarr; <a href="float.html">float</a></code></td><td><span class="funcdesc"><p>Returns the maximum distance across every pair of points comprising the given geometries. Note if the geometries are the same, it will return the maximum distance between the geometry’s vertexes.</p>
</span></td></tr>
<tr><td><a name="st_mlinefromtext"></a><code>st_mlinefromtext(str: <a href="string.html">string</a>, srid: <a href="int.html">int</a>) &rarr; geometry</code></td><td><span class="funcdesc"><p>Returns the Geometry from a WKT or EWKT representation with an SRID. If the shape underneath is not MultiLineString, NULL is returned. If the SRID is present in both the EWKT and the argument, the argument value is used.</p>
Expand Down
72 changes: 72 additions & 0 deletions pkg/geo/geomfn/polygon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package geomfn

import (
"github.com/cockroachdb/cockroach/pkg/geo"
"github.com/cockroachdb/errors"
"github.com/twpayne/go-geom"
)

// MakePolygon creates a Polygon geometry from linestring and optional inner linestrings.
// Returns errors if geometries are not linestrings.
func MakePolygon(outer *geo.Geometry, interior ...*geo.Geometry) (*geo.Geometry, error) {
layout := geom.XY
outerGeomT, err := outer.AsGeomT()
if err != nil {
return nil, err
}
outerRing, ok := outerGeomT.(*geom.LineString)
if !ok {
return nil, errors.Newf("argument must be LINESTRING geometries")
}
if outerRing.NumCoords() < 4 {
return nil, errors.Newf("shell must have at least 4 points")
}
if !isClosed(layout, outerRing) {
return nil, errors.Newf("shell must be closed")
}
srid := outerRing.SRID()
coords := make([][]geom.Coord, len(interior)+1)
coords[0] = outerRing.Coords()
for i, g := range interior {
interiorRingGeomT, err := g.AsGeomT()
if err != nil {
return nil, err
}
interiorRing, ok := interiorRingGeomT.(*geom.LineString)
if !ok {
return nil, errors.Newf("argument must be LINESTRING geometries")
}
if interiorRing.SRID() != srid {
return nil, errors.Newf("mixed SRIDs are not allowed")
}
if interiorRing.NumCoords() < 4 {
return nil, errors.Newf("holes must have at least 4 points")
}
if !isClosed(layout, interiorRing) {
return nil, errors.Newf("holes must be closed")
}
coords[i+1] = interiorRing.Coords()
}
Copy link
Contributor

@otan otan Jul 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more minor detail: can you also add a test to make sure linestrings are closed with at least four points, e.g.

otan=# select st_makepolygon('LINESTRING(0 0, 1 0)');
ERROR:  lwpoly_from_lwlines: shell must have at least 4 points
otan=# select st_makepolygon('LINESTRING(0 0, 1 0, 1 1, 0.1 0.1)');
ERROR:  lwpoly_from_lwlines: shell must be closed

(same for interior loops)

Copy link
Contributor Author

@hiepd hiepd Jul 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check for number of points and closure. Thanks! Not sure if this is only Postgis behavior or geom should implement this check as well. At the moment, geom doesn't check for this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but we'll make do with this for now :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #51074.


polygon, err := geom.NewPolygon(layout).SetSRID(srid).SetCoords(coords)
if err != nil {
return nil, err
}
return geo.NewGeometryFromGeomT(polygon)
}

// isClosed checks if a LineString is closed to make a valid Polygon.
// Returns whether the last coordinate is the same as the first.
func isClosed(layout geom.Layout, g *geom.LineString) bool {
return g.Coord(0).Equal(layout, g.Coord(g.NumCoords()-1))
}
260 changes: 260 additions & 0 deletions pkg/geo/geomfn/polygon_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,260 @@
// Copyright 2020 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package geomfn

import (
"testing"

"github.com/cockroachdb/cockroach/pkg/geo"
"github.com/cockroachdb/cockroach/pkg/geo/geopb"
"github.com/cockroachdb/errors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMakePolygon(t *testing.T) {
testCases := []struct {
name string
outer string
outerSRID geopb.SRID
interior []string
interiorSRIDs []geopb.SRID
expected string
expectedSRID geopb.SRID
err error
}{
{
"Single input variant - 2D",
"LINESTRING(75 29,77 29,77 29, 75 29)",
geopb.DefaultGeometrySRID,
[]string{},
[]geopb.SRID{},
"POLYGON((75 29,77 29,77 29,75 29))",
geopb.DefaultGeometrySRID,
nil,
},
{
"Single input variant - 2D with SRID",
"LINESTRING(75 29,77 29,77 29, 75 29)",
geopb.SRID(4326),
[]string{},
[]geopb.SRID{},
"POLYGON((75 29,77 29,77 29,75 29))",
geopb.SRID(4326),
nil,
},
{
"Single input variant - 2D square",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.DefaultGeometrySRID,
[]string{},
[]geopb.SRID{},
"POLYGON((40 80, 80 80, 80 40, 40 40, 40 80))",
geopb.DefaultGeometrySRID,
nil,
},
{
"With inner holes variant - 2D single interior ring",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.DefaultGeometrySRID,
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
},
[]geopb.SRID{geopb.DefaultGeometrySRID},
"POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70))",
geopb.DefaultGeometrySRID,
nil,
},
{
"With inner holes variant - 2D single interior ring with SRIDs",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
},
[]geopb.SRID{geopb.SRID(4326)},
"POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70))",
geopb.SRID(4326),
nil,
},
{
"With inner holes variant - 2D two interior rings",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.DefaultGeometrySRID,
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
"LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)",
},
[]geopb.SRID{geopb.DefaultGeometrySRID, geopb.DefaultGeometrySRID},
"POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70),(60 60,75 60,75 45,60 45,60 60))",
geopb.DefaultGeometrySRID,
nil,
},
{
"With inner holes variant - 2D two interior rings with SRID",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
"LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)",
},
[]geopb.SRID{geopb.SRID(4326), geopb.SRID(4326)},
"POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70),(60 60,75 60,75 45,60 45,60 60))",
geopb.SRID(4326),
nil,
},
{
"ERROR: Invalid argument - POINT",
"POINT(3 2)",
geopb.DefaultGeometrySRID,
[]string{},
[]geopb.SRID{},
"",
geopb.DefaultGeometrySRID,
errors.Newf("argument must be LINESTRING geometries"),
},
{
"ERROR: Invalid argument - POINT rings",
"LINESTRING(75 29,77 29,77 29, 75 29)",
geopb.DefaultGeometrySRID,
[]string{"POINT(3 2)"},
[]geopb.SRID{0},
"",
geopb.DefaultGeometrySRID,
errors.Newf("argument must be LINESTRING geometries"),
},
{
"ERROR: Unmatched SRIDs - Single interior ring",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
},
[]geopb.SRID{geopb.SRID(26918)},
"",
geopb.DefaultGeometrySRID,
errors.Newf("mixed SRIDs are not allowed"),
},
{
"ERROR: Unmatched SRIDs - Default SRID on interior ring",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
},
[]geopb.SRID{geopb.DefaultGeometrySRID},
"",
geopb.DefaultGeometrySRID,
errors.Newf("mixed SRIDs are not allowed"),
},
{
"ERROR: Unmatched SRIDs - Two interior rings",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)",
"LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)",
},
[]geopb.SRID{geopb.SRID(4326), geopb.SRID(26918)},
"",
geopb.DefaultGeometrySRID,
errors.Newf("mixed SRIDs are not allowed"),
},
{
"ERROR: Unclosed shell",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 70)",
geopb.DefaultGeographySRID,
[]string{},
[]geopb.SRID{},
"",
geopb.DefaultGeometrySRID,
errors.Newf("shell must be closed"),
},
{
"ERROR: Unclosed interior ring",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 70 50, 50 50, 50 60)",
},
[]geopb.SRID{geopb.SRID(4326), geopb.SRID(26918)},
"",
geopb.DefaultGeometrySRID,
errors.Newf("holes must be closed"),
},
{
"ERROR: Shell has 3 points",
"LINESTRING(40 80, 80 80, 40 80)",
geopb.DefaultGeographySRID,
[]string{},
[]geopb.SRID{},
"",
geopb.DefaultGeometrySRID,
errors.Newf("shell must have at least 4 points"),
},
{
"ERROR: Shell has 2 points",
"LINESTRING(40 80, 40 80)",
geopb.DefaultGeographySRID,
[]string{},
[]geopb.SRID{},
"",
geopb.DefaultGeometrySRID,
errors.Newf("shell must have at least 4 points"),
},
{
"ERROR: Interior ring has 3 points",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 70 70, 50 70)",
},
[]geopb.SRID{geopb.SRID(4326), geopb.SRID(26918)},
"",
geopb.DefaultGeometrySRID,
errors.Newf("holes must have at least 4 points"),
},
{
"ERROR: Interior ring has 2 points",
"LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)",
geopb.SRID(4326),
[]string{
"LINESTRING(50 70, 50 70)",
},
[]geopb.SRID{geopb.SRID(4326), geopb.SRID(26918)},
"",
geopb.DefaultGeometrySRID,
errors.Newf("holes must have at least 4 points"),
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
outer, err := geo.MustParseGeometry(tc.outer).CloneWithSRID(tc.outerSRID)
require.NoError(t, err)
interior := make([]*geo.Geometry, 0, len(tc.interior))
for i, g := range tc.interior {
interiorRing, err := geo.MustParseGeometry(g).CloneWithSRID(tc.interiorSRIDs[i])
require.NoError(t, err)
interior = append(interior, interiorRing)
}
polygon, err := MakePolygon(outer, interior...)
if tc.err != nil {
require.Errorf(t, err, tc.err.Error())
} else {
require.NoError(t, err)
expected, err := geo.MustParseGeometry(tc.expected).CloneWithSRID(tc.expectedSRID)
require.NoError(t, err)
assert.Equal(t, expected, polygon)
}
})
}
}
Loading