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

hiepd · 2020-07-04T08:53:06Z

Implement ST_MakePolygon

Release note (sql change): Implement ST_MakePolygon functionality for geometry operator

	Postgres	CockroachDB
SELECT ST_MakePolygon('MULTIPOINT(0 0, 1 1)');	ERROR: Shell is not a line	ERROR: st_makepolygon(): argument must be LINESTRING geometries
SELECT ST_MakePolygon('abc');	HINT: "ab" <-- parse error at position 2 within geometry	ERROR: unknown signature: st_makepolygon(string)
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY[ ST_GeomFromText('MULTIPOINT(50 70, 70 70, 70 50, 50 50, 50 70)') ]);	ERROR: Hole 0 is not a line	ERROR: st_makepolygon(): argument must be LINESTRING geometries
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY['abc']);	ERROR: parse error - invalid geometry HINT: "ab" <-- parse error at position 2 within geometry	ERROR: st_makepolygon(): argument must be LINESTRING geometries
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)', 4326), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)') ]);	ERROR: lwpoly_from_lwlines: mixed SRIDs in input lines	ERROR: st_makepolygon(): mixed SRIDs are not allowed
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)', 4326), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)', 3857) ]);	ERROR: lwpoly_from_lwlines: mixed SRIDs in input lines	ERROR: st_makepolygon(): mixed SRIDs are not allowed
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)', 4326), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)', 4326), ST_GeomFromText('LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)', 3857) ]);	ERROR: lwpoly_from_lwlines: mixed SRIDs in input lines	ERROR: st_makepolygon(): mixed SRIDs are not allowed
SELECT ST_MakePolygon(ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 70)'));	ERROR: lwpoly_from_lwlines: shell must be closed	ERROR: st_makepolygon(): shell must be closed
SELECT ST_MakePolygon(ST_GeomFromText('LINESTRING(40 80, 80 80, 40 80)'));	ERROR: lwpoly_from_lwlines: shell must have at least 4 points	ERROR: st_makepolygon(): shell must have at least 4 points
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 60)'), ST_GeomFromText('LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)') ]);	ERROR: lwpoly_from_lwlines: holes must be closed	ERROR: st_makepolygon(): holes must be closed
SELECT ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 50 70)') ]);	ERROR: lwpoly_from_lwlines: holes must have at least 4 points	ERROR: st_makepolygon(): holes must have at least 4 points
SELECT ST_AsEWKT(ST_MakePolygon( ST_GeomFromText('LINESTRING(75 29,77 29,77 29, 75 29)')));	POLYGON((75 29,77 29,77 29,75 29))	POLYGON ((75 29, 77 29, 77 29, 75 29))
SELECT ST_AsEWKT(ST_MakePolygon( ST_GeomFromText('LINESTRING(75 29,77 29,77 29, 75 29)', 4326) ));	SRID=4326;POLYGON((75 29,77 29,77 29,75 29))	SRID=4326;POLYGON ((75 29, 77 29, 77 29, 75 29))
SELECT ST_AsEWKT(ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)') ]));	POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70))	POLYGON ((40 80, 80 80, 80 40, 40 40, 40 80), (50 70, 70 70, 70 50, 50 50, 50 70))
SELECT ST_AsEWKT(ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)'), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)'), ST_GeomFromText('LINESTRING(60 60, 75 60, 75 45, 60 45, 60 60)') ]));	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))	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))
SELECT ST_AsEWKT(ST_MakePolygon( ST_GeomFromText('LINESTRING(40 80, 80 80, 80 40, 40 40, 40 80)', 4326), ARRAY[ ST_GeomFromText('LINESTRING(50 70, 70 70, 70 50, 50 50, 50 70)', 4326) ]));	SRID=4326;POLYGON((40 80,80 80,80 40,40 40,40 80),(50 70,70 70,70 50,50 50,50 70))	SRID=4326;POLYGON ((40 80, 80 80, 80 40, 40 40, 40 80), (50 70, 70 70, 70 50, 50 50, 50 70))

NOTES

Since Geometry from geo package does not support 3D, the following are ignored for now:

create 3D polygon from 3D linestrings or linestrings with measures

cockroach-teamcity · 2020-07-04T08:53:09Z

All committers have signed the CLA.

blathers-crl · 2020-07-04T08:53:11Z

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

I have added a few people who may be able to assist in reviewing:

@otan (author of geo/geomfn: implement ST_MakePolygon({geometry,_geometry}) #49305)

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

cockroach-teamcity · 2020-07-04T08:53:16Z

This change is

blathers-crl · 2020-07-04T09:07:59Z

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

pkg/sql/sem/builtins/geo_builtins.go

otan

Thanks for submitting this! Just a couple of small nits.

pkg/geo/geomfn/polygon.go

blathers-crl · 2020-07-07T07:08:07Z

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

blathers-crl · 2020-07-07T07:26:39Z

Thank you for updating your pull request.

Before a member of our team reviews your PR, I have some potential action items for you:

We notice you have more than one commit in your PR. We try break logical changes into separate commits, but commits such as "fix typo" or "address review commits" should be squashed into one commit and pushed with --force
When CI has completed, please ensure no errors have appeared.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

otan

Please also refactor your commit message to:

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

Closes #49305

Release note (sql change): Implement ST_MakePolygon functionality
for Geometry types.

Note title the important thing that the title is the first line of the commit message.

Thanks!

pkg/geo/geomfn/polygon.go

otan · 2020-07-07T14:36:01Z

pkg/geo/geomfn/polygon.go

+			return nil, errors.Newf("mixed SRIDs are not allowed")
+		}
+		coords[i+1] = interiorRing.Coords()
+	}


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)

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.

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

Filed #51074.

Closes cockroachdb#49305 Release note (sql change): Implement ST_MakePolygon functionality for Geometry types.

otan

Thanks!

otan · 2020-07-07T17:22:09Z

bors r+

craig · 2020-07-07T18:13:55Z

Build succeeded

GitHub CI (Cockroach)

Amruta-Ranade · 2020-12-07T23:10:05Z

@hiepd Thank you for contributing to CockroachDB this year. As a token of our appreciation, we would like to send you a gift. Please DM me on our community Slack @amruta so I can send you a link. (If you don’t want a gift, we also have a charitable contribution choice.) All orders need to be in by December 13, so please contact me as soon as possible :)

blathers-crl bot added O-community Originated from the community X-blathers-triaged blathers was able to find an owner labels Jul 4, 2020

blathers-crl bot requested a review from otan July 4, 2020 08:53

hiepd force-pushed the geo/49305_st_makepolygon branch from 0e4f278 to 8987ff0 Compare July 4, 2020 09:07

hiepd force-pushed the geo/49305_st_makepolygon branch from 8987ff0 to 749162f Compare July 4, 2020 09:13

otan reviewed Jul 6, 2020

View reviewed changes

pkg/sql/sem/builtins/geo_builtins.go Outdated Show resolved Hide resolved

pkg/sql/sem/builtins/geo_builtins.go Outdated Show resolved Hide resolved

otan reviewed Jul 6, 2020

View reviewed changes

pkg/geo/geomfn/polygon.go Outdated Show resolved Hide resolved

pkg/geo/geomfn/polygon.go Outdated Show resolved Hide resolved

otan reviewed Jul 6, 2020

View reviewed changes

pkg/geo/geomfn/polygon.go Outdated Show resolved Hide resolved

blathers-crl bot requested a review from otan July 7, 2020 07:08

hiepd force-pushed the geo/49305_st_makepolygon branch from cdeda5b to f0d5b80 Compare July 7, 2020 07:43

otan reviewed Jul 7, 2020

View reviewed changes

pkg/geo/geomfn/polygon.go Outdated Show resolved Hide resolved

otan reviewed Jul 7, 2020

View reviewed changes

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

f9a494f

Closes cockroachdb#49305 Release note (sql change): Implement ST_MakePolygon functionality for Geometry types.

hiepd force-pushed the geo/49305_st_makepolygon branch from f0d5b80 to f9a494f Compare July 7, 2020 16:54

blathers-crl bot requested a review from otan July 7, 2020 16:54

otan approved these changes Jul 7, 2020

View reviewed changes

otan mentioned this pull request Jul 7, 2020

geo: perform basic validation for Geospatial objects in spatialObjectFromGeomT #51074

Closed

craig bot merged commit 256678f into cockroachdb:master Jul 7, 2020

lnhsingh mentioned this pull request Aug 26, 2020

geo/geomfn: implement ST_MakePolygon({geometry,_geometry}) cockroachdb/docs#8167

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hiepd commented Jul 4, 2020 •

edited

Loading

cockroach-teamcity commented Jul 4, 2020 •

edited

Loading

blathers-crl bot commented Jul 4, 2020

cockroach-teamcity commented Jul 4, 2020

blathers-crl bot commented Jul 4, 2020

otan left a comment

blathers-crl bot commented Jul 7, 2020

blathers-crl bot commented Jul 7, 2020

otan left a comment

otan Jul 7, 2020 •

edited

Loading

hiepd Jul 7, 2020 •

edited

Loading

otan Jul 7, 2020

otan Jul 7, 2020

otan left a comment

otan commented Jul 7, 2020

craig bot commented Jul 7, 2020

Amruta-Ranade commented Dec 7, 2020

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

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

Conversation

hiepd commented Jul 4, 2020 • edited Loading

NOTES

cockroach-teamcity commented Jul 4, 2020 • edited Loading

blathers-crl bot commented Jul 4, 2020

cockroach-teamcity commented Jul 4, 2020

blathers-crl bot commented Jul 4, 2020

otan left a comment

Choose a reason for hiding this comment

blathers-crl bot commented Jul 7, 2020

blathers-crl bot commented Jul 7, 2020

otan left a comment

Choose a reason for hiding this comment

otan Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

hiepd Jul 7, 2020 • edited Loading

Choose a reason for hiding this comment

otan Jul 7, 2020

Choose a reason for hiding this comment

otan Jul 7, 2020

Choose a reason for hiding this comment

otan left a comment

Choose a reason for hiding this comment

otan commented Jul 7, 2020

craig bot commented Jul 7, 2020

Build succeeded

Amruta-Ranade commented Dec 7, 2020

hiepd commented Jul 4, 2020 •

edited

Loading

cockroach-teamcity commented Jul 4, 2020 •

edited

Loading

otan Jul 7, 2020 •

edited

Loading

hiepd Jul 7, 2020 •

edited

Loading