StefanRRichter commented on a change in pull request #6707: [FLINK-10157]
[State TTL] Allow `null` user values in map state with TTL
URL: https://github.com/apache/flink/pull/6707#discussion_r219131023
##########
File path:
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/NullableSerializer.java
##########
@@ -56,33 +59,51 @@
public class NullableSerializer<T> extends TypeSerializer<T> {
private static final long serialVersionUID = 3335569358214720033L;
+ @Nonnull
private final TypeSerializer<T> originalSerializer;
- private NullableSerializer(TypeSerializer<T> originalSerializer) {
+ private NullableSerializer(@Nonnull TypeSerializer<T>
originalSerializer) {
Preconditions.checkNotNull(originalSerializer, "The original
serializer cannot be null");
this.originalSerializer = originalSerializer;
}
/**
- * This method tries to serialize null value with the {@code
originalSerializer}
+ * This method tries to serialize {@code null} value with the {@code
originalSerializer}
* and wraps it in case of {@link NullPointerException}, otherwise it
returns the {@code originalSerializer}.
+ *
+ * @param originalSerializer serializer to wrap and add {@code null}
support
+ * @return serializer which supports {@code null} values
*/
- public static <T> TypeSerializer<T>
wrapIfNullIsNotSupported(TypeSerializer<T> originalSerializer) {
+ public static <T> TypeSerializer<T> wrapIfNullIsNotSupported(@Nonnull
TypeSerializer<T> originalSerializer) {
return checkIfNullSupported(originalSerializer) ?
originalSerializer : wrap(originalSerializer);
}
- private static <T> boolean checkIfNullSupported(TypeSerializer<T>
originalSerializer) {
+ /**
+ * This method checks if {@code serializer} supports {@code null} value.
+ *
+ * @param serializer serializer to check
+ */
+ public static <T> boolean checkIfNullSupported(@Nonnull
TypeSerializer<T> serializer) {
try {
- originalSerializer.serialize(null, new
DataOutputSerializer(1));
-
Preconditions.checkArgument(originalSerializer.copy(null) == null);
- } catch (NullPointerException | IOException e) {
+ int length = serializer.getLength() > 0 ?
serializer.getLength() : 1;
+ DataOutputSerializer dos = new
DataOutputSerializer(length);
+ serializer.serialize(null, dos);
+ DataInputDeserializer dis = new
DataInputDeserializer(dos.getSharedBuffer());
+ Preconditions.checkArgument(serializer.deserialize(dis)
== null);
Review comment:
My main point is, you use precondition that could produce an exception and
then immediately catch the exception and translate it to `false`. So my
question is, why use precondition and not just an `if` that affects the result.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
