RocMarshal commented on code in PR #26486:
URL: https://github.com/apache/flink/pull/26486#discussion_r2057170506
##########
flink-core/src/main/java/org/apache/flink/api/common/accumulators/Histogram.java:
##########
@@ -53,12 +53,7 @@ public TreeMap<Integer, Integer> getLocalValue() {
public void merge(Accumulator<Integer, TreeMap<Integer, Integer>> other) {
// Merge the values into this map
for (Map.Entry<Integer, Integer> entryFromOther :
other.getLocalValue().entrySet()) {
- Integer ownValue = this.treeMap.get(entryFromOther.getKey());
- if (ownValue == null) {
- this.treeMap.put(entryFromOther.getKey(),
entryFromOther.getValue());
- } else {
- this.treeMap.put(entryFromOther.getKey(),
entryFromOther.getValue() + ownValue);
- }
+ this.treeMap.merge(entryFromOther.getKey(),
entryFromOther.getValue(), Integer::sum);
Review Comment:
@davidradl Thanks for the comment.
I looked into it and didn’t find any existing test cases. In my limited
read, this API is a built-in method in the Java JDK, so theoretically, it
shouldn’t have any issues. If we need to add a new test case to verify whether
the Histogram#merge method works correctly, I’d be happy to do that. :)
--
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]
