Jackie-Jiang commented on code in PR #16002:
URL: https://github.com/apache/pinot/pull/16002#discussion_r2178427998
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -83,8 +88,9 @@ public abstract class BaseH3IndexCreator implements
GeoSpatialIndexCreator {
int _nextDocId;
- BaseH3IndexCreator(File indexDir, String columnName, H3IndexResolution
resolution)
+ BaseH3IndexCreator(File indexDir, String columnName, H3IndexResolution
resolution, String tableNameWithType)
Review Comment:
Same for other constructors
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -108,10 +114,19 @@ public Geometry deserialize(byte[] bytes) {
}
@Override
- public void add(Geometry geometry)
+ public void add(@Nullable Geometry geometry)
throws IOException {
- Preconditions.checkState(geometry instanceof Point, "H3 index can only be
applied to Point, got: %s",
- geometry.getGeometryType());
+ if (geometry == null || !(geometry instanceof Point)) {
+ if (_tableNameWithType != null) {
+ String metricKeyName =
+ _tableNameWithType + "-" +
H3IndexType.INDEX_DISPLAY_NAME.toUpperCase(Locale.US) + "-indexingError";
+ ServerMetrics.get().addMeteredTableValue(metricKeyName,
ServerMeter.INDEXING_FAILURES, 1);
+ } else {
Review Comment:
Can `_tableNameWithType` ever be `null`?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -477,6 +477,9 @@ private void rewriteForwardIndexForCompressionChange(String
column, ColumnMetada
try (ForwardIndexReader<?> reader = ForwardIndexType.read(segmentWriter,
columnMetadata)) {
IndexCreationContext.Builder builder =
IndexCreationContext.builder().withIndexDir(indexDir).withColumnMetadata(columnMetadata);
+ if (_tableConfig != null) {
Review Comment:
We shouldn't create index handler when table config is unavailable. If
`_tableConfig` can be null here, we should check the code path and prevent
index handler being created
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/inv/geospatial/BaseH3IndexCreator.java:
##########
@@ -83,8 +88,9 @@ public abstract class BaseH3IndexCreator implements
GeoSpatialIndexCreator {
int _nextDocId;
- BaseH3IndexCreator(File indexDir, String columnName, H3IndexResolution
resolution)
+ BaseH3IndexCreator(File indexDir, String columnName, H3IndexResolution
resolution, String tableNameWithType)
Review Comment:
(minor) Put `tableNameWithType` in front of `columnName`
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
