weiqingy commented on PR #28880:
URL: https://github.com/apache/flink/pull/28880#issuecomment-5390491215
Thanks for the careful review, @gyfora!
Two considerations led me to the original placement. First, migration must
use the old snapshot persisted with the state. A snapshot → serializer →
snapshot round trip is not always lossless; for example, re-snapshotting a
restored POJO serializer can assign a placeholder name to a missing field and
describe a schema that was never written. Second, keeping `migrate(oldSnapshot,
value)` beside `resolveSchemaCompatibility(oldSnapshot)` keeps the migration
decision and action on the same class.
However, your responsibility-boundary point stands: value handling otherwise
belongs to `TypeSerializer`, alongside `serialize`, `deserialize`, and `copy`.
An alternative would be:
```java
public T migrate(TypeSerializerSnapshot<T> oldSerializerSnapshot, T value) {
return value;
}
```
The runtime would call `newSerializer.migrate(persistedOldSnapshot, value)`,
preserving the exact old schema while keeping value transformation on the
serializer.
The trade-off is that compatibility resolution and migration would live on
different classes, but cross-referencing them in their Javadocs should make
that contract explicit. On balance, I now mildly prefer this placement on
`TypeSerializer`.
Does this match what you had in mind? If so, I'll update this PR, the
stacked PRs, and the FLIP, and note the signature change on the FLIP-527 vote
thread.
--
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]
