You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by xi...@apache.org on 2026/04/15 11:31:32 UTC

(pinot) 14/25: Handle null H3 coordinates with continueOnError (#18040)

This is an automated email from the ASF dual-hosted git repository.

xiangfu pushed a commit to branch release/1.5.0-STP-2.134.x
in repository https://gitbox.apache.org/repos/asf/pinot.git

commit c16234ac3f3304313fce1294853d6cbb92beac92
Author: Xiang Fu <xi...@apache.org>
AuthorDate: Tue Mar 31 03:23:12 2026 -0700

    Handle null H3 coordinates with continueOnError (#18040)
---
 .../impl/inv/geospatial/BaseH3IndexCreator.java    | 25 +++++-
 .../segment/local/segment/index/H3IndexTest.java   | 88 ++++++++++++++++++++++
 2 files changed, 111 insertions(+), 2 deletions(-)

diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java
index 5daa0ff163f..9d5d449b130 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java
@@ -19,6 +19,7 @@
 package org.apache.pinot.segment.local.segment.creator.impl.inv.geospatial;
 
 import com.google.common.base.Preconditions;
+import com.uber.h3core.exceptions.H3Exception;
 import java.io.BufferedOutputStream;
 import java.io.DataOutputStream;
 import java.io.File;
@@ -119,15 +120,35 @@ public abstract class BaseH3IndexCreator implements GeoSpatialIndexCreator {
   public void add(@Nullable Geometry geometry)
       throws IOException {
     if (_continueOnError && !(geometry instanceof Point)) {
-      MetricUtils.updateIndexingErrorMetric(_tableNameWithType, H3IndexType.INDEX_DISPLAY_NAME);
-      _nextDocId++;
+      skipInvalidGeometry();
       return;
     }
     Preconditions.checkState(geometry != null, "Null geometry record found and continueOnError is disabled");
     Preconditions.checkState(geometry instanceof Point, "H3 index can only be applied to Point, got: %s",
         geometry.getGeometryType());
     Coordinate coordinate = geometry.getCoordinate();
+    if (_continueOnError && coordinate == null) {
+      skipInvalidGeometry();
+      return;
+    }
     Preconditions.checkState(coordinate != null, "Point has null coordinate and continueOnError is disabled");
+    try {
+      addCoordinate(coordinate);
+    } catch (H3Exception e) {
+      if (_continueOnError) {
+        skipInvalidGeometry();
+        return;
+      }
+      throw e;
+    }
+  }
+
+  private void skipInvalidGeometry() {
+    MetricUtils.updateIndexingErrorMetric(_tableNameWithType, H3IndexType.INDEX_DISPLAY_NAME);
+    _nextDocId++;
+  }
+
+  private void addCoordinate(Coordinate coordinate) {
     // TODO: support multiple resolutions
     long h3Id = H3Utils.H3_CORE.latLngToCell(coordinate.y, coordinate.x, _lowestResolution);
     RoaringBitmapWriter<RoaringBitmap> bitmapWriter = _postingListMap.get(h3Id);
diff --git a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/H3IndexTest.java b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/H3IndexTest.java
index 7e1bee3bf57..2ea6ae55e64 100644
--- a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/H3IndexTest.java
+++ b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/H3IndexTest.java
@@ -21,6 +21,7 @@ package org.apache.pinot.segment.local.segment.index;
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.Lists;
+import com.uber.h3core.exceptions.H3Exception;
 import java.io.File;
 import java.io.IOException;
 import java.util.Collections;
@@ -49,6 +50,7 @@ import org.apache.pinot.spi.config.table.FieldConfig;
 import org.locationtech.jts.geom.Coordinate;
 import org.locationtech.jts.geom.MultiPoint;
 import org.locationtech.jts.geom.Point;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
@@ -208,6 +210,58 @@ public class H3IndexTest implements PinotBuffersAfterMethodCheckRule {
     }
   }
 
+  @Test
+  public void testSkipPointWithNullCoordinate()
+      throws Exception {
+    String columnName = "skipPointWithNullCoordinate";
+    int res = 5;
+    H3IndexResolution resolution = new H3IndexResolution(Collections.singletonList(res));
+
+    try (GeoSpatialIndexCreator creator = new OnHeapH3IndexCreator(TEMP_DIR, columnName, "myTable_OFFLINE", true,
+        resolution)) {
+      Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, 20));
+      Point pointWithNullCoordinate = GeometryUtils.GEOMETRY_FACTORY.createPoint((Coordinate) null);
+      creator.add(point);
+      creator.add(pointWithNullCoordinate);
+      creator.add(point);
+
+      creator.seal();
+    }
+
+    File indexFile = new File(TEMP_DIR, columnName + V1Constants.Indexes.H3_INDEX_FILE_EXTENSION);
+    try (PinotDataBuffer buffer = PinotDataBuffer.mapReadOnlyBigEndianFile(indexFile);
+        H3IndexReader reader = new ImmutableH3IndexReader(buffer)) {
+      long h3Id = H3Utils.H3_CORE.latLngToCell(20, 10, res);
+      Assert.assertEquals(reader.getDocIds(h3Id), ImmutableRoaringBitmap.bitmapOf(0, 2));
+    }
+  }
+
+  @Test
+  public void testSkipPointWithInvalidCoordinate()
+      throws Exception {
+    String columnName = "skipPointWithInvalidCoordinate";
+    int res = 5;
+    H3IndexResolution resolution = new H3IndexResolution(Collections.singletonList(res));
+
+    try (GeoSpatialIndexCreator creator = new OnHeapH3IndexCreator(TEMP_DIR, columnName, "myTable_OFFLINE", true,
+        resolution)) {
+      Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, 20));
+      Point pointWithInvalidCoordinate = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, Double.NaN));
+      creator.add(point);
+      creator.add(pointWithInvalidCoordinate);
+      creator.add(point);
+
+      creator.seal();
+    }
+
+    File indexFile = new File(TEMP_DIR, columnName + V1Constants.Indexes.H3_INDEX_FILE_EXTENSION);
+    try (PinotDataBuffer buffer = PinotDataBuffer.mapReadOnlyBigEndianFile(indexFile);
+        H3IndexReader reader = new ImmutableH3IndexReader(buffer)) {
+      long h3Id = H3Utils.H3_CORE.latLngToCell(20, 10, res);
+      Assert.assertEquals(reader.getDocIds(h3Id), ImmutableRoaringBitmap.bitmapOf(0, 2));
+    }
+  }
+
   @Test
   public void testSkipInvalidGeometryContinueOnErrorFalse()
       throws Exception {
@@ -262,6 +316,40 @@ public class H3IndexTest implements PinotBuffersAfterMethodCheckRule {
     }
   }
 
+  @Test
+  public void testSkipPointWithNullCoordinateContinueOnErrorFalse()
+      throws Exception {
+    String columnName = "skipPointWithNullCoordinate";
+    int res = 5;
+    H3IndexResolution resolution = new H3IndexResolution(Collections.singletonList(res));
+
+    try (GeoSpatialIndexCreator creator = new OnHeapH3IndexCreator(TEMP_DIR, columnName, "myTable_OFFLINE", false,
+        resolution)) {
+      Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, 20));
+      Point pointWithNullCoordinate = GeometryUtils.GEOMETRY_FACTORY.createPoint((Coordinate) null);
+      creator.add(point);
+
+      Assert.assertThrows(IllegalStateException.class, () -> creator.add(pointWithNullCoordinate));
+    }
+  }
+
+  @Test
+  public void testSkipPointWithInvalidCoordinateContinueOnErrorFalse()
+      throws Exception {
+    String columnName = "skipPointWithInvalidCoordinate";
+    int res = 5;
+    H3IndexResolution resolution = new H3IndexResolution(Collections.singletonList(res));
+
+    try (GeoSpatialIndexCreator creator = new OnHeapH3IndexCreator(TEMP_DIR, columnName, "myTable_OFFLINE", false,
+        resolution)) {
+      Point point = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, 20));
+      Point pointWithInvalidCoordinate = GeometryUtils.GEOMETRY_FACTORY.createPoint(new Coordinate(10, Double.NaN));
+      creator.add(point);
+
+      Assert.assertThrows(H3Exception.class, () -> creator.add(pointWithInvalidCoordinate));
+    }
+  }
+
   public static class ConfTest extends AbstractSerdeIndexContract {
 
     protected void assertEquals(H3IndexConfig expected) {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org