davidradl commented on code in PR #26486:
URL: https://github.com/apache/flink/pull/26486#discussion_r2060180993
##########
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:
Looking at the javadoc for merge if entryFromOther.getValue() is null, then
the entry is removed, is this possible?
Testing the cases where null is our own Value (because the record does not
exist) and null for the other value would be useful I think, as well as the
happy path.
--
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]
