Copilot commented on code in PR #2899:
URL: https://github.com/apache/sedona/pull/2899#discussion_r3179617079
##########
common/src/main/java/org/apache/sedona/common/Functions.java:
##########
@@ -868,6 +868,22 @@ public static String asWKT(Geometry geometry) {
return GeomUtils.getWKT(geometry);
}
+ /** PostGIS-format text for a Box2D: {@code BOX(xmin ymin, xmax ymax)}. NULL
on null input. */
+ public static String asWKT(Box2D box) {
+ if (box == null) {
+ return null;
+ }
+ return "BOX("
+ + box.getXMin()
+ + " "
+ + box.getYMin()
+ + ", "
+ + box.getXMax()
+ + " "
+ + box.getYMax()
+ + ")";
Review Comment:
This overload makes the public `Functions.asWKT` API return `BOX(...)`,
which is not valid WKT. The existing `asWKT` helpers are used as geometry
serializers and are round-trippable through WKT/EWKT parsers in
`FunctionsTest`, so callers that treat `asWKT(*)` as a generic geometry encoder
will start getting non-parseable output for `Box2D` values.
##########
common/src/main/java/org/apache/sedona/common/Constructors.java:
##########
@@ -289,6 +289,29 @@ public static Geometry makeEnvelope(double minX, double
minY, double maxX, doubl
return makeEnvelope(minX, minY, maxX, maxY, 0);
}
+ /**
+ * Convert a {@link Box2D} to a closed rectangular polygon. NULL on null
input. Mirrors PostGIS
+ * {@code box2d::geometry}.
+ */
+ public static Geometry geomFromBox2D(Box2D box) {
Review Comment:
This new common-layer helper is only exercised indirectly through Spark SQL
tests. `ConstructorsTest` already has direct unit coverage for adjacent
constructors like `makeEnvelope`, so we should add common-module tests here for
null and edge-case boxes; otherwise regressions in `geomFromBox2D` won't be
caught until the Spark integration suites run.
##########
common/src/main/java/org/apache/sedona/common/Constructors.java:
##########
@@ -289,6 +289,29 @@ public static Geometry makeEnvelope(double minX, double
minY, double maxX, doubl
return makeEnvelope(minX, minY, maxX, maxY, 0);
}
+ /**
+ * Convert a {@link Box2D} to a closed rectangular polygon. NULL on null
input. Mirrors PostGIS
+ * {@code box2d::geometry}.
+ */
+ public static Geometry geomFromBox2D(Box2D box) {
+ if (box == null) {
+ return null;
+ }
+ double xmin = box.getXMin();
+ double ymin = box.getYMin();
+ double xmax = box.getXMax();
+ double ymax = box.getYMax();
+ Coordinate[] coords =
+ new Coordinate[] {
+ new Coordinate(xmin, ymin),
+ new Coordinate(xmin, ymax),
+ new Coordinate(xmax, ymax),
+ new Coordinate(xmax, ymin),
+ new Coordinate(xmin, ymin)
+ };
+ return GEOMETRY_FACTORY.createPolygon(coords);
Review Comment:
This reimplements the same envelope-to-polygon ring assembly that
`polygonFromEnvelope(...)` already does a few lines above. Keeping two copies
of that logic means any future fix to rectangle construction has to be made in
both places, which makes the constructors easy to let drift apart.
##########
common/src/main/java/org/apache/sedona/common/Functions.java:
##########
@@ -868,6 +868,22 @@ public static String asWKT(Geometry geometry) {
return GeomUtils.getWKT(geometry);
}
+ /** PostGIS-format text for a Box2D: {@code BOX(xmin ymin, xmax ymax)}. NULL
on null input. */
+ public static String asWKT(Box2D box) {
Review Comment:
There is no direct `FunctionsTest` coverage for this new formatter even
though that suite already exercises the existing `asWKT` behavior. Right now
only the Spark SQL path is asserted, so a regression in the common `BOX(...)`
formatting would not be caught until the integration tests run.
--
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]
