Copilot commented on code in PR #2865:
URL: https://github.com/apache/sedona/pull/2865#discussion_r3157568993
##########
common/src/test/java/org/apache/sedona/common/Geography/FunctionTest.java:
##########
@@ -251,4 +251,66 @@ public void contains_nullHandling() throws ParseException {
assertFalse(Functions.contains(g1, null));
assertFalse(Functions.contains(null, g1));
}
+
+ // ─── Level 4: ST_Buffer ──────────────────────────────────────────────────
+
+ @Test
+ public void buffer_nullInputReturnsNull() throws ParseException {
+ assertNull(Functions.buffer(null, 100.0));
+ assertNull(Functions.buffer(null, 100.0, "quad_segs=4"));
+ }
+
+ @Test
Review Comment:
The new docs state that when the input Geography has no SRID (SRID=0), the
buffered result should be normalized to EPSG:4326. There isn’t a test covering
the SRID=0 case in the new buffer tests; adding one would prevent regressions
(and would catch the current SRID propagation issue in
`common/.../geography/Functions.buffer`).
```suggestion
@Test
public void buffer_sridZeroNormalizesResultTo4326() throws ParseException {
Geography origin = Constructors.geogFromWKT("POINT (0 0)", 0);
Geography buffered = Functions.buffer(origin, 1000.0);
assertNotNull(buffered);
assertEquals(4326, Functions.srid(buffered));
}
@Test
```
##########
common/src/main/java/org/apache/sedona/common/geography/Functions.java:
##########
@@ -147,6 +147,53 @@ public static String asEWKT(Geography geography) {
return geography.toEWKT();
}
+ // ─── Level 4: spherical buffer ───────────────────────────────────────────
+
+ /**
+ * Returns a Geography that represents the metric ε-buffer of {@code g} on
the sphere, where
+ * {@code radiusMeters} is interpreted as meters along the spheroid.
Implementation reuses the
+ * existing geometry-side spheroidal buffer (UTM project → JTS planar buffer
→ unproject), which
+ * gives accurate sub-UTM-zone results; for very large geographies the UTM
round-trip's accuracy
+ * caveats apply (see ST_Buffer's docs).
+ */
+ public static Geography buffer(Geography g, double radiusMeters) {
+ return buffer(g, radiusMeters, "");
+ }
+
+ /**
+ * Geography is inherently spheroidal, so the {@code useSpheroid} flag (only
meaningful for the
+ * planar Geometry version of ST_Buffer) is rejected for Geography inputs.
This overload exists to
+ * give a clear, actionable error when callers try to pass it; without it
the resolver would
+ * coerce the boolean to a string and fail later inside the
buffer-parameters parser with a
+ * confusing message.
+ */
+ public static Geography buffer(Geography g, double radiusMeters, boolean
useSpheroid) {
+ throw new IllegalArgumentException(
+ "ST_Buffer does not accept a useSpheroid argument for Geography inputs
(Geography is "
+ + "always spheroidal). Use ST_Buffer(geog, distance) or "
+ + "ST_Buffer(geog, distance, parameters) instead.");
+ }
+
+ /**
+ * Same as {@link #buffer(Geography, double)} but allows a JTS-style buffer
parameters string
+ * ({@code "quad_segs=8 endcap=round join=round mitre_limit=5.0
side=both"}). The string is parsed
+ * by the existing geometry-side parser.
+ */
+ public static Geography buffer(Geography g, double radiusMeters, String
parameters) {
+ if (g == null) return null;
+ Geometry jts = toJTS(g);
+ if (jts == null) return null;
+ int srid = g.getSRID();
+ // Geography is always lon/lat; default to WGS84 when the source has no
SRID set.
+ jts.setSRID(srid != 0 ? srid : 4326);
+ Geometry buffered =
+ org.apache.sedona.common.Functions.buffer(jts, radiusMeters, true,
parameters);
+ if (buffered == null) return null;
+ Geography result = Constructors.geomToGeography(buffered);
+ result.setSRID(srid);
Review Comment:
When the input Geography has SRID = 0, this method sets the JTS SRID to 4326
for buffering, but then overwrites the output Geography SRID back to 0
(`result.setSRID(srid)`). That contradicts the comment and the new docs note
that SRID-less inputs should be normalized to WGS84, and it also discards the
SRID already set by `Constructors.geomToGeography(buffered)`.
Consider only setting the result SRID when the input SRID is non-zero, or
setting it to `4326` when the input SRID is 0 (or simply not overriding the
SRID at all).
```suggestion
if (srid != 0) {
result.setSRID(srid);
}
```
--
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]
