tzulitai commented on a change in pull request #12263:
URL: https://github.com/apache/flink/pull/12263#discussion_r427970574
##########
File path:
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -367,21 +367,21 @@ public int getVersion() {
/**
* A {@link TypeSerializerSnapshot} for RowSerializer.
*/
- // TODO not fully functional yet due to FLINK-17520
public static final class RowSerializerSnapshot extends
CompositeTypeSerializerSnapshot<Row, RowSerializer> {
private static final int VERSION = 3;
- private static final int VERSION_WITHOUT_ROW_KIND = 2;
+ private static final int LAST_VERSION_WITHOUT_ROW_KIND = 2;
- private boolean legacyModeEnabled = false;
+ private int readVersion = VERSION;
public RowSerializerSnapshot() {
super(RowSerializer.class);
}
RowSerializerSnapshot(RowSerializer serializerInstance) {
super(serializerInstance);
+ this.readVersion = serializerInstance.legacyModeEnabled
? LAST_VERSION_WITHOUT_ROW_KIND : VERSION;
Review comment:
I don't think this line is needed, unless I'm missing something in the
tests.
The read version should only ever be changed if this snapshot was created by
restoring from a snapshot.
In this case, this constructor is only ever used to create a new snapshot
when checkpointing occurs - the read version should be the default value
(`VALUE`).
##########
File path:
flink-core/src/main/java/org/apache/flink/api/java/typeutils/runtime/RowSerializer.java
##########
@@ -60,7 +60,7 @@
public static final int ROW_KIND_OFFSET = 2;
- private static final long serialVersionUID = 2L;
+ private static final long serialVersionUID = 1L; // legacy, don't touch
Review comment:
nit: maybe add a comment that this can only be touched after support for
1.9 savepoints is ditched.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
