Null handling for the sketch-backed distinct counts, plus a filtered DISTINCTCOUNTTHETASKETCH group-by MV fix

[Jackie-Jiang]

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.

A pre-existing wrong-results fix, pulled in

DistinctCountThetaSketchAggregationFunction#aggregateGroupByMV indexed the group keys and the
multi-value arrays by the filter ordinal instead of by the row — groupKeysArray[filterIndex],
where filterIndex counts filters. With a single filter, every matching row was credited to row 0's
group keys using row 0's values. Seventeen subscripts across the single-value and multi-value
branches. .get(filterIndex + 1), which really is a filter ordinal, is unchanged.

The plan was to send this separately, but one of the seventeen could not be deferred. Making
deserializeSketches null-aware leaves null in the array for a null row, and that subscript reads
the array, so CustomObjectAccumulator#apply's null check turned the wrong answer into an NPE for
DISTINCTCOUNTTHETASKETCH(sketchCol, params, predicate, '$1') … GROUP BY mvCol. Fixing one of a
pair and deferring its twin reads worse than fixing both, so the whole set is here.

A filtered DISTINCTCOUNTTHETASKETCH grouped by a multi-value column therefore returns different,
correct, results after this change — independently of the null handling option.

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.

The indexing fix adds filtered group-by-MV cases to
DistinctCountThetaSketchAggregationFunctionTest for the single-value, multi-value and string
branches, plus the null-hole case in DistinctCountSketchNullHandlingTest. Run against the pre-fix
commit all four fail with the specific wrong answers — every row collapsed onto one group — rather
than merely throwing.

AggregationFunctionNullContractTest pins the nineteen types that now honour the option. That set
came from a run, not a prediction.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[yashmayya]

Focused on correctness. The mechanical part is clean: I checked every wrapped range in the diff and found no loop bound left at length, no wrong row variable, and the group-by paths of all ten classes consistent.

Two paths were missed, both the non-grouped aggregate over a serialized-sketch column, and both inconsistent with their own group-by siblings. Third comment is a pre-existing bug this PR's rename brought into focus.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 31.26791% with 1919 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.01%. Comparing base (2ae7195) to head (d36aaec).

Files with missing lines	Patch %	Lines
...n/DistinctCountThetaSketchAggregationFunction.java	51.93%	265 Missing and 70 partials ⚠️
.../function/DistinctCountULLAggregationFunction.java	14.23%	224 Missing and 5 partials ⚠️
...nction/DistinctCountBitmapAggregationFunction.java	13.76%	206 Missing and 7 partials ⚠️
...ntPartitionedDistinctCountAggregationFunction.java	21.40%	209 Missing and 4 partials ⚠️
...ction/DistinctCountHLLPlusAggregationFunction.java	26.11%	194 Missing and 4 partials ⚠️
...seDistinctCountSmartSketchAggregationFunction.java	7.06%	171 Missing ⚠️
.../function/DistinctCountHLLAggregationFunction.java	38.32%	163 Missing and 6 partials ⚠️
...ion/DistinctCountCPCSketchAggregationFunction.java	10.19%	137 Missing and 4 partials ⚠️
...tion/DistinctCountSmartHLLAggregationFunction.java	3.12%	62 Missing ⚠️
...tion/DistinctCountSmartULLAggregationFunction.java	1.66%	59 Missing ⚠️
... and 6 more

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #19316       +/-   ##
=============================================
+ Coverage     39.17%   67.01%   +27.84%     
- Complexity     1423     1424        +1     
=============================================
  Files          3462     3462               
  Lines        220695   221647      +952     
  Branches      35258    34951      -307     
=============================================
+ Hits          86447   148545    +62098     
+ Misses       126373    61267    -65106     
- Partials       7875    11835     +3960     

Flag	Coverage Δ
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	67.01% <31.26%> (+27.84%)	⬆️
lane-a	100.00% <ø> (ø)
lane-b	0.00% <ø> (ø)
temurin	67.01% <31.26%> (+27.84%)	⬆️
unittests	67.01% <31.26%> (+27.84%)	⬆️
unittests1	57.71% <31.26%> (?)
unittests2	38.96% <0.28%> (-0.21%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[xiangfu0]

Documentation follow-up: pinot-contrib/pinot-docs#999 (merged)