Jackie-Jiang opened a new pull request, #19163:
URL: https://github.com/apache/pinot/pull/19163
## Summary
[#18170] removed the `getCardinality() <= 1` short-circuit from
`MutableColumnStatistics.isSorted()` while refactoring the surrounding code.
The returned value is unchanged — a column with a single distinct value still
scans to `true` — but every single-valued constant column now walks the entire
forward index with a `Dictionary.compare` per document at segment-commit time.
For variable-width types the compare is not cheap:
`StringOffHeapMutableDictionary.compare` materializes two `String`s per
document, so an N-document constant column allocates 2N strings only to find
them equal.
This restores the check and addresses two things that multiplied the cost.
**`isSorted()` was recomputed on every call.** It is queried up to three
times per column during segment creation — `BaseSegmentCreator.adaptConfig`
(twice when the forward index is disabled), `ForwardIndexCreatorFactory` via
`IndexCreationContext.Common`, which delegates straight through to the
`ColumnShape` with no caching of its own, and the `segment.metadata` write.
`MutableColumnStatistics` and `MutableNoDictColumnStatistics` now memoize it.
Lazily rather than in the constructor: the compaction subclasses call
`super(...)` and then compute their own `_isSorted` in a single pass,
overriding the getter, so eager computation would burn a wasted scan for every
compacted column.
**`CompactedColumnStatistics` paid the same per-document compare.** It
tracks sortedness inline while collecting used dict ids, so the cardinality
short-circuit does not apply as-is. Two changes there:
- when the dictionary holds a single value the SV scan is skipped entirely —
every document maps to the only dict id, so the used dict ids are exactly `{0}`
and an SV column contributes one entry per valid document
- the remaining compare is guarded by `dictId != prevDictId`, so a
low-cardinality column only compares at run boundaries, and the redundant
`usedDictIds.add` is skipped for repeats
MV columns are untouched — they are never sorted, and their per-document
entry counts and row lengths cannot be derived from the dictionary.
## Test coverage
The existing `testFixedWidthConstantValue` asserted `isSorted()` is `true`
at cardinality 1, which passes on both the short-circuiting and the scanning
path — that is why the regression went unnoticed. The new assertions are on the
interaction with the forward index instead:
- `verify(forwardIndex, never()).getDictId(anyInt())` in
`MutableColumnStatisticsTest.testFixedWidthConstantValue` and in a new
`CompactedColumnStatisticsTest.testSingleValueDictionarySkipsScan`
- `testIsSortedScansOnce` calls `isSorted()` twice over a three-value column
and asserts exactly `numDocs` reads, pinning the memoization
[#18170]: https://github.com/apache/pinot/pull/18170
--
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]
