arunkumarucet commented on code in PR #19128:
URL: https://github.com/apache/pinot/pull/19128#discussion_r3681709674
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/H3IndexHandler.java:
##########
@@ -212,12 +213,20 @@ private void
handleDictionaryBasedColumn(SegmentDirectory.Writer segmentWriter,
.createIndexReader(segmentWriter, colIndexConf, columnMetadata);
GeoSpatialIndexCreator h3IndexCreator =
StandardIndexes.h3().createIndexCreator(context, config)) {
int numDocs = columnMetadata.getTotalDocs();
+ // Decode each distinct dictionary value at most once. Old segments
reloaded after a geo column was added are
+ // dominated by a single empty default value (cardinality 1), so caching
by dictId avoids re-reading and
+ // re-decoding that same value for every doc. Decoding through the
creator's toGeometry() fast-paths the empty
+ // default value to null instead of failing the whole reload with a
BufferUnderflowException, tolerating it the
+ // same way the segment-creation path does.
Review Comment:
Good catch — fixed in 197027a0bd. I dropped the
`Geometry[dictionary.length()]` cache. The reload paths that hit this are the
cardinality-1 empty-default case, so I now decode that single value once and
reuse it for every doc; higher-cardinality columns decode per doc via
`toGeometry()` (which still fast-paths the empty default, so no per-row
exception and no retained heap).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
