Jackie-Jiang opened a new pull request, #19217: URL: https://github.com/apache/pinot/pull/19217
## Summary The integer tuple sketch family never received the query's null handling option. Its constructors did not take it, so a null row's column default was folded into the union whatever the query asked for, and `extractFinalResult` could not tell the two modes apart when rendering an empty input. Second of the follow-ups recorded as known deviation 1 on `AggregationFunction` in #19158, after #19211. ### Behavior change With `enableNullHandling=true`, `DISTINCTCOUNTTUPLESKETCH`, `DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH`, `SUMVALUESINTEGERSUMTUPLESKETCH` and `AVGVALUEINTEGERSUMTUPLESKETCH` now skip rows that carry no sketch. **On the multi-stage engine the change is unconditional, and no query option controls it.** `AggregateOperator` builds every aggregation function with null handling enabled and never consults the query's option, so these functions previously discarded a flag the engine had already set to `true`. Worth a release note on the same footing as the multi-value functions in #19158. With the option disabled, each function still renders exactly what it rendered before. ### Only the rows that carry a sketch are deserialized This family's input is always a `BYTES` column of serialized sketches. The old code deserialized every row up front with `deserializeSketches(values, length)` and then unioned them, so a null row was heapified from the column default before it could be skipped. It is now a per-row `deserializeSketch`, called only inside the non-null ranges. The base is re-parented from `BaseSingleInputAggregationFunction` onto `NullableSingleInputAggregationFunction`, which extends it, so this is a compatible swap that supplies both the option and `forEachNotNull`. ### The accumulator is created inside the range `getAccumulator` eagerly created *and stored* an accumulator in the result holder before the aggregation loop. Left that way, a block of nulls would store an empty accumulator and the "nothing was aggregated" signal would never reach `extractFinalResult`. It is now created inside the non-null range, so an untouched holder stays untouched. ### Each function renders its own identity when the option is off The four functions do not share an empty answer, so each preserves its own: | function | disabled-mode value | why | |---|---|---| | `DISTINCTCOUNTRAWINTEGERSUMTUPLESKETCH` | base64 of an empty sketch | built through a new `emptyAccumulator()` where the value is rendered | | `DISTINCTCOUNTTUPLESKETCH` | `0` | an empty sketch estimates to zero | | `SUMVALUESINTEGERSUMTUPLESKETCH` | `0` | an empty sketch sums to `0 / theta` | | `AVGVALUEINTEGERSUMTUPLESKETCH` | `NULL` | already `NULL` in both modes via the retained-entries check | The raw variant is the reason the identity is constructed at the render point rather than substituted during extraction: substituting upstream is what destroys the signal, and returning `null` there would silently turn a rendered value into `NULL`. ### Tests `AggregationFunctionNullContractTest` **cannot reach these functions**. Its synthetic `BYTES` block supplies empty byte arrays, which are not deserializable sketches, so all four are pinned in its skip list. The suite passes unchanged here, and that is not evidence about this change. `IntegerTupleSketchNullHandlingTest` is therefore the only coverage, and says so in its Javadoc. It builds real serialized sketches and covers null rows being skipped, an all-null input producing no intermediate result, and the per-function answers in both modes. The untouched-holder behaviour is mutation-tested: hoisting the accumulator creation back out of the range fails `testEveryRowNullYieldsNoIntermediateResult`. -- 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]
