gortiz opened a new pull request, #20002:
URL: https://github.com/apache/pinot/pull/20002
## Summary
A REALTIME table cannot commit any segment when its sorted column is in
`noDictionaryColumns` and the table uses ingestion aggregation
(`ingestionConfig.aggregationConfigs` or `aggregateMetrics`). The segment build
fails with a `NullPointerException`, and the partition stops ingesting. This PR
changes the sort step at seal time: it now picks the sort path by the index
that exists, not by "the column has a dictionary".
```
java.lang.NullPointerException: Cannot invoke
"org.apache.pinot.segment.spi.index.mutable.MutableInvertedIndex.getDocIds(int)"
because "invertedIndex" is null
at
o.a.p.segment.local.indexsegment.mutable.MutableSegmentImpl.getSortedDocIdsWithInvertedIndex(MutableSegmentImpl.java:1405)
at
o.a.p.segment.local.indexsegment.mutable.MutableSegmentImpl.getSortedDocIdIterationOrderWithSortedColumn(MutableSegmentImpl.java:1378)
at
o.a.p.segment.local.realtime.converter.RealtimeSegmentConverter.build(RealtimeSegmentConverter.java:124)
```
## Background
Two changes interact:
- #17965 allows a sorted column without a dictionary.
`RealtimeSegmentConfig.Builder` adds the inverted index to the sorted column
only when the table config enables its dictionary.
- #18955 allows no-dictionary aggregation-key columns.
`MutableSegmentImpl.isNoDictionaryColumn` forces a dictionary on DIMENSION,
DATE_TIME and TIME columns in the consuming segment, because the aggregation
key uses dictIds.
As a result, the sorted column has a dictionary but no inverted index in the
consuming segment. `getSortedDocIdIterationOrderWithSortedColumn` sees the
dictionary and reads the inverted index, which is null.
| Column in the consuming segment | Sort path before | Sort path after |
|---|---|---|
| No dictionary | Raw forward index | Raw forward index |
| Dictionary and inverted index | Inverted index | Inverted index |
| Dictionary, no inverted index | Inverted index (NPE) | **New:** dictIds
from the forward index |
## Change
- `MutableSegmentImpl.getSortedDocIdIterationOrderWithSortedColumn` uses the
inverted-index path only when the inverted index exists.
- The new `getSortedDocIdsWithDictIdForwardIndex` sorts the dictIds with
`dictionary::compare` and gives a rank to each dictId. Then it does a counting
sort of the docIds by the rank of their forward-index dictId. The cost is O(n +
k log k) for n docs and k distinct values. The sort is stable, so the docIds of
one value stay in ascending order, as in the inverted-index path.
- The raw forward-index path is not usable here. With a dictionary, the
forward index stores dictIds, not values.
- The rest of the build already supports this column.
`RealtimeSegmentStatsContainer` uses `MutableColumnStatistics` for a column
with a dictionary. The immutable segment writes the column as raw, as the table
config requires.
The other option was to also add the inverted index when aggregation forces
the dictionary. It needs a copy of the force logic in
`RealtimeSegmentConfig.Builder`. It also costs memory for an inverted index on
a sort key that often has a high cardinality.
## Tests
`RealtimeSegmentConverterTest.testNoDictSortedColumnAsIngestionAggregationKey`
covers INT, LONG, STRING and BYTES, with the row-major and the column-major
segment builders. Each key is inserted twice, in reverse order. The test
asserts that the rows aggregate, that the column is sorted and has no
dictionary in the immutable segment, and that the SUM values are correct.
Without the fix, all cases fail with the NPE above.
The existing `RealtimeSegmentConverterTest` and `MutableSegmentImpl*Test`
suites pass.
--
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]
