XComp commented on code in PR #23490:
URL: https://github.com/apache/flink/pull/23490#discussion_r1383635290
##########
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/PojoSerializer.java:
##########
@@ -473,25 +519,41 @@ public T deserialize(T reuse, DataInputView source)
throws IOException {
}
}
- if ((flags & NO_SUBCLASS) != 0) {
+ if (isRecord) {
Review Comment:
Your suspicion/caution was correct - it's actually not that straight-forward
setting `reuse = null` due to the recursive call of
`deserialize(DataInputView)`/`deserialize(T, DataInputView)` on the fields. -.-
Anyway, I went through the code once more to figure out how we could reduce
the redundant code here a bit more:
```java
private void deserializeFields(
DataInputView source,
BiConsumerWithException<Integer, Object, IllegalAccessException>
fieldInitializer)
throws IOException {
deserializeFields(
source,
fieldInitializer,
fieldIdx -> fieldSerializers[fieldIdx].deserialize(source));
}
private void deserializeFields(
DataInputView source,
T reuse,
BiConsumerWithException<Integer, Object, IllegalAccessException>
fieldInitializer)
throws IOException {
deserializeFields(
source,
fieldInitializer,
fieldIdx -> {
final Object reuseFieldValue =
reuse == null ? null :
fields[fieldIdx].get(reuse);
if (reuseFieldValue != null) {
return
fieldSerializers[fieldIdx].deserialize(reuseFieldValue, source);
} else {
return
fieldSerializers[fieldIdx].deserialize(source);
}
});
}
private void deserializeFields(
DataInputView source,
BiConsumerWithException<Integer, Object, IllegalAccessException>
fieldInitializer,
FunctionWithException<Integer, Object, ? extends Exception>
valueSupplier)
throws IOException {
try {
for (int i = 0; i < numFields; i++) {
boolean isNull = source.readBoolean();
if (fields[i] != null && isNull) {
fieldInitializer.accept(i, null);
} else if (fields[i] != null && !isNull) {
fieldInitializer.accept(i, valueSupplier.apply(i));
} else if (fields[i] == null && !isNull) {
// read and dump a pre-existing field value
fieldSerializers[i].deserialize(source);
}
}
} catch (IllegalAccessException e) {
throw new RuntimeException(
"Error during POJO copy, this should not happen since we
check the fields before.",
e);
} catch (Throwable e) {
ExceptionUtils.rethrowIOException(e);
}
}
```
...just as a proposal. I just saw your comment that your not too excited
about using consumers. I see your point there and I'm not pushing for it. I
wanted to share my findings anyway.
--
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
