This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new e5ec005aba0 Skip empty/default geometry values when building H3 index
on reload (#19032)
e5ec005aba0 is described below
commit e5ec005aba02d2f789a611dc9806f4206b3acc52
Author: Arunkumar Saravanan <[email protected]>
AuthorDate: Thu Jul 30 12:29:17 2026 +0530
Skip empty/default geometry values when building H3 index on reload (#19032)
When a geo column with an H3 index is added to a schema, segments created
before the source columns existed have no data to derive the geometry from,
so the derived BYTES column is filled with its default null value (an empty
byte array). Building the H3 index over those rows during segment reload
called GeometrySerializer.deserialize() directly on the empty bytes,
throwing
a BufferUnderflowException that propagated out of the Helix state transition
and parked the segment in an ERROR state, blocking upgrades.
The segment-creation path already tolerates this via the
GeoSpatialIndexCreator
default add(value, dictId), which swallows deserialization failures and
treats
them as null geometry (skipped when continueOnError is enabled). Route the
two
H3IndexHandler reload paths through the same tolerant add(value, dictId) so
empty/default values are skipped consistently instead of crashing the
reload.
Add SegmentPreProcessorTest#testH3IndexCreationOnEmptyDefaultValue (v1 and
v3)
covering a derived H3 column whose default null value is the empty byte
array.
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
---
.../index/loader/invertedindex/H3IndexHandler.java | 9 ++-
.../index/loader/SegmentPreProcessorTest.java | 39 ++++++++++++
.../data/newColumnsSchemaWithH3EmptyDefault.json | 72 ++++++++++++++++++++++
3 files changed, 117 insertions(+), 3 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java
index d81e4d9cc1d..b0fae087d5f 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java
@@ -28,7 +28,6 @@ import org.apache.commons.io.FileUtils;
import org.apache.pinot.segment.local.segment.index.loader.BaseIndexHandler;
import org.apache.pinot.segment.local.segment.index.loader.LoaderUtils;
import
org.apache.pinot.segment.local.segment.index.readers.geospatial.ImmutableH3IndexReader;
-import org.apache.pinot.segment.local.utils.GeometrySerializer;
import org.apache.pinot.segment.spi.ColumnMetadata;
import org.apache.pinot.segment.spi.V1Constants;
import org.apache.pinot.segment.spi.creator.IndexCreationContext;
@@ -215,7 +214,10 @@ public class H3IndexHandler extends BaseIndexHandler {
int numDocs = columnMetadata.getTotalDocs();
for (int i = 0; i < numDocs; i++) {
int dictId = forwardIndexReader.getDictId(i, readerContext);
-
h3IndexCreator.add(GeometrySerializer.deserialize(dictionary.getBytesValue(dictId)));
+ // Route through add(value, dictId) so that empty/default geometry
values (e.g. old segments reloaded after
+ // the geo column was added, which have no source data to build a
Point from) are tolerated the same way the
+ // segment-creation path tolerates them, instead of failing the whole
reload with a BufferUnderflowException.
+ h3IndexCreator.add(dictionary.getBytesValue(dictId), dictId);
}
h3IndexCreator.seal();
}
@@ -234,7 +236,8 @@ public class H3IndexHandler extends BaseIndexHandler {
GeoSpatialIndexCreator h3IndexCreator =
StandardIndexes.h3().createIndexCreator(context, config)) {
int numDocs = columnMetadata.getTotalDocs();
for (int i = 0; i < numDocs; i++) {
-
h3IndexCreator.add(GeometrySerializer.deserialize(forwardIndexReader.getBytes(i,
readerContext)));
+ // See handleDictionaryBasedColumn: add(value, dictId) tolerates
empty/default geometry values on reload.
+ h3IndexCreator.add(forwardIndexReader.getBytes(i, readerContext), -1);
}
h3IndexCreator.seal();
}
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
index 61947d0e60a..424e33d8a29 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessorTest.java
@@ -136,6 +136,8 @@ public class SegmentPreProcessorTest implements
PinotBuffersAfterClassCheckRule
private static final String NEW_COLUMNS_SCHEMA_WITH_FST =
"data/newColumnsSchemaWithFST.json";
private static final String NEW_COLUMNS_SCHEMA_WITH_TEXT =
"data/newColumnsSchemaWithText.json";
private static final String NEW_COLUMNS_SCHEMA_WITH_H3_JSON =
"data/newColumnsSchemaWithH3Json.json";
+ private static final String NEW_COLUMNS_SCHEMA_WITH_H3_EMPTY_DEFAULT =
+ "data/newColumnsSchemaWithH3EmptyDefault.json";
private static final String NEW_COLUMNS_SCHEMA_WITH_NO_FORWARD_INDEX =
"data/newColumnsSchemaWithForwardIndexDisabled.json";
private static final String NEW_INT_METRIC_COLUMN_NAME = "newIntMetric";
@@ -164,6 +166,7 @@ public class SegmentPreProcessorTest implements
PinotBuffersAfterClassCheckRule
private final Schema _newColumnsSchemaWithFST;
private final Schema _newColumnsSchemaWithText;
private final Schema _newColumnsSchemaWithH3Json;
+ private final Schema _newColumnsSchemaWithH3EmptyDefault;
private final Schema _newColumnsSchemaWithForwardIndexDisabled;
private Set<String> _noDictionaryColumns;
@@ -208,6 +211,9 @@ public class SegmentPreProcessorTest implements
PinotBuffersAfterClassCheckRule
resourceUrl = classLoader.getResource(NEW_COLUMNS_SCHEMA_WITH_H3_JSON);
assertNotNull(resourceUrl);
_newColumnsSchemaWithH3Json = Schema.fromFile(new
File(resourceUrl.getFile()));
+ resourceUrl =
classLoader.getResource(NEW_COLUMNS_SCHEMA_WITH_H3_EMPTY_DEFAULT);
+ assertNotNull(resourceUrl);
+ _newColumnsSchemaWithH3EmptyDefault = Schema.fromFile(new
File(resourceUrl.getFile()));
resourceUrl =
classLoader.getResource(NEW_COLUMNS_SCHEMA_WITH_NO_FORWARD_INDEX);
assertNotNull(resourceUrl);
_newColumnsSchemaWithForwardIndexDisabled = Schema.fromFile(new
File(resourceUrl.getFile()));
@@ -1665,6 +1671,39 @@ public class SegmentPreProcessorTest implements
PinotBuffersAfterClassCheckRule
assertEquals(singleFileIndex.length(), initFileSize);
}
+ /// Regression test for the H3 index builder crashing a segment on
empty/default geometry values.
+ ///
+ /// When a geo column is added to the schema after a segment was built, old
segments have no source
+ /// data to derive it from, so the derived BYTES column is filled with its
default null value -- the
+ /// empty byte array. Building an H3 index over those rows used to call
+ ///
[org.apache.pinot.segment.local.utils.GeometrySerializer#deserialize(byte\[\])]
directly on the
+ /// empty bytes, throwing a `BufferUnderflowException` that propagated out
of the reload and parked
+ /// the segment in an ERROR state. The handler now routes through the
creator's tolerant add path,
+ /// which skips undeserializable/default values when `continueOnError` is
enabled (set by
+ /// [#resetIndexConfigs()]), exactly like the segment-creation path.
+ @Test(dataProvider = "bothV1AndV3")
+ public void testH3IndexCreationOnEmptyDefaultValue(SegmentVersion
segmentVersion)
+ throws Exception {
+ buildSegment(segmentVersion);
+
+ // Add newH3Col as a derived column whose default null value is the empty
byte array (no explicit
+ // defaultNullValue in the schema), mirroring old segments reloaded after
a geo column was added.
+ runPreProcessor(_newColumnsSchemaWithH3EmptyDefault);
+ SegmentMetadataImpl segmentMetadata = new SegmentMetadataImpl(INDEX_DIR);
+ assertNotNull(segmentMetadata.getColumnMetadataFor("newH3Col"));
+
+ // Build the H3 index over the empty/default values. This must not throw
and must produce the index.
+ _fieldConfigMap.put("newH3Col",
+ new FieldConfig("newH3Col", FieldConfig.EncodingType.DICTIONARY,
List.of(FieldConfig.IndexType.H3), null,
+ Map.of("resolutions", "5")));
+ runPreProcessor(_newColumnsSchemaWithH3EmptyDefault);
+
+ try (SegmentDirectory segmentDirectory = new
SegmentLocalFSDirectory(INDEX_DIR, ReadMode.mmap);
+ SegmentDirectory.Reader reader = segmentDirectory.createReader()) {
+ assertTrue(reader.hasIndexFor("newH3Col", StandardIndexes.h3()));
+ }
+ }
+
@Test(dataProvider = "bothV1AndV3")
public void testIfNeedProcess(SegmentVersion segmentVersion)
throws Exception {
diff --git
a/pinot-segment-local/src/test/resources/data/newColumnsSchemaWithH3EmptyDefault.json
b/pinot-segment-local/src/test/resources/data/newColumnsSchemaWithH3EmptyDefault.json
new file mode 100644
index 00000000000..7ea00135351
--- /dev/null
+++
b/pinot-segment-local/src/test/resources/data/newColumnsSchemaWithH3EmptyDefault.json
@@ -0,0 +1,72 @@
+{
+ "schemaName": "testDataMV",
+ "dimensionFieldSpecs": [
+ {
+ "name": "column1",
+ "dataType": "INT"
+ },
+ {
+ "name": "column2",
+ "dataType": "INT"
+ },
+ {
+ "name": "column3",
+ "dataType": "STRING"
+ },
+ {
+ "name": "column4",
+ "dataType": "STRING"
+ },
+ {
+ "name": "column5",
+ "dataType": "STRING"
+ },
+ {
+ "name": "newH3Col",
+ "dataType": "BYTES"
+ },
+ {
+ "name": "column6",
+ "dataType": "INT",
+ "singleValueField": false
+ },
+ {
+ "name": "column7",
+ "dataType": "INT",
+ "singleValueField": false
+ },
+ {
+ "name": "column8",
+ "dataType": "INT"
+ },
+ {
+ "name": "column9",
+ "dataType": "INT"
+ },
+ {
+ "name": "column10",
+ "dataType": "INT"
+ },
+ {
+ "name": "column13",
+ "dataType": "INT"
+ },
+ {
+ "name": "weeksSinceEpochSunday",
+ "dataType": "INT"
+ }
+ ],
+ "metricFieldSpecs": [
+ {
+ "name": "count",
+ "dataType": "INT"
+ }
+ ],
+ "timeFieldSpec": {
+ "incomingGranularitySpec": {
+ "timeType": "DAYS",
+ "dataType": "INT",
+ "name": "daysSinceEpoch"
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
