xiangfu0 commented on code in PR #19017:
URL: https://github.com/apache/pinot/pull/19017#discussion_r3628306317
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileSmartTDigestAggregationFunction.java:
##########
@@ -309,6 +324,9 @@ public SerializedIntermediateResult
serializeIntermediateResult(Object o) {
@Override
public Object deserializeIntermediateResult(CustomObject customObject) {
+ if (customObject.getType() ==
ObjectSerDeUtils.ObjectType.TDigest.getValue()) {
Review Comment:
I kept accumulator reconstruction function-specific. Changing the shared
`TDIGEST_SER_DE` deserializer would make every TDigest consumer receive
Pinot\u0027s aggregation accumulator instead of the library `MergingDigest`.
Adding a new object type/serde would change the intermediate type ID and
complicate rolling upgrades. Both percentile TDigest functions now reconstruct
the accumulator from the existing TDigest bytes in
`deserializeIntermediateResult`, so the wire format and generic serde contract
stay unchanged.
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileSmartTDigestAggregationFunction.java:
##########
@@ -265,30 +266,38 @@ public Object extractGroupByResult(GroupByResultHolder
groupByResultHolder, int
}
@Override
- public Object merge(Object intermediateResult1, Object intermediateResult2) {
+ @Nullable
+ public Object merge(@Nullable Object intermediateResult1, @Nullable Object
intermediateResult2) {
+ if (intermediateResult1 == null) {
+ return intermediateResult2;
+ }
+ if (intermediateResult2 == null) {
+ return intermediateResult1;
+ }
if (intermediateResult1 instanceof TDigest) {
Review Comment:
Applied in 6071b0f005: both merge branches now check
`PercentileTDigestAccumulator` directly before passing the value to
`mergeIntoAccumulator`.
--
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]
