xiangfu0 commented on code in PR #18996:
URL: https://github.com/apache/pinot/pull/18996#discussion_r3594928077
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileTDigestAggregationFunction.java:
##########
@@ -305,13 +300,16 @@ public ColumnDataType getIntermediateResultColumnType() {
@Override
public SerializedIntermediateResult serializeIntermediateResult(TDigest
tDigest) {
+ if (tDigest instanceof PercentileTDigestAccumulator) {
+ tDigest = ((PercentileTDigestAccumulator) tDigest).toTDigest();
Review Comment:
This conversion loses the small-encoding capacity for oversized compact
TDigest inputs. `toTDigest()` can preserve an oversized small-encoded digest,
but the generic `TDIGEST_SER_DE.serialize()` below calls `asBytes()` and
rewrites it as verbose bytes. A valid compact digest with more centroids than
`2 * ceil(compression) + 10` then becomes unreadable by the t-digest 3.2
decoder because verbose bytes no longer carry the larger main capacity, so
distributed StarTree percentile queries can fail during server-to-broker
intermediate reduction. Please preserve small encoding for these
capacity-preserving accumulators and add an intermediate-result round-trip test
for this case.
--
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]
