masteryhx commented on code in PR #21635:
URL: https://github.com/apache/flink/pull/21635#discussion_r1085568912
##########
flink-core/src/main/java/org/apache/flink/api/common/typeutils/CompositeTypeSerializerSnapshot.java:
##########
@@ -290,26 +332,26 @@ protected void readOuterSnapshot(
throws IOException {}
/**
- * Checks whether the outer snapshot is compatible with a given new
serializer.
+ * Checks the schema compatibility of the given old serializer snapshot
based on the outer
+ * snapshot.
*
- * <p>The base implementation of this method just returns {@code true},
i.e. it assumes that the
- * outer serializer only has nested serializers and no extra information,
and therefore the
- * result of the check must always be true. Otherwise, if the outer
serializer contains some
+ * <p>The base implementation of this method assumes that the outer
serializer only has nested
+ * serializers and no extra information, and therefore the result of the
check is {@link
+ * OuterSchemaCompatibility#COMPATIBLE_AS_IS}. Otherwise, if the outer
serializer contains some
* extra information that has been persisted as part of the serializer
snapshot, this must be
* overridden. Note that this method and the corresponding methods {@link
* #writeOuterSnapshot(DataOutputView)}, {@link #readOuterSnapshot(int,
DataInputView,
* ClassLoader)} needs to be implemented.
*
- * @param newSerializer the new serializer, which contains the new outer
information to check
- * against.
- * @return a flag indicating whether or not the new serializer's outer
information is compatible
- * with the one written in this snapshot.
- * @deprecated this method is deprecated, and will be removed in the
future. Please implement
- * {@link #resolveOuterSchemaCompatibility(TypeSerializer)} instead.
+ * @param oldSerializerSnapshot the old serializer snapshot, which
contains the old outer
+ * information to check against.
+ * @return a {@link OuterSchemaCompatibility} indicating whether or the
new serializer's outer
+ * information is compatible, requires migration, or incompatible with
the one written in
+ * this snapshot.
*/
- @Deprecated
- protected boolean isOuterSnapshotCompatible(S newSerializer) {
Review Comment:
Actually, `isOuterSnapshotCompatible` will not be used in current
implementation.
Currently, the old method called `OuterSchemaCompatibility
resolveOuterSchemaCompatibility(S newSerializer)` behaves like the old method
in TypeSerializer -- it calls the new method as its default implementation.
Users could make its logic work without modifying any codes.
Also if they need to migrate, they could implement the new method called
`OuterSchemaCompatibility
resolveOuterSchemaCompatibility(TypeSerializerSnapshot<T>
oldSerializerSnapshot)` and remove the old method.
In this logic, I think maybe you're right. Users will find its original
`isOuterSnapshotCompatible` could not work which will confused them.
Maybe we still need to remove it and add descripation in the release note,
WDYT?
--
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
