cyb70289 commented on code in PR #52276:
URL: https://github.com/apache/arrow/pull/52276#discussion_r4269301692
##########
cpp/src/arrow/util/tdigest.cc:
##########
@@ -168,8 +168,8 @@ class TDigest::TDigestImpl {
if (total_weight != total_weight_) {
return Status::Invalid("tdigest total weight mismatch");
}
- // check if buffer expanded
- if (tdigests_[0].capacity() > delta_ || tdigests_[1].capacity() > delta_) {
+ // check if buffer size exceeded delta
Review Comment:
The point is at [line
104](https://github.com/apache/arrow/blob/28d9769101d5f136210b91fb882197d81cb780c6/cpp/src/arrow/util/tdigest.cc#L104)
`td.push_back(centroid); // should never exceed capacity and trigger
reallocation`
A correctly implemented algorithm shall not write above pre-allocated
capacity. I don't add check code there for performance, and write above
capacity is not a disaster (the algorithm still works).
tdigests_[n] may be cleared during usage, size() doesn't guarantee the
vector was not expanded.
I think we can record the actual buffer size at construction time, and check
against that.
Note this is a loose check. As this PR points out, real capacity is often
bigger than required.
--
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]
