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. However, as I
understand it, 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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
