You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sedona.apache.org by GitBox <gi...@apache.org> on 2022/04/23 00:48:15 UTC
[GitHub] [incubator-sedona] Pooja444 opened a new pull request, #621: [Sedona-113] Add ST_PointN to Apache Sedona
Pooja444 opened a new pull request, #621:
URL: https://github.com/apache/incubator-sedona/pull/621
## Did you read the Contributor Guide?
- Yes, I have read [Contributor Rules](https://sedona.apache.org/community/rule/) and [Contributor Development Guide](https://sedona.apache.org/community/develop/)
## Is this PR related to a JIRA ticket?
- Yes, the URL of the assoicated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-113. The PR name follows the format `[SEDONA-XXX] my subject`.
## What changes were proposed in this PR?
Added ST_PointN function to Flink and SQL
## How was this patch tested?
Added unit tests in Java, Scala and Python
## Did this PR include necessary documentation updates?
- Yes, I have updated the documentation update.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [SEDONA-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856858980
##########
flink/src/test/java/org/apache/sedona/flink/FunctionTest.java:
##########
@@ -82,6 +87,27 @@ public void testReverse() {
}
@Test
+ public void testPointN_positiveN() {
+ int n = 1;
+ Table polygonTable = createPolygonTable(1);
+ Table linestringTable = polygonTable.select(call(Functions.ST_ExteriorRing.class.getSimpleName(), $(polygonColNames[0])));
+ Table pointTable = linestringTable.select(call(Functions.ST_PointN.class.getSimpleName(), $("_c0"), n));
+ Point point = (Point) first(pointTable).getField(0);
+ assert point != null;
+ Assert.assertEquals("POINT (-0.5 -0.5)", point.toString());
+ }
+
+ @Test
+ public void testPointN_negativeN() {
+ int n = -3;
+ Table polygonTable = createPolygonTable(1);
+ Table linestringTable = polygonTable.select(call(Functions.ST_ExteriorRing.class.getSimpleName(), $(polygonColNames[0])));
+ Table pointTable = linestringTable.select(call(Functions.ST_PointN.class.getSimpleName(), $("_c0"), n));
+ Point point = (Point) first(pointTable).getField(0);
+ assert point != null;
+ Assert.assertEquals("POINT (0.5 0.5)", point.toString());
+ }
+
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #621: [SEDONA-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r857045727
##########
sql/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala:
##########
@@ -1620,6 +1620,47 @@ case class ST_Reverse(inputExpressions: Seq[Expression])
}
/**
+ * Returns the nth point in the geometry, provided it is a linestring
+ *
+ * @param inputExpressions sequence of 2 input arguments, a geometry and a value 'n'
+ */
+case class ST_PointN(inputExpressions: Seq[Expression])
+ extends Expression with CodegenFallback {
+ inputExpressions.validateLength(2)
+
+ override def nullable: Boolean = true
+
+ lazy val GeometryFactory = new GeometryFactory()
+ lazy val emptyGeometry: GeometryCollection = GeometryFactory.createGeometryCollection(null)
Review Comment:
This emptyGeometry is NOT consistent with PostGIS requirement. ST_PointN should return NULL if no input linestring. You return an empty GeometryCollection but not NULL. https://postgis.net/docs/ST_PointN.html
Please fix this and add corresponding test cases for non-line string input.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856843811
##########
core/src/main/java/org/apache/sedona/core/utils/GeomUtils.java:
##########
@@ -81,6 +84,38 @@ public static Geometry getInteriorPoint(Geometry geometry) {
return geometry.getInteriorPoint();
}
+ /**
+ * Return the nth point from the given geometry (which could be a linestring or a circular linestring)
+ * If the value of n is negative, return a point backwards
+ * E.g. if n = 1, return 1st point, if n = -1, return last point
+ *
+ * @param lineString from which the nth point is to be returned
+ * @param n is the position of the point in the geometry
+ * @return a point
+ */
+ public static Geometry getNthPoint(LineString lineString, int n) {
+ if (lineString == null || n == 0) {
+ return null;
+ }
+
+ int p = lineString.getNumPoints();
+ if (Math.abs(n) > p) {
+ return null;
+ }
+
+ Coordinate[] nthCoordinate = new Coordinate[1];
+ if (n > 0) {
+ nthCoordinate[0] = lineString.getCoordinates()[n - 1];
+ } else {
+ nthCoordinate[0] = lineString.getCoordinates()[p + n];
+ }
+ return new Point(new CoordinateArraySequence(nthCoordinate), lineString.getFactory());
+ }
+
+ public static Geometry getExteriorRing(Geometry geometry) {
+ return geometry.getFactory().createLinearRing(geometry.getCoordinates());
Review Comment:
Sure, I updated this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [SEDONA-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r857042504
##########
docs/api/flink/Function.md:
##########
@@ -152,6 +152,55 @@ Input: `POLYGON ((-0.5 -0.5, -0.5 0.5, 0.5 0.5, 0.5 -0.5, -0.5 -0.5))`
Output: `POLYGON ((-0.5 -0.5, 0.5 -0.5, 0.5 0.5, -0.5 0.5, -0.5 -0.5))`
+## ST_PointN
+
+Introduction: Return the Nth point in a single linestring or circular linestring in the geometry. Negative values are counted backwards from the end of the LineString, so that -1 is the last point. Returns NULL if there is no linestring in the geometry.
+
+Format: `ST_PointN(A:geometry, B:integer)`
+
+Since: `v1.2.1`
+
+Examples:
+
+```SQL
+SELECT ST_PointN(df.geometry, 2)
+FROM df
+```
+
+Input: `LINESTRING(0 0, 1 2, 2 4, 3 6), 2`
+
+Output: `POINT (1 2)`
+
+Input: `LINESTRING(0 0, 1 2, 2 4, 3 6), -2`
+
+Output: `POINT (2 4)`
+
+Input: `CIRCULARSTRING(1 1, 1 2, 2 4, 3 6, 1 2, 1 1), -1`
+
+Output: `POINT (1 1)`
+
+
+## ST_ExteriorRing
+
+Introduction: Returns a LINESTRING representing the exterior ring (shell) of a POLYGON. Returns NULL if the geometry is not a polygon.
+
+Format: `ST_ExteriorRing(A:geometry)`
+
+Since: `v1.2.1`
+
+Examples:
+
+```SQL
+SELECT ST_ExteriorRing(df.geometry)
+FROM df
+```
+
+Input: `POLYGON((0 0, 1 1, 2 1, 0 1, 1 -1))`
Review Comment:
Updated it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856845472
##########
flink/src/test/java/org/apache/sedona/flink/FunctionTest.java:
##########
@@ -82,6 +87,27 @@ public void testReverse() {
}
@Test
+ public void testPointN_positiveN() {
+ int n = 1;
+ Table polygonTable = createPolygonTable(1);
+ Table linestringTable = polygonTable.select(call(Functions.ST_ExteriorRing.class.getSimpleName(), $(polygonColNames[0])));
+ Table pointTable = linestringTable.select(call(Functions.ST_PointN.class.getSimpleName(), $("_c0"), n));
+ Point point = (Point) first(pointTable).getField(0);
+ assert point != null;
+ Assert.assertEquals("POINT (-0.5 -0.5)", point.toString());
+ }
+
+ @Test
+ public void testPointN_negativeN() {
+ int n = -3;
+ Table polygonTable = createPolygonTable(1);
+ Table linestringTable = polygonTable.select(call(Functions.ST_ExteriorRing.class.getSimpleName(), $(polygonColNames[0])));
+ Table pointTable = linestringTable.select(call(Functions.ST_PointN.class.getSimpleName(), $("_c0"), n));
+ Point point = (Point) first(pointTable).getField(0);
+ assert point != null;
+ Assert.assertEquals("POINT (0.5 0.5)", point.toString());
+ }
+
Review Comment:
Please add a test for exterior ring.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856841378
##########
docs/api/flink/Function.md:
##########
@@ -152,6 +152,34 @@ Input: `POLYGON ((-0.5 -0.5, -0.5 0.5, 0.5 0.5, 0.5 -0.5, -0.5 -0.5))`
Output: `POLYGON ((-0.5 -0.5, 0.5 -0.5, 0.5 0.5, -0.5 0.5, -0.5 -0.5))`
+## ST_PointN
Review Comment:
Please add docs for ST_ExteriorRing too.
##########
core/src/main/java/org/apache/sedona/core/utils/GeomUtils.java:
##########
@@ -81,6 +84,38 @@ public static Geometry getInteriorPoint(Geometry geometry) {
return geometry.getInteriorPoint();
}
+ /**
+ * Return the nth point from the given geometry (which could be a linestring or a circular linestring)
+ * If the value of n is negative, return a point backwards
+ * E.g. if n = 1, return 1st point, if n = -1, return last point
+ *
+ * @param lineString from which the nth point is to be returned
+ * @param n is the position of the point in the geometry
+ * @return a point
+ */
+ public static Geometry getNthPoint(LineString lineString, int n) {
+ if (lineString == null || n == 0) {
+ return null;
+ }
+
+ int p = lineString.getNumPoints();
+ if (Math.abs(n) > p) {
+ return null;
+ }
+
+ Coordinate[] nthCoordinate = new Coordinate[1];
+ if (n > 0) {
+ nthCoordinate[0] = lineString.getCoordinates()[n - 1];
+ } else {
+ nthCoordinate[0] = lineString.getCoordinates()[p + n];
+ }
+ return new Point(new CoordinateArraySequence(nthCoordinate), lineString.getFactory());
+ }
+
+ public static Geometry getExteriorRing(Geometry geometry) {
+ return geometry.getFactory().createLinearRing(geometry.getCoordinates());
Review Comment:
Use this function in JTS Polygon. Dont create LinearRing by coordinates as they might NOT be the rings.https://locationtech.github.io/jts/javadoc/org/locationtech/jts/geom/Polygon.html#getExteriorRing--
##########
sql/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala:
##########
@@ -105,6 +105,7 @@ object Catalog {
ST_Multi,
ST_PointOnSurface,
ST_Reverse,
+ ST_PointN,
Review Comment:
Register ST_ExteriorRing as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
jiayuasu commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856845091
##########
docs/api/flink/Function.md:
##########
@@ -152,6 +152,55 @@ Input: `POLYGON ((-0.5 -0.5, -0.5 0.5, 0.5 0.5, 0.5 -0.5, -0.5 -0.5))`
Output: `POLYGON ((-0.5 -0.5, 0.5 -0.5, 0.5 0.5, -0.5 0.5, -0.5 -0.5))`
+## ST_PointN
+
+Introduction: Return the Nth point in a single linestring or circular linestring in the geometry. Negative values are counted backwards from the end of the LineString, so that -1 is the last point. Returns NULL if there is no linestring in the geometry.
+
+Format: `ST_PointN(A:geometry, B:integer)`
+
+Since: `v1.2.1`
+
+Examples:
+
+```SQL
+SELECT ST_PointN(df.geometry, 2)
+FROM df
+```
+
+Input: `LINESTRING(0 0, 1 2, 2 4, 3 6), 2`
+
+Output: `POINT (1 2)`
+
+Input: `LINESTRING(0 0, 1 2, 2 4, 3 6), -2`
+
+Output: `POINT (2 4)`
+
+Input: `CIRCULARSTRING(1 1, 1 2, 2 4, 3 6, 1 2, 1 1), -1`
+
+Output: `POINT (1 1)`
+
+
+## ST_ExteriorRing
+
+Introduction: Returns a LINESTRING representing the exterior ring (shell) of a POLYGON. Returns NULL if the geometry is not a polygon.
+
+Format: `ST_ExteriorRing(A:geometry)`
+
+Since: `v1.2.1`
+
+Examples:
+
+```SQL
+SELECT ST_ExteriorRing(df.geometry)
+FROM df
+```
+
+Input: `POLYGON((0 0, 1 1, 2 1, 0 1, 1 -1))`
Review Comment:
The example here is NOT a valid WKT format
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856841863
##########
sql/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala:
##########
@@ -105,6 +105,7 @@ object Catalog {
ST_Multi,
ST_PointOnSurface,
ST_Reverse,
+ ST_PointN,
Review Comment:
@jiayuasu It was already implemented in Scala, so it's already present in the file (line number 86)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856843802
##########
docs/api/flink/Function.md:
##########
@@ -152,6 +152,34 @@ Input: `POLYGON ((-0.5 -0.5, -0.5 0.5, 0.5 0.5, 0.5 -0.5, -0.5 -0.5))`
Output: `POLYGON ((-0.5 -0.5, 0.5 -0.5, 0.5 0.5, -0.5 0.5, -0.5 -0.5))`
+## ST_PointN
Review Comment:
Added
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] Pooja444 commented on a diff in pull request #621: [Sedona-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
Pooja444 commented on code in PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621#discussion_r856841863
##########
sql/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala:
##########
@@ -105,6 +105,7 @@ object Catalog {
ST_Multi,
ST_PointOnSurface,
ST_Reverse,
+ ST_PointN,
Review Comment:
@jiayuasu It was already implemented in Scala, so it's already present in the file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-sedona] jiayuasu merged pull request #621: [SEDONA-113] Add ST_PointN to Apache Sedona
Posted by GitBox <gi...@apache.org>.
jiayuasu merged PR #621:
URL: https://github.com/apache/incubator-sedona/pull/621
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org