Fix: Update BoundingBox for empty and antimeridian handling

Author: zhangfengcdt

parquet-column/src/main/java/org/apache/parquet/column/statistics/geospatial/BoundingBox.java

@@ -18,6 +18,7 @@
  */
 package org.apache.parquet.column.statistics.geospatial;
 
+import org.apache.parquet.ShouldNeverHappenException;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.Envelope;
 import org.locationtech.jts.geom.Geometry;
@@ -169,7 +170,7 @@ public boolean isXYEmpty() {
    * @return true if the X dimension is empty, false otherwise.
    */
   public boolean isXEmpty() {
-    return Double.isInfinite(xMin) && Double.isInfinite(xMax);
+    return Double.isInfinite(xMin - xMax);
   }
 
   /**
@@ -178,7 +179,7 @@ public boolean isXEmpty() {
    * @return true if the Y dimension is empty, false otherwise.
    */
   public boolean isYEmpty() {
-    return Double.isInfinite(yMin) && Double.isInfinite(yMax);
+    return Double.isInfinite(yMin - yMax);
   }
 
   /**
@@ -187,7 +188,7 @@ public boolean isYEmpty() {
    * @return true if the Z dimension is empty, false otherwise.
    */
   public boolean isZEmpty() {
-    return Double.isInfinite(zMin) && Double.isInfinite(zMax);
+    return Double.isInfinite(zMin - zMax);
   }
 
   /**
@@ -196,14 +197,28 @@ public boolean isZEmpty() {
    * @return true if the M dimension is empty, false otherwise.
    */
   public boolean isMEmpty() {
-    return Double.isInfinite(mMin) && Double.isInfinite(mMax);
+    return Double.isInfinite(mMin - mMax);
+  }
+
+  /**
+   * Checks if the X dimension of this bounding box wraps around the antimeridian.
+   * This occurs when the minimum X value is greater than the maximum X value,
+   * which is allowed by the Parquet specification for geometries that cross the antimeridian.
+   *
+   * @return true if the X dimension wraps around, false otherwise.
+   */
+  public boolean isXWraparound() {
+    return isWraparound(xMin, xMax);
   }
 
   /**
    * Expands this bounding box to include the bounds of another box.
    * After merging, this bounding box will contain both its original extent
    * and the extent of the other bounding box.
    *
+   * If either this bounding box or the other has wraparound X coordinates,
+   * the X dimension will be marked as invalid (set to NaN) in the result.
+   *
    * @param other the other BoundingBox whose bounds will be merged into this one
    */
   public void merge(BoundingBox other) {
@@ -218,16 +233,27 @@ public void merge(BoundingBox other) {
       return;
     }
 
-    this.xMin = Math.min(this.xMin, other.xMin);
-    this.xMax = Math.max(this.xMax, other.xMax);
+    // We don't yet support merging wraparound bounds.
+    // Rather than throw, we mark the X bounds as invalid.
+    if (isXWraparound() || other.isXWraparound()) {
+      // Mark X dimension as invalid by setting to NaN
+      xMin = Double.NaN;
+      xMax = Double.NaN;
+    } else {
+      // Normal case - merge X bounds
+      this.xMin = Math.min(this.xMin, other.xMin);
+      this.xMax = Math.max(this.xMax, other.xMax);
+    }
+
+    // Always merge Y, Z, and M bounds
     this.yMin = Math.min(this.yMin, other.yMin);
     this.yMax = Math.max(this.yMax, other.yMax);
     this.zMin = Math.min(this.zMin, other.zMin);
     this.zMax = Math.max(this.zMax, other.zMax);
     this.mMin = Math.min(this.mMin, other.mMin);
     this.mMax = Math.max(this.mMax, other.mMax);
 
-    // Update the validity of this bounding box based on the other bounding box
+    // Update the validity of this bounding box
     valid = isXYValid();
   }
 
@@ -272,12 +298,33 @@ public void update(Geometry geometry) {
    * - X bounds are only updated if both minX and maxX are not NaN
    * - Y bounds are only updated if both minY and maxY are not NaN
    *
-   * This allows partial updates while preserving valid dimensions.
+   * Note: JTS (Java Topology Suite) does not natively support wraparound envelopes
+   * or geometries that cross the antimeridian (±180° longitude). It operates strictly
+   * in a 2D Cartesian coordinate space and doesn't account for the Earth's spherical
+   * nature or longitudinal wrapping.
+   *
+   * When JTS encounters a geometry that crosses the antimeridian, it will represent
+   * it with an envelope spanning from the westernmost to easternmost points, often
+   * covering most of the Earth's longitude range (e.g., minX=-180, maxX=180).
+   *
+   * The wraparound check below is defensive but should never be triggered with standard
+   * JTS geometry operations, as JTS will never produce an envelope with minX > maxX.
+   *
+   * @throws ShouldNeverHappenException if the update creates an X wraparound condition
    */
   private void updateBounds(double minX, double maxX, double minY, double maxY) {
     if (!Double.isNaN(minX) && !Double.isNaN(maxX)) {
-      xMin = Math.min(xMin, minX);
-      xMax = Math.max(xMax, maxX);
+      double newXMin = Math.min(xMin, minX);
+      double newXMax = Math.max(xMax, maxX);
+
+      // Check if the update would create a wraparound condition
+      // This should never happen with standard JTS geometry operations
+      if (isWraparound(newXMin, newXMax)) {
+        throw new ShouldNeverHappenException("Wraparound X is not supported by BoundingBox.update()");
+      }
+
+      xMin = newXMin;
+      xMax = newXMax;
     }
 
     if (!Double.isNaN(minY) && !Double.isNaN(maxY)) {
@@ -302,6 +349,16 @@ public void reset() {
     valid = true;
   }
 
+  /**
+   * The Parquet specification allows X bounds to be "wraparound" to allow for
+   * more compact bounding boxes when a geometry happens to include components
+   * on both sides of the antimeridian (e.g., the nation of Fiji). This function
+   * checks for that case (see GeoStatistics::lower_bound/upper_bound for more details).
+   */
+  public static boolean isWraparound(double xmin, double xmax) {
+    return !Double.isInfinite(xmin - xmax) && xmin > xmax;
+  }
+
   /**
    * Creates a copy of the current bounding box.
    *

parquet-column/src/test/java/org/apache/parquet/column/statistics/geospatial/TestBoundingBox.java

@@ -681,4 +681,103 @@ public void testMergingRowGroupBoundingBoxes() {
     Assert.assertEquals(900.0, reverseMergeBox.getYMax(), 0.0);
     Assert.assertTrue(reverseMergeBox.isValid());
   }
+
+  @Test
+  public void testIsXValidAndIsYValid() {
+    // Test with valid X and Y
+    BoundingBox validBox = new BoundingBox(1, 2, 3, 4, 5, 6, 7, 8);
+    Assert.assertTrue(validBox.isXValid());
+    Assert.assertTrue(validBox.isYValid());
+
+    // Test with invalid X (NaN)
+    BoundingBox invalidXBox = new BoundingBox(Double.NaN, 2, 3, 4, 5, 6, 7, 8);
+    Assert.assertFalse(invalidXBox.isXValid());
+    Assert.assertTrue(invalidXBox.isYValid());
+
+    // Test with invalid Y (NaN)
+    BoundingBox invalidYBox = new BoundingBox(1, 2, Double.NaN, 4, 5, 6, 7, 8);
+    Assert.assertTrue(invalidYBox.isXValid());
+    Assert.assertFalse(invalidYBox.isYValid());
+
+    // Test with both X and Y invalid
+    BoundingBox invalidXYBox = new BoundingBox(Double.NaN, Double.NaN, Double.NaN, Double.NaN, 5, 6, 7, 8);
+    Assert.assertFalse(invalidXYBox.isXValid());
+    Assert.assertFalse(invalidXYBox.isYValid());
+    Assert.assertFalse(invalidXYBox.isXYValid());
+  }
+
+  @Test
+  public void testIsXEmptyAndIsYEmpty() {
+    // Empty bounding box (initial state)
+    BoundingBox emptyBox = new BoundingBox();
+    Assert.assertTrue(emptyBox.isXEmpty());
+    Assert.assertTrue(emptyBox.isYEmpty());
+    Assert.assertTrue(emptyBox.isXYEmpty());
+
+    // Non-empty box
+    BoundingBox nonEmptyBox = new BoundingBox(1, 2, 3, 4, 5, 6, 7, 8);
+    Assert.assertFalse(nonEmptyBox.isXEmpty());
+    Assert.assertFalse(nonEmptyBox.isYEmpty());
+    Assert.assertFalse(nonEmptyBox.isXYEmpty());
+
+    // Box with empty X dimension only
+    GeometryFactory gf = new GeometryFactory();
+    BoundingBox emptyXBox = new BoundingBox();
+    // Only update Y dimension
+    emptyXBox.update(gf.createPoint(new Coordinate(Double.NaN, 5)));
+    Assert.assertTrue(emptyXBox.isXEmpty());
+    Assert.assertFalse(emptyXBox.isYEmpty());
+    Assert.assertTrue(emptyXBox.isXYEmpty());
+
+    // Box with empty Y dimension only
+    BoundingBox emptyYBox = new BoundingBox();
+    // Only update X dimension
+    emptyYBox.update(gf.createPoint(new Coordinate(10, Double.NaN)));
+    Assert.assertFalse(emptyYBox.isXEmpty());
+    Assert.assertTrue(emptyYBox.isYEmpty());
+    Assert.assertTrue(emptyYBox.isXYEmpty());
+  }
+
+  @Test
+  public void testIsXWraparound() {
+    // Normal bounding box (no wraparound)
+    BoundingBox normalBox = new BoundingBox(1, 2, 3, 4, 5, 6, 7, 8);
+    Assert.assertFalse(normalBox.isXWraparound());
+
+    // Wraparound box (xMin > xMax)
+    BoundingBox wraparoundBox = new BoundingBox(170, 20, 10, 20, 0, 0, 0, 0);
+    Assert.assertTrue(wraparoundBox.isXWraparound());
+
+    // Edge case: equal bounds
+    BoundingBox equalBoundsBox = new BoundingBox(10, 10, 20, 20, 0, 0, 0, 0);
+    Assert.assertFalse(equalBoundsBox.isXWraparound());
+
+    // Test static method directly
+    Assert.assertTrue(BoundingBox.isWraparound(180, -180));
+    Assert.assertFalse(BoundingBox.isWraparound(-180, 180));
+  }
+
+  @Test
+  public void testWraparoundHandlingInMerge() {
+    // Test with two normal boxes
+    BoundingBox box1 = new BoundingBox(10, 20, 10, 20, 0, 0, 0, 0);
+    BoundingBox box2 = new BoundingBox(15, 25, 15, 25, 0, 0, 0, 0);
+    box1.merge(box2);
+
+    Assert.assertTrue(box1.isValid());
+    Assert.assertEquals(10.0, box1.getXMin(), 0.0);
+    Assert.assertEquals(25.0, box1.getXMax(), 0.0);
+
+    // Test with one wraparound box
+    BoundingBox normalBox = new BoundingBox(0, 10, 0, 10, 0, 0, 0, 0);
+    BoundingBox wraparoundBox = new BoundingBox(170, -170, 5, 15, 0, 0, 0, 0);
+
+    normalBox.merge(wraparoundBox);
+
+    Assert.assertFalse(normalBox.isValid());
+    Assert.assertTrue(Double.isNaN(normalBox.getXMin()));
+    Assert.assertTrue(Double.isNaN(normalBox.getXMax()));
+    Assert.assertEquals(0.0, normalBox.getYMin(), 0.0);
+    Assert.assertEquals(15.0, normalBox.getYMax(), 0.0);
+  }
 }