kbendick commented on a change in pull request #1972:
URL: https://github.com/apache/iceberg/pull/1972#discussion_r592027533
##########
File path: api/src/main/java/org/apache/iceberg/catalog/TableIdentifier.java
##########
@@ -28,7 +29,7 @@
/**
* Identifies a table in iceberg catalog.
*/
-public class TableIdentifier {
+public class TableIdentifier implements Serializable {
Review comment:
For some of these classes where they are implementing Serializable,
should we consider adding a `serialVersionUID`?
I have many users (specifically for Flink) that encounter subtle issues when
some Serializable class does not specify `serialVersionUID` and the compiler
changes the assigned seriallVersionUID. Very often, these users need to allow
some or all of their jobs existing state to be discarded when this happens.
I'm not sure if `TableIdentifier` gets placed into state anywhere ourselves,
but for classes that are part of the public api is there an argument against
including them?
Additionally, does anybody have any arguments for or against including
serial version uid on the interfaces which are also extending serializable? For
example, `DataFile` stands out to me in particular as things that could
potentially cause issue. I could imagine that something along the lines of
`Class<DataFile> clazz = DataFile.class` could potentially get serialized and
cause issues when the interface changes.
----------------------------------------------------------------
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
