xiangfu0 opened a new pull request, #19017: URL: https://github.com/apache/pinot/pull/19017
## Summary Follow-up bugfix to #18996, split out of #19015 per review feedback. `PercentileTDigestAccumulator` can legitimately hold capacity-preserving state: a t-digest with more centroids than a freshly allocated `MergingDigest` of the same compression can hold (e.g. a SMALL-encoded digest with explicit main/buffer capacities stored in a pre-aggregated `BYTES` column). #18996 handled this in the accumulator's own `serialize()`, but two paths were missed: 1. **`percentileRawTDigest` final results**: `SerializedTDigest.toString()` serializes the accumulator through the generic `TDIGEST_SER_DE` (`byteSize()` + `asBytes()`), which re-encoded the digest as a verbose payload with more centroids than a plain `MergingDigest.fromBytes` reader allocates. Any downstream reader of the emitted hex — including t-digest itself — fails with `ArrayIndexOutOfBoundsException: Index 50 out of bounds for length 50`. 2. **`percentileSmartTDigest`** still built and merged plain `MergingDigest`s and deserialized intermediates with `MergingDigest.fromBytes`, so it had the same re-encoding problem on its intermediate-result path and missed the accumulator routing entirely. ## Fix - `PercentileTDigestAccumulator` now overrides `byteSize()`/`asBytes(ByteBuffer)` to emit its `serialize()` bytes, so every generic `TDigest` serializer (`ObjectSerDeUtils.TDIGEST_SER_DE`, `CustomSerDeUtils.TDIGEST_SER_DE`) produces capacity-preserving, plain-reader-compatible bytes polymorphically — fixing the `percentileRawTDigest` final-result path and any other call site handed an accumulator (including the generic `ObjectSerDeUtils.serialize(Object)` dispatch), with no per-call-site special casing. `byteSize()` computes the length analytically without materializing the bytes, so the serde's two-call pattern still encodes only once. - `PercentileSmartTDigestAggregationFunction` routes value-list conversion, merging, and intermediate-result deserialization through `PercentileTDigestAccumulator`, matching the other percentile t-digest paths from #18996. - The now-redundant accumulator special case in `PercentileTDigestAggregationFunction.serializeIntermediateResult` is simplified to the plain polymorphic serde call. ## Behavior notes - All existing quantile results are unchanged (existing `PercentileSmartTDigestAggregationFunctionTest` behavior suites pass unmodified). - When the accumulator carries a SMALL-encoded input digest through verbatim, the emitted final payload for `percentileRawTDigest` stays SMALL-encoded instead of being re-encoded verbose. `MergingDigest.fromBytes` reads both encodings; only a hand-written parser assuming verbose layout would notice. - Rolling upgrade safe in both directions: emitted intermediates are verbose within reader capacity in the common case, and SMALL with explicit capacities (readable by plain `MergingDigest.fromBytes`) in the capacity-preserving case. ## Testing - New `PercentileRawTDigestAggregationFunctionTest`: reproduces the `ArrayIndexOutOfBoundsException` on the final-result path (fails on current master, passes with the fix), plus merged-state and regular-digest round-trips. - New tests in `PercentileSmartTDigestAggregationFunctionTest`: intermediate-result round-trip through the accumulator for capacity-preserving state (fails on current master), materialized capacity-preserving serialization through the generic serde (small encoding + `byteSize()` length consistency), merged-state serialization, and accumulator + raw value-list merges in both argument orders. - 547 `pinot-core` percentile/t-digest/serde tests green; `spotless`, `checkstyle`, `license` checks pass. -- 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]
