azagrebin 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_r219130240
##########
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:
Catching exceptions will also produce false which will also result in
returning false. But I think they should be rather moved out because if
serializer can serialize null then it must also properly deserialize and copy
it. This can be a sanity check after try and before returning true. What do you
think?
----------------------------------------------------------------
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
