[GH-2830] Fix S2 Geography WKB writer to emit OGC-conformant polygon closure

common/src/main/java/org/apache/sedona/common/S2Geography/WKBWriter.java

@@ -313,23 +313,7 @@ private void writePolygon(int geometryType, PolygonGeography poly, OutStream os)
       throws IOException {
     writeByteOrder(os);
     writeGeometryType(geometryType, poly, os);
-    S2Polygon s2poly = poly.polygon;
-    List<S2Loop> loops = s2poly.getLoops();
-    writeInt(loops.size(), os);
-    for (S2Loop loop : loops) {
-      int n = loop.numVertices();
-      writeInt(n, os);
-      for (int i = 0; i < n; i++) {
-        S2LatLng ll = new S2LatLng(loop.vertex(i));
-        double lon = ll.lngDegrees();
-        double lat = ll.latDegrees();
-
-        ByteOrderValues.putDouble(lon, buf, byteOrder);
-        os.write(buf, 8);
-        ByteOrderValues.putDouble(lat, buf, byteOrder);
-        os.write(buf, 8);
-      }
-    }
+    writeS2PolygonRings(poly.polygon, os);
   }
 
   private void writeMultiPolygon(int geometryType, MultiPolygonGeography multiPoly, OutStream os)
@@ -352,32 +336,49 @@ private void writeMultiPolygon(int geometryType, MultiPolygonGeography multiPoly
       // 4a) Nested Polygon header
       writeByteOrder(os);
       writeGeometryType(WKBConstants.wkbPolygon, pg, os);
+      // 4b) Ring count + rings
+      writeS2PolygonRings(((PolygonGeography) pg).polygon, os);
+    }
 
-      // 4b) Number of rings
-      PolygonGeography s2poly = (PolygonGeography) pg;
-      List<S2Loop> loops = s2poly.polygon.getLoops();
-      writeInt(loops.size(), os);
+    // 5) Restore SRID flag
+    this.includeSRID = oldIncludeSRID;
+  }
 
-      // 4c) For each ring, write vertex count + coords
-      for (S2Loop loop : loops) {
-        int n = loop.numVertices();
-        writeInt(n, os);
+  /**
+   * Write the body of a POLYGON WKB: the ring count followed by each ring as {@code N+1} vertices
+   * with the closing vertex equal to the first (OGC closure). S2Loop stores N distinct vertices
+   * (loop is implicitly closed from {@code vertex(n-1)} back to {@code vertex(0)}); OGC WKB
+   * requires the closing duplicate so downstream OGC-strict readers (e.g., geoarrow-c's WKB reader
+   * used by sedona-db's s2geography kernels) don't treat the ring as unclosed/degenerate.
+   *
+   * <p>A degenerate empty loop ({@code n == 0}) is preserved as a 0-point ring rather than having a
+   * closing vertex fabricated for it.
+   */
+  private void writeS2PolygonRings(S2Polygon s2poly, OutStream os) throws IOException {
+    List<S2Loop> loops = s2poly.getLoops();
+    writeInt(loops.size(), os);
+    for (S2Loop loop : loops) {
+      int n = loop.numVertices();
+      if (n == 0) {
+        writeInt(0, os);
+      } else {
+        writeInt(n + 1, os);
         for (int i = 0; i < n; i++) {
-          S2LatLng ll = new S2LatLng(loop.vertex(i));
-          double lon = ll.lngDegrees();
-          double lat = ll.latDegrees();
-
-          // X (lon) then Y (lat)—matches Point/LineString ordering
-          ByteOrderValues.putDouble(lon, buf, byteOrder);
-          os.write(buf, 8);
-          ByteOrderValues.putDouble(lat, buf, byteOrder);
-          os.write(buf, 8);
+          writeS2PointLonLat(loop.vertex(i), os);
         }
+        // Closing duplicate (== vertex(0))
+        writeS2PointLonLat(loop.vertex(0), os);
       }
     }
+  }
 
-    // 5) Restore SRID flag
-    this.includeSRID = oldIncludeSRID;
+  /** Write a single S2Point as (lon, lat) doubles in WKB ordering. */
+  private void writeS2PointLonLat(S2Point point, OutStream os) throws IOException {
+    S2LatLng ll = new S2LatLng(point);
+    ByteOrderValues.putDouble(ll.lngDegrees(), buf, byteOrder);
+    os.write(buf, 8);
+    ByteOrderValues.putDouble(ll.latDegrees(), buf, byteOrder);
+    os.write(buf, 8);
   }
 
   private void writeGeographyCollection(int geometryType, GeographyCollection gc, OutStream os)

common/src/test/java/org/apache/sedona/common/S2Geography/WKBWriterTest.java

@@ -19,11 +19,16 @@
 package org.apache.sedona.common.S2Geography;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import com.google.common.geometry.S2LatLng;
+import com.google.common.geometry.S2Loop;
 import com.google.common.geometry.S2Point;
+import com.google.common.geometry.S2Polygon;
 import com.google.common.geometry.S2Polyline;
 import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 import java.util.ArrayList;
 import java.util.List;
 import org.junit.Test;
@@ -125,6 +130,115 @@ public void MultiPolygonTest() throws ParseException, IOException {
     assertEquals(0, TestHelper.compareTo(geo, geoWKB));
   }
 
+  /**
+   * Verify that {@link WKBWriter#write(Geography)} emits OGC-conformant POLYGON WKB: each ring's
+   * point count is N+1 with the closing vertex equal to the first. S2Loop stores N distinct
+   * vertices internally (closure is implicit) but the OGC WKB spec requires the closing duplicate;
+   * dropping it produces non-OGC bytes that strict downstream readers (e.g., {@code
+   * GeoArrowWKBReader} used by sedona-db's s2geography kernels) treat as a degenerate / open ring,
+   * which makes {@code ST_Intersects} return {@code true} against every geometry.
+   */
+  @Test
+  public void PolygonRingClosureTest() throws ParseException, IOException {
+    // Single-ring polygon: 4 distinct corners in WKT, must serialize as 5 points.
+    String wkt = "POLYGON((0 0, 0 10, 10 10, 10 0, 0 0))";
+    Geography geo = new WKTReader().read(wkt);
+
+    byte[] wkb = new WKBWriter(2, ByteOrderValues.LITTLE_ENDIAN).write(geo);
+    ByteBuffer bb = ByteBuffer.wrap(wkb).order(ByteOrder.LITTLE_ENDIAN);
+
+    // Header: 1 byte order + 4 type + 4 num_rings + 4 num_points
+    assertEquals("byte order LE", 0x01, bb.get(0));
+    assertEquals("type POLYGON", 3, bb.getInt(1));
+    assertEquals("num rings", 1, bb.getInt(5));
+    int numPoints = bb.getInt(9);
+    assertEquals("ring must be OGC-closed (N+1 points with last==first)", 5, numPoints);
+
+    // Decode all 5 (lon, lat) pairs and verify last == first.
+    int coordOffset = 13;
+    double firstLon = bb.getDouble(coordOffset);
+    double firstLat = bb.getDouble(coordOffset + 8);
+    double lastLon = bb.getDouble(coordOffset + (numPoints - 1) * 16);
+    double lastLat = bb.getDouble(coordOffset + (numPoints - 1) * 16 + 8);
+    assertEquals("closing vertex lon", firstLon, lastLon, 0.0);
+    assertEquals("closing vertex lat", firstLat, lastLat, 0.0);
+    assertEquals(wkb.length, coordOffset + numPoints * 16);
+
+    // Verify that the closed WKB round-trips through the reader (sanity: the
+    // reader must tolerate the N+1 form we now emit).
+    WKBReader readerWKB = new WKBReader();
+    Geography decoded = readerWKB.read(wkb);
+    assertEquals(0, TestHelper.compareTo(geo, decoded));
+  }
+
+  /** Same invariant for {@code MULTIPOLYGON} (per-ring N+1 closure inside each polygon). */
+  @Test
+  public void MultiPolygonRingClosureTest() throws ParseException, IOException {
+    String wkt =
+        "MULTIPOLYGON(((0 0,0 10,10 10,10 0,0 0),(1 1,1 9,9 9,9 1,1 1)),((-9 0,-9 10,-1 10,-1 0,-9 0)))";
+    Geography geo = new WKTReader().read(wkt);
+    byte[] wkb = new WKBWriter(2, ByteOrderValues.LITTLE_ENDIAN).write(geo);
+
+    // Crawl the WKB: outer MULTIPOLYGON header is 1+4+4 = 9 bytes.
+    // Each inner POLYGON: 1 byte order + 4 type + 4 num_rings + (per ring: 4 num_points + 16*N
+    // pts).
+    // We assert every ring's num_points >= 4 AND its last point equals its first.
+    ByteBuffer bb = ByteBuffer.wrap(wkb).order(ByteOrder.LITTLE_ENDIAN);
+    assertEquals(0x01, bb.get(0));
+    assertEquals(6 /* MULTIPOLYGON */, bb.getInt(1));
+    int numPolys = bb.getInt(5);
+    assertEquals(2, numPolys);
+
+    int p = 9;
+    for (int pi = 0; pi < numPolys; pi++) {
+      assertEquals("inner polygon byte order LE", 0x01, bb.get(p));
+      assertEquals("inner geometry type POLYGON", 3, bb.getInt(p + 1));
+      int numRings = bb.getInt(p + 5);
+      p += 9;
+      for (int ri = 0; ri < numRings; ri++) {
+        int n = bb.getInt(p);
+        p += 4;
+        assertTrue(
+            "ring " + ri + " of polygon " + pi + " must have >=4 points (got " + n + ")", n >= 4);
+        double fx = bb.getDouble(p);
+        double fy = bb.getDouble(p + 8);
+        double lx = bb.getDouble(p + (n - 1) * 16);
+        double ly = bb.getDouble(p + (n - 1) * 16 + 8);
+        assertEquals("closing vertex lon (polygon " + pi + " ring " + ri + ")", fx, lx, 0.0);
+        assertEquals("closing vertex lat (polygon " + pi + " ring " + ri + ")", fy, ly, 0.0);
+        p += n * 16;
+      }
+    }
+    assertEquals("consumed entire WKB", wkb.length, p);
+
+    // Sanity round-trip.
+    Geography decoded = new WKBReader().read(wkb);
+    assertEquals(0, TestHelper.compareTo(geo, decoded));
+  }
+
+  /**
+   * Guards the {@code n == 0} path in {@link WKBWriter#writePolygon}: an empty/degenerate {@link
+   * S2Loop} (zero vertices) must serialize as a 0-point ring instead of attempting to fabricate a
+   * closing vertex (which would throw {@code ArithmeticException} on a naive {@code i % n}
+   * indexer).
+   */
+  @Test
+  public void PolygonEmptyLoopTest() throws IOException {
+    S2Polygon emptyPoly = new S2Polygon(new S2Loop(new ArrayList<S2Point>()));
+    Geography geo = new PolygonGeography(emptyPoly);
+
+    byte[] wkb = new WKBWriter(2, ByteOrderValues.LITTLE_ENDIAN).write(geo);
+    ByteBuffer bb = ByteBuffer.wrap(wkb).order(ByteOrder.LITTLE_ENDIAN);
+
+    // 1 byte order + 4 type + 4 num_rings + 4 num_points = 13 bytes total,
+    // num_points must be 0 (no fabricated closing vertex on an empty loop).
+    assertEquals(13, wkb.length);
+    assertEquals(0x01, bb.get(0));
+    assertEquals(3 /* POLYGON */, bb.getInt(1));
+    assertEquals("expected 1 ring (the empty one)", 1, bb.getInt(5));
+    assertEquals("empty ring must serialize as 0 points", 0, bb.getInt(9));
+  }
+
   @Test
   public void CollectionTest() throws ParseException, IOException {
     String wkt =