scwhittle commented on code in PR #40750:
URL: https://github.com/apache/beam/pull/40750#discussion_r4483341199
##########
runners/google-cloud-dataflow-java/worker/src/main/java/org/apache/beam/runners/dataflow/worker/windmill/state/WindmillMapViaMultimap.java:
##########
@@ -62,19 +62,26 @@ public void put(KeyT key, ValueT value) {
multimap.put(key, value);
}
+ @SuppressWarnings("nullness")
@Override
public ReadableState<ValueT> computeIfAbsent(
KeyT key, Function<? super KeyT, ? extends ValueT> mappingFunction) {
// Note that computeIfAbsent comments indicate that the read is lazy but
this matches the
- // existing eager
- // behavior of WindmillMap.
+ // existing eager behavior of WindmillMap.
Iterable<ValueT> existingValues = multimap.get(key).read();
- if (Iterables.isEmpty(existingValues)) {
+ Iterator<ValueT> iterator = existingValues.iterator();
+ if (!iterator.hasNext()) {
ValueT inserted = mappingFunction.apply(key);
multimap.put(key, inserted);
return ReadableStates.immediate(inserted);
} else {
- return
ReadableStates.immediate(Iterables.getOnlyElement(existingValues));
+ ValueT existing = Iterators.getOnlyElement(iterator);
+ if (existing == null) {
Review Comment:
ok maybe we should just follow the comments then, even if it seems confusing
that it doesn't match Map.computeIfAbsent. A fix we could consider throughout
is that if we are calling the computefn and it returns null should we treat
that as not adding the entry instead of adding null value.
--
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]
