Document and enforce the AggregationFunction null contract

[Jackie-Jiang]

Summary

Null handling is a per-query flag, and the two modes place very different requirements on an AggregationFunction — but that contract was never written down. Implementations drifted apart as a result: some resolved the "nothing was aggregated" case, some threw on it, some carried defensive null branches no caller could reach, and @Nullable annotations disagreed with the code they annotated.

This writes the contract onto AggregationFunction and brings the implementations in line with it.

Behavior change: ten multi-value functions now honour enableNullHandling

DISTINCT_COUNT_MV, DISTINCT_SUM_MV, DISTINCT_AVG_MV, PERCENTILE_MV, PERCENTILE_EST_MV, PERCENTILE_KLL_MV and PERCENTILE_TDIGEST_MV hard-coded nullHandlingEnabled = false in their constructors — they did not accept the parameter at all, so the query option had no effect on them whatever it was set to.

The null-aware machinery was already present and correct in the shared base classes; it was simply switched off. Threading the option through the constructors is the whole fix.

PERCENTILE_RAW_EST_MV, PERCENTILE_RAW_TDIGEST_MV and PERCENTILE_RAW_KLL_MV change with them. Each one extends or wraps one of the seven, so it reaches the option through the function it delegates to and needed no edit of its own — seven constructors, ten functions whose answers move.

For a query with enableNullHandling=true, these functions now skip null rows instead of folding in the column default, which changes their results. DISTINCT_SUM_MV over an all-null input returns NULL rather than 0, for example. With the option disabled they are unaffected.

Fixes an NPE in the broker response path

PERCENTILERAWEST, PERCENTILERAWKLL, PERCENTILERAWTDIGEST and their MV variants wrapped the intermediate result in a serializer (SerializedQuantileDigest / SerializedKLL / SerializedTDigest) without checking it. Those wrappers dereference what they are handed in both toString() and compareTo(), so a null intermediate did not fail at extraction — it failed later, when the broker rendered the value.

For the KLL variants this is reachable on the single-stage engine in both modes, because PercentileKLLAggregationFunction.extractAggregationResult returns the holder's value directly and that is null for an untouched holder. A query whose segments are all pruned hits it.

Fixes PERCENTILETDIGEST answering NaN where the contract says NULL

PercentileTDigestAggregationFunction.extractFinalResult had no empty-accumulator branch, and its extractAggregationResult builds an empty digest rather than returning null for an untouched holder. TDigest.quantile() returns NaN for an empty digest, so PERCENTILETDIGEST over an all-null column with enableNullHandling=true answered NaN while PERCENTILE, PERCENTILEEST and PERCENTILEKLL answered NULL for the same query.

The branch now matches its three siblings, which already had it. PERCENTILETDIGESTMV inherits the method, and PERCENTILESMARTTDIGEST gets the same test on its t-digest branch so that it agrees with its own value-list branch — which of the two a query lands in depends only on whether the accumulator crossed the conversion threshold.

The test is gated on the null handling option, so the disabled mode is unchanged.

Makes extractFinalResult the sole decision point wherever the option is available

Substituting an empty accumulator in the extraction methods destroys the "nothing was aggregated" signal before extractFinalResult can act on it, and forces every later stage to re-derive it with a per-type emptiness probe. Every function that receives the null handling option now returns null from both extraction methods and renders the disabled-mode value in extractFinalResult instead:

• AVG, MIN_MAX_RANGE, VAR_POP/VAR_SAMP/STDDEV_POP/STDDEV_SAMP
• DISTINCT_COUNT, DISTINCT_SUM, DISTINCT_AVG (via their shared base) and DISTINCT_COUNT_OFF_HEAP
• PERCENTILE, PERCENTILE_EST, PERCENTILE_TDIGEST, PERCENTILE_SMART_TDIGEST, with the raw and multi-value variants of each

Answers are unchanged in both modes. Each disabled-mode value was read off what that function's substituted accumulator rendered — 0.0 for a distinct sum over an empty set, NaN for a distinct average, Long.MIN_VALUE for an empty QuantileDigest, NaN for an empty TDigest. The raw percentile wrappers changed with their delegates rather than after them: they build the empty digest where it is rendered and serialize it, so a raw percentile still emits a serialized empty digest with the option off.

Two answers do change, both toward the contract. VAR_POP answered NULL for a group with no rows while MIN_MAX_RANGE answered -Infinity for the identical query, because the two disagreed about which extraction path substitutes — reachable through a filtered aggregation, where one group key space is shared across every aggregation and a group created by one can hold no rows for another. And DISTINCT_COUNT_OFF_HEAP no longer calls close() on a shared placeholder set.

The functions that never receive the option are left alone. They cannot be conformed separately: without it, extractFinalResult has nothing to decide with. That is now recorded as part of the first known deviation rather than as a deviation of its own.

The contract

• Null handling disabled — nulls read as the column default, the accumulator is primitive, and no null tracking happens. An untouched accumulator is indistinguishable from one that aggregated to the type's identity, so this mode cannot tell "nothing was aggregated" apart from a real result: the answer is whatever the accumulator's initial state renders to (0 for SUM, +Infinity for MIN). This is a performance path, and those answers are a backward-compatibility constraint rather than an attempt at SQL conformance.
• Null handling enabled — SQL semantics. A null intermediate result means nothing was aggregated, and extractFinalResult is the only place that decides what that means for a given function: 0 for the counting functions, null for the value functions.

Object-backed accumulators whose type has no identity to render (MAXSTRING, MINSTRING, ANYVALUE) return null even with the flag off, so extractFinalResult has to accept null in both modes.

Settles the merge identity once, in the caller

The "nothing was aggregated" case is the identity of merging and means the same thing for every aggregation, so it does not belong in each implementation. AggregationFunctionUtils#merge and #mergeFinalResult resolve a null operand and only then delegate, and all six call sites route through them.

Two of those call sites — AggregationResultsBlockMerger and SortedRecordsMerger — previously had no null handling at all; two others carried // TODO: Fix it blocks that this removes. With the identity settled in one place, merge implementations only ever see two real values, so the null branches inside them are unreachable and have been dropped.

Collapses the multi-value variants onto their single-value implementations

Fifteen MV classes were carrying code they did not need:

• Twelve (MaxMV, MinMV, SumMV, AvgMV, MinMaxRangeMV, the percentile MVs, the HLL MVs) overrode all three aggregate* methods purely to force the multi-value path. Their parents already dispatch on blockValSet.isSingleValue() in all three, and select the same method for a multi-value column, so the overrides only re-derived a decision the parent already makes.
• Three (DistinctCountMV, DistinctSumMV, DistinctAvgMV) were siblings of their single-value counterparts rather than subclasses, duplicating extractFinalResult, mergeFinalResult and getFinalResultColumnType. Those copies had already drifted — the MV ones disagreed with the SV ones on the empty-input answer, which this also fixes.

This removes 52 methods. It is not purely cosmetic: the deleted overrides called aggregateMV unconditionally, which fails on a single-value block set, whereas the parent's dispatch handles it.

Also

• extractFinalResult resolves the "nothing was aggregated" case across the remaining functions instead of dereferencing it.
• @Nullable annotations are aligned with what each implementation actually does — added where a method genuinely returns null, removed from merge parameters where the annotation contradicted the contract.
• BaseBooleanAggregationFunction.aggregateGroupByMV mirrors its single-value counterpart and skips null rows when null handling is enabled.

Tests

The test harness now probes block value types until one drives the function, rather than consulting a hard-coded map of which function reads which type, and feeds every input expression the function declares instead of only the first. Single-value string, bytes and int blocks were added alongside the existing long and double ones.

That brought 18 functions into the census that were previously skipped, including MINSTRING, MAXSTRING, SUMINT, SUMLONG, SUMPRECISION, FIRSTWITHTIME, LASTWITHTIME, COVARPOP, ARRAYAGG and LISTAGG. Fourteen of them turned out to honour the query's null handling option and had never been checked.

AggregationFunctionNullContractTest enforces the contract against every aggregation function that can be constructed generically — 95 of the 103 AggregationFunctionType values, under both flag settings. The eight it cannot construct are pinned in both directions, so a newly added function cannot drop out of the contract unnoticed and a stale exclusion cannot linger.

It asserts that the final result renders, not merely that extraction returned. That distinction is what the raw percentile bug turned on: extraction succeeded and the failure surfaced downstream.

It also covers the behavior change directly. testNullHandlingOptionReachesEveryFunctionThatHonoursIt aggregates an all-null block through every function under both settings and pins the exact set whose answer depends on the option, in both directions: a function that starts honouring it has to be added deliberately, and one that stops is a regression. All ten above were absent from that set before this change.

The harness drives single-value long and double columns, so functions that read another value type, need a multi-value block, or take more than one input column cannot be aggregated through it. Those are pinned in a second set rather than skipped silently, and the rest of the contract is still asserted against them.

BooleanAggQueriesTest gains multi-value group-by coverage for BOOL_AND / BOOL_OR, including groups that separate skipping a null from folding it in as the column default.

Known deviations

Four are recorded as a TODO on the interface rather than addressed here:

• Several aggregation methods still fold the column default into the aggregate instead of skipping null rows when null handling is enabled: the distinct-count family, the tuple and frequency sketches, the statistical functions, the first/last-with-time functions, and the funnel family.

• The multi-stage engine constructs every aggregation function with null handling enabled and never consults the query option, so a query that disables it still gets enabled-mode semantics there. SUM over an all-pruned query is NULL on the multi-stage engine and 0 on the single-stage engine. This may well be intended, given the multi-stage engine is the SQL-conformant one — flagging it rather than changing it.

• With null handling disabled, a null reaching the data table is mis-serialized unless the column type is OBJECT: the writer uses the encoding reserved for OBJECT whatever the column type is, which overruns the narrower fixed-size slot of STRING, INT and FLOAT, reads back as a value for LONG and DOUBLE, and reads back as an empty array for array types. Group-by keys already work around it by writing a placeholder alongside a null bitmap; aggregate values need the same treatment.

  This PR widens that deviation rather than fixing it, deliberately. Eleven functions used to throw on a null intermediate result and now return null for it, so on the server-side-final-result path a query that previously failed loudly can now write a data table no reader can interpret. It needs that option and zero aggregated rows, so the path is narrow, but a silent wrong answer is a worse failure than the exception it replaces. Fixing the writer is follow-up work.

• The object-backed functions substitute an empty accumulator in extractAggregationResult, extractGroupByResult or both, rather than returning null and letting extractFinalResult render the disabled-mode value. Which path substitutes is inconsistent across the nineteen functions with an object result holder, and sometimes within one, so a function can answer differently depending on whether the query groups. A filtered aggregation makes the difference observable, because one group key space is shared across every aggregation in the query, so a group created by one of them can hold no rows for another. With null handling disabled VARPOP then answers NULL for that group while MINMAXRANGE answers -Infinity, though their aggregation paths agree.

  Conforming them means making extractFinalResult the sole decision point — both extraction methods return null for an untouched accumulator and the disabled-mode value is rendered there. That is a follow-up rather than part of this PR: it changes what a server puts on the wire for every function that substitutes today, so it needs its own upgrade-ordering review, and the test harness added here has to be extended to reach the sketch functions before the change can be verified rather than asserted.

[Copilot]

Pull request overview

This PR formalizes and enforces Apache Pinot’s AggregationFunction null-handling contract (null-handling enabled vs disabled), centralizes merge-identity behavior (treating null as the empty-multiset identity) in AggregationFunctionUtils, and updates aggregation implementations and execution/reduction call sites to follow the contract—preventing downstream NPEs in broker result rendering for “all segments pruned / empty intermediate” cases.

Changes:

• Document the null contract on AggregationFunction, align method signatures/@Nullable annotations, and ensure extractFinalResult resolves null intermediate results safely.
• Add AggregationFunctionUtils.merge(..) / mergeFinalResult(..) and route merge call sites through them to consistently treat null as the merge identity.
• Add/expand tests to enforce the contract across aggregation types and cover boolean MV group-by null semantics.

Reviewed changes

Copilot reviewed 82 out of 82 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java	Use AggregationFunctionUtils.merge* to handle null merge identity in MSQ group-by merge.
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageAggregationExecutor.java	Use AggregationFunctionUtils.merge to handle null merge identity in MSQ aggregation merge.
pinot-core/src/test/java/org/apache/pinot/queries/BooleanAggQueriesTest.java	Add MV group-by coverage for BOOL_AND/BOOL_OR under both null-handling modes.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/PercentileSmartTDigestAggregationFunctionTest.java	Remove merge-with-null test now that null identity is handled by caller utilities.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MinStringAggregationFunctionTest.java	Remove merge-with-null test now that null identity is handled by caller utilities.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/MaxStringAggregationFunctionTest.java	Remove merge-with-null test now that null identity is handled by caller utilities.
pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionNullContractTest.java	New contract test enforcing safe extraction/rendering for empty intermediates and merge identity.
pinot-core/src/main/java/org/apache/pinot/core/query/reduce/AggregationDataTableReducer.java	Route intermediate/final merging through AggregationFunctionUtils.merge*.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/VarianceAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/TimeSeriesAggregationFunction.java	Handle null intermediate result in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumValuesIntegerTupleSketchAggregationFunction.java	Handle null intermediate result in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumPrecisionAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumMVAggregationFunction.java	Make star-tree pre-aggregated branch null-aware via fold/skip-null helpers.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumLongAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumIntAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/SumAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/StUnionAggregationFunction.java	Ensure extractFinalResult resolves empty-multiset (null) safely.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileTDigestAggregationFunction.java	Resolve null intermediate results in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileSmartTDigestAggregationFunction.java	Remove null-merge branches; resolve null intermediate in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawTDigestAggregationFunction.java	Avoid NPE by returning null for empty-multiset in serializer wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawKLLMVAggregationFunction.java	Avoid NPE by returning null for empty-multiset in serializer wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawKLLAggregationFunction.java	Avoid NPE by returning null for empty-multiset in serializer wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileRawEstAggregationFunction.java	Avoid NPE by returning null for empty-multiset in serializer wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileKLLAggregationFunction.java	Align extraction/merge behavior with null contract (no null merge inputs).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileEstAggregationFunction.java	Resolve null intermediate results in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileAggregationFunction.java	Resolve null intermediate results in extractFinalResult (preserve empty-list sentinel semantics).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/ParentExprMinMaxAggregationFunction.java	Align @Nullable behavior for group-by/final extraction.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/ModeAggregationFunction.java	Resolve null intermediate results in extractFinalResult (preserve empty-map sentinel semantics).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinStringAggregationFunction.java	Remove null-merge branches; align extraction nullability (also contains indentation regression).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinMaxRangeAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinLongAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MinAggregationFunction.java	Align nullability and remove unreachable null-merge branches (also contains indentation regression).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxStringAggregationFunction.java	Remove null-merge branches; align extraction nullability (also contains indentation regression).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxLongAggregationFunction.java	Align nullability and remove unreachable null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/MaxAggregationFunction.java	Align nullability and remove unreachable null-merge branches (also contains indentation regression).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/LastWithTimeAggregationFunction.java	Resolve null intermediate results in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IntegerTupleSketchAggregationFunction.java	Remove null-merge branches consistent with centralized merge identity.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/IdSetAggregationFunction.java	Resolve null intermediate results in extractFinalResult (no object to serialize).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/HistogramAggregationFunction.java	Align group-by extraction nullability and final extraction signature.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelStepDurationStatsAggregationFunction.java	Align final extraction nullability; remove unreachable null-merge-final branch.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelMaxStepAggregationFunction.java	Align final extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelMatchStepAggregationFunction.java	Align final extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelEventsFunctionEvalAggregationFunction.java	Remove null-merge branches; align extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelCompleteCountAggregationFunction.java	Align final extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/window/FunnelBaseAggregationFunction.java	Remove null-merge branches; align extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/funnel/FunnelCountAggregationFunction.java	Remove null-merge branches; align final extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FrequentStringsSketchAggregationFunction.java	Remove null-merge branches; resolve null intermediate safely in final wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FrequentLongsSketchAggregationFunction.java	Remove null-merge branches; resolve null intermediate safely in final wrapper.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FourthMomentAggregationFunction.java	Align final extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FirstWithTimeAggregationFunction.java	Resolve null intermediate results in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FastHLLAggregationFunction.java	Resolve null intermediate results for empty-multiset (distinct count = 0).
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctSumMVAggregationFunction.java	Resolve null intermediate results consistently with SV variant.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctSumAggregationFunction.java	Resolve null intermediate results consistently with empty-set behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountULLAggregationFunction.java	Remove null-merge branches per centralized merge identity.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountThetaSketchAggregationFunction.java	Align final extraction nullability annotation usage.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountSmartULLAggregationFunction.java	Align final extraction signature to accept @Nullable intermediate.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountSmartHLLPlusAggregationFunction.java	Remove null-merge branches; make final-result merge non-null per contract.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountMVAggregationFunction.java	Align @Nullable annotation placement with behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLPlusAggregationFunction.java	Align @Nullable annotation placement with behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountHLLAggregationFunction.java	Align @Nullable annotation placement with behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountCPCSketchAggregationFunction.java	Remove null-merge branches; align final extraction annotation placement.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountBitmapAggregationFunction.java	Align @Nullable annotation placement with behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctCountAggregationFunction.java	Align @Nullable annotation placement with behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctAvgMVAggregationFunction.java	Resolve null intermediate results consistently with SV variant.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/DistinctAvgAggregationFunction.java	Resolve null intermediate results consistently with empty-set behavior.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/CovarianceAggregationFunction.java	Resolve null intermediate results in final extraction.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/CountMVAggregationFunction.java	Make star-tree pre-aggregated branches null-aware via fold/skip-null helpers.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/CountAggregationFunction.java	Make star-tree pre-aggregated branches null-aware via fold/skip-null helpers.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/ChildAggregationFunction.java	Align final extraction signature for @Nullable intermediate.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/BaseBooleanAggregationFunction.java	Skip null rows in MV group-by when null handling enabled; align nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AvgValueIntegerTupleSketchAggregationFunction.java	Handle null intermediate result in extractFinalResult.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AvgAggregationFunction.java	Make serialized-avg aggregation paths null-aware; align nullability and remove null-merge branches.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/array/SumArrayLongAggregationFunction.java	Align extraction nullability for empty-multiset cases.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/array/SumArrayDoubleAggregationFunction.java	Align extraction nullability for empty-multiset cases.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/array/ListAggFunction.java	Remove null-merge branches; resolve null intermediate result in final extraction.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/array/BaseArrayAggFunction.java	Align aggregation/group-by extraction nullability.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AnyValueAggregationFunction.java	Align extraction nullability; remove merge null-branch per centralized merge identity.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunctionUtils.java	Add centralized merge / mergeFinalResult helpers resolving null as merge identity.
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/AggregationFunction.java	Document/enforce null contract; adjust merge/mergeFinalResult signatures and semantics.
pinot-core/src/main/java/org/apache/pinot/core/operator/combine/merger/AggregationResultsBlockMerger.java	Route block merging through AggregationFunctionUtils.merge.
pinot-core/src/main/java/org/apache/pinot/core/data/table/SortedRecordsMerger.java	Route record merging through AggregationFunctionUtils.merge.
pinot-core/src/main/java/org/apache/pinot/core/data/table/IndexedTable.java	Route intermediate/final record merging through AggregationFunctionUtils.merge*.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 6.70732% with 153 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.95%. Comparing base (baaf0a7) to head (a9d301f).

Files with missing lines	Patch %	Lines
...gregation/function/AggregationFunctionFactory.java	0.00%	17 Missing ⚠️
...ation/function/BaseBooleanAggregationFunction.java	0.00%	13 Missing ⚠️
...gation/function/PercentileAggregationFunction.java	0.00%	9 Missing ⚠️
...ion/PercentileSmartTDigestAggregationFunction.java	0.00%	8 Missing ⚠️
...aggregation/function/AggregationFunctionUtils.java	30.00%	6 Missing and 1 partial ⚠️
...ation/function/DistinctAvgAggregationFunction.java	0.00%	5 Missing ⚠️
...ation/function/DistinctSumAggregationFunction.java	0.00%	5 Missing ⚠️
...ion/function/PercentileEstAggregationFunction.java	0.00%	4 Missing ⚠️
...ction/PercentileRawTDigestAggregationFunction.java	0.00%	4 Missing ⚠️
...function/PercentileTDigestAggregationFunction.java	0.00%	4 Missing ⚠️
... and 40 more

❗ There is a different number of reports uploaded between BASE (baaf0a7) and HEAD (a9d301f). Click for more details.

HEAD has 4 uploads less than BASE

Flag	BASE (baaf0a7)	HEAD (a9d301f)
unittests1	1	0
unittests	2	1
java-25	5	4
temurin	5	4

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #19158       +/-   ##
=============================================
- Coverage     66.62%   38.95%   -27.68%     
+ Complexity     1423     1422        -1     
=============================================
  Files          3443     3443               
  Lines        218626   218365      -261     
  Branches      34792    34721       -71     
=============================================
- Hits         145662    85058    -60604     
- Misses        61240   125548    +64308     
+ Partials      11724     7759     -3965     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-25	38.95% <6.70%> (-27.68%)	⬇️
temurin	38.95% <6.70%> (-27.68%)	⬇️
unittests	38.94% <6.70%> (-27.68%)	⬇️
unittests1	?
unittests2	38.94% <6.70%> (+0.04%)	⬆️

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.

[yashmayya]

I read the full diff at head (17ff99e). The direction is correct, and the merge centralization is complete rather than partial.

What I confirmed

• All six call sites use the new helpers. I searched every .merge( and .mergeFinalResult( receiver in main/. No unconverted caller of AggregationFunction.merge is left. The only direct calls that remain are in the PercentileRaw* and DistinctCountRaw* wrappers. Each of those delegates from inside its own merge, so both operands are real values. The removal of the null branches from the implementations is therefore safe.
• The two multi-stage executor rewrites keep the same behavior. The old if (x == null) continue; is the identity that the helper now returns.
• The multi-value collapse is correct, and it repairs a real fault. For each collapsed class, the dispatch of the parent is a superset of the deleted override. It selects the same method for a multi-value block. On master, MinMaxRangeMVAggregationFunction.aggregate called aggregateMV for every block. A pre-aggregated star-tree column is BYTES, so that call went to getDoubleValuesMV(). The parent tests for BYTES first. AvgMV had the same fault.
• The single-value and multi-value alignment is now complete. Every family whose single-value class takes the option now has a multi-value class that takes it too. The HLL, bitmap and theta families take no option on either side.
• DistinctCountMV and DistinctSumMV lost their mergeFinalResult overrides safely. Both parents hold an equivalent implementation, so neither function falls back to UnsupportedOperationException.
• The NPE is real and reachable. EmptyResponseUtils.buildEmptyAggregationResultTable calls extractFinalResult(extractAggregationResult(createAggregationResultHolder())) directly.

The description understates the blast radius

The description says this: when the option is disabled, the seven multi-value functions are unaffected. That holds for the single-stage engine. It does not hold for the multi-stage engine.

AggregateOperator builds every aggregation function with null handling enabled (AggregateOperator.java:321). The constructors of these multi-value functions discarded the flag before, so the final stage never reached the isEmpty() && _nullHandlingEnabled branch. Now it reaches it.

For a multi-stage query with default options, over an input where nothing was aggregated:

Function	master	this PR
percentileMV(mv, 50)	-Infinity	NULL
distinctSumMV(mv)	0.0	NULL
distinctAvgMV(mv)	NaN	NULL

The new values are better. But the change is unconditional on that engine, and no query option controls it. Add this to the description, and to the release notes.

The count is also low. The factory threads the option into ten functions, not seven. PERCENTILERAWESTMV, PERCENTILERAWKLLMV and PERCENTILERAWTDIGESTMV change too.

CI is green, so no test covers multi-stage multi-value aggregates over an empty input today.

The regression test is missing

The description says that this NPE reached production. No test covers the path that produced it. EmptyResponseUtilsTest is the natural home. That test also runs ColumnDataType.convert, which is the call that threw. The render() helper in the new test calls toString() only, so it does not reproduce that step.

The rest is in the line comments. Two of them matter. The contract promises an answer that three percentile functions do not give. The change also makes a documented serialization fault newly reachable.