Jackie-Jiang opened a new pull request, #19316: URL: https://github.com/apache/pinot/pull/19316
## Summary Part of #19218. `DISTINCTCOUNTBITMAP`, `DISTINCTCOUNTHLL`, `DISTINCTCOUNTHLLPLUS`, `DISTINCTCOUNTULL`, `DISTINCTCOUNTTHETASKETCH`, `DISTINCTCOUNTCPCSKETCH`, `FASTHLL`, `SEGMENTPARTITIONEDDISTINCTCOUNT` and the raw and smart variants of each never received the query's null handling option. A null row was aggregated as the column default whatever the query asked for, so `DISTINCTCOUNTHLL` over a column of all nulls answered `1` — the default counted once — instead of `0`. They now extend `NullableSingleInputAggregationFunction` and skip null rows when the option is enabled. The change reaches every aggregation path: `aggregate`, `aggregateGroupBySV` and `aggregateGroupByMV`, single-value and multi-value columns, and the dictionary, raw and serialized-sketch input paths. ### What does not change With the option disabled, null rows are still read as the column default. That is the answer this mode has always given and is a backward-compatibility constraint, not an oversight. The empty-input answer is unchanged in every case. Each counting function already returned `0` for a null intermediate result, and the extractors that substitute an empty accumulator are untouched — so the raw CPC and theta sketch variants still render a serialized empty sketch for a locally empty segment rather than `NULL`. That distinction is per-class and was checked one class at a time rather than assumed. ### Two things a loop-shaped fix does not reach Not every read of the block is a loop. Two range operations needed the same treatment: `RoaringBitmap#addN` over an entire dictionary-id array, and a `subList` of the entire string array in the smart sketch base. Both took every row regardless of the null bitmap. ### Testing `DistinctCountSketchNullHandlingTest` is new. It covers what `AggregationFunctionNullContractTest` cannot: that harness drives one synthetic single-value block through `aggregate` only, so the group-by paths, the multi-value column paths and the serialized sketch input are checked nowhere else. `DistinctCountULLAggregationFunctionTest` and `SegmentPartitionedDistinctCountAggregationFunctionTest` gain enabled/disabled multi-value cases. The multi-value and dictionary-encoded cases were verified by mutation rather than by inspection: reverting a wrapped range back to the full block makes them fail with the null rows counted, so they guard the behaviour instead of passing vacuously. `AggregationFunctionNullContractTest` pins the nineteen types that now honour the option. That set came from a run, not a prediction. -- 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]
