Copilot commented on code in PR #2298:
URL: https://github.com/apache/sedona/pull/2298#discussion_r2289612768
##########
spark/common/src/test/scala/org/apache/sedona/sql/geography/ConstructorsTest.scala:
##########
@@ -233,4 +233,14 @@ class ConstructorsTest extends TestBaseScala {
assertEquals(4326, geom.getSRID)
assert(geom.getGeometryType == "LineString")
}
+
+ it("Passed ST_GeomToGeography multilingstring") {
Review Comment:
The test name contains a spelling error: "multilingstring" should be
"multilinestring".
```suggestion
it("Passed ST_GeomToGeography multilinestring") {
```
##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -264,4 +264,241 @@ private static Geometry collectionToGeom(Geography g,
GeometryFactory gf) {
}
return gf.createGeometryCollection(gs);
}
+
+ public static Geography geomToGeography(Geometry geom) throws Exception {
+ if (geom == null) {
+ return null;
+ }
+ Geography geography;
+ if (geom instanceof Point) {
+ geography = pointToGeog((Point) geom);
+ } else if (geom instanceof MultiPoint) {
+ geography = mPointToGeog((MultiPoint) geom);
+ } else if (geom instanceof LineString) {
+ geography = lineToGeog((LineString) geom);
+ } else if (geom instanceof MultiLineString) {
+ geography = mLineToGeog((MultiLineString) geom);
+ } else if (geom instanceof Polygon) {
+ geography = polyToGeog((Polygon) geom);
+ } else if (geom instanceof MultiPolygon) {
+ geography = mPolyToGeog((MultiPolygon) geom);
+ } else if (geom instanceof GeometryCollection) {
+ geography = geomCollToGeog((GeometryCollection) geom);
+ } else {
+ throw new UnsupportedOperationException(
+ "Geometry type is not supported: " +
geom.getClass().getSimpleName());
+ }
+ geography.setSRID(geom.getSRID());
+ return geography;
+ }
+
+ private static Geography pointToGeog(Geometry geom) throws IOException {
+ Coordinate[] pts = geom.getCoordinates();
+ if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+ return new SinglePointGeography();
+ }
+ double lon = pts[0].x;
+ double lat = pts[0].y;
+
+ S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+ S2Builder builder = new S2Builder.Builder().build();
+ S2PointVectorLayer layer = new S2PointVectorLayer();
+ builder.startLayer(layer);
+ builder.addPoint(s2Point);
+
+ S2Error error = new S2Error();
+ if (!builder.build(error)) {
+ throw new IOException("Failed to build S2 point layer: " + error.text());
+ }
+ List<S2Point> points = layer.getPointVector();
+ if (points.isEmpty()) {
+ return new SinglePointGeography();
+ }
+ return new SinglePointGeography(points.get(0));
+ }
+
+ private static Geography mPointToGeog(Geometry geom) throws IOException {
+ Coordinate[] pts = geom.getCoordinates();
+ if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+ return new PointGeography();
+ }
Review Comment:
The method checks if `pts.length == 0` but then accesses `pts[0]` in the
same condition, which will throw an ArrayIndexOutOfBoundsException when the
array is empty.
```suggestion
if (pts.length == 0) {
return new PointGeography();
}
if (Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
return new PointGeography();
}
```
##########
common/src/main/java/org/apache/sedona/common/geography/Constructors.java:
##########
@@ -264,4 +264,241 @@ private static Geometry collectionToGeom(Geography g,
GeometryFactory gf) {
}
return gf.createGeometryCollection(gs);
}
+
+ public static Geography geomToGeography(Geometry geom) throws Exception {
+ if (geom == null) {
+ return null;
+ }
+ Geography geography;
+ if (geom instanceof Point) {
+ geography = pointToGeog((Point) geom);
+ } else if (geom instanceof MultiPoint) {
+ geography = mPointToGeog((MultiPoint) geom);
+ } else if (geom instanceof LineString) {
+ geography = lineToGeog((LineString) geom);
+ } else if (geom instanceof MultiLineString) {
+ geography = mLineToGeog((MultiLineString) geom);
+ } else if (geom instanceof Polygon) {
+ geography = polyToGeog((Polygon) geom);
+ } else if (geom instanceof MultiPolygon) {
+ geography = mPolyToGeog((MultiPolygon) geom);
+ } else if (geom instanceof GeometryCollection) {
+ geography = geomCollToGeog((GeometryCollection) geom);
+ } else {
+ throw new UnsupportedOperationException(
+ "Geometry type is not supported: " +
geom.getClass().getSimpleName());
+ }
+ geography.setSRID(geom.getSRID());
+ return geography;
+ }
+
+ private static Geography pointToGeog(Geometry geom) throws IOException {
+ Coordinate[] pts = geom.getCoordinates();
+ if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+ return new SinglePointGeography();
+ }
+ double lon = pts[0].x;
+ double lat = pts[0].y;
+
+ S2Point s2Point = S2LatLng.fromDegrees(lat, lon).toPoint();
+ S2Builder builder = new S2Builder.Builder().build();
+ S2PointVectorLayer layer = new S2PointVectorLayer();
+ builder.startLayer(layer);
+ builder.addPoint(s2Point);
+
+ S2Error error = new S2Error();
+ if (!builder.build(error)) {
+ throw new IOException("Failed to build S2 point layer: " + error.text());
+ }
+ List<S2Point> points = layer.getPointVector();
+ if (points.isEmpty()) {
+ return new SinglePointGeography();
+ }
+ return new SinglePointGeography(points.get(0));
+ }
+
+ private static Geography mPointToGeog(Geometry geom) throws IOException {
+ Coordinate[] pts = geom.getCoordinates();
+ if (pts.length == 0 || Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
+ return new PointGeography();
+ }
Review Comment:
Array access `pts[0]` will throw ArrayIndexOutOfBoundsException when
`pts.length == 0`. The conditions should be separated or reordered to check
array length first.
```suggestion
if (pts.length == 0) {
return new PointGeography();
}
if (Double.isNaN(pts[0].x) || Double.isNaN(pts[0].y)) {
return new PointGeography();
}
```
##########
spark/common/src/test/scala/org/apache/sedona/sql/geography/ConstructorsDataFrameAPITest.scala:
##########
@@ -127,4 +127,14 @@ class ConstructorsDataFrameAPITest extends TestBaseScala {
assert(geom.getSRID == 4326)
}
+ it("Passed ST_GeomToGeography multilingstring") {
Review Comment:
The test name contains a spelling error: "multilingstring" should be
"multilinestring".
```suggestion
it("Passed ST_GeomToGeography multilinestring") {
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
