Copilot commented on code in PR #19017:
URL: https://github.com/apache/pinot/pull/19017#discussion_r3619789291
##########
pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/PercentileTDigestAccumulator.java:
##########
@@ -476,19 +477,42 @@ public double compression() {
return _compression;
}
+ /// Returns the length of the [#serialize()] bytes rather than the
materialized [MergingDigest] byte size, so
+ /// generic `TDigest` serializers ([#byteSize()] followed by
[#asBytes(ByteBuffer)]) emit the capacity-preserving
+ /// encoding. Re-encoding through a materialized [MergingDigest] can produce
a verbose digest with more centroids
+ /// than a freshly allocated [MergingDigest] of the same compression can
hold, which readers reject with
+ /// [ArrayIndexOutOfBoundsException]. The length is computed without
materializing the bytes by mirroring the
+ /// [#serialize()] branches; the mutations here (flush, capacity
normalization) are idempotent, so the following
+ /// [#asBytes(ByteBuffer)] call writes exactly this many bytes.
@Override
public int byteSize() {
- return toTDigest().byteSize();
+ if (_pendingSerializedTDigest != null) {
+ return _pendingSerializedTDigest.length;
+ }
+ flush();
+ if (requiresCapacityPreservingEncoding()) {
+ return SMALL_HEADER_SIZE + SMALL_CENTROID_SIZE * _numCentroids;
+ }
Review Comment:
`byteSize()` can report a SMALL (capacity-preserving) size even when
`serialize()` would throw for `_numCentroids > Short.MAX_VALUE` in
`toCapacityPreservingBytes()`. That makes `TDIGEST_SER_DE.serialize()`
potentially allocate a large byte[] and then fail in `asBytes()`, and it breaks
the expectation that `byteSize()` reflects what `asBytes()` will be able to
write. Consider mirroring the `toCapacityPreservingBytes()` guard here (or
falling back to verbose/recompressed encoding).
--
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]
