[GitHub] [flink] matriv edited a comment on pull request #18479: [FLINK-25387] Introduce ExecNodeMetadata

[GitBox <gi...@apache.org>]

matriv edited a comment on pull request #18479:
URL: https://github.com/apache/flink/pull/18479#issuecomment-1021436854


   > I have one specific comment about the `context` field. Why have you opted for mixing together in the serialization the instance identifier and the type identifier (composed by name and version)? For me "type" identity and "instance" identity are very different concepts, and they definitely deserve different fields.
   
   Because, with the upgrade story, we can have an `@ExecNodeMetadata` annotation with the same name, on a subclass of a current ExecNode class, which does something new/different and defines a newer `version`. So we need the combination of `name` + `version` to uniquely identify the class when we lookup and rebuild the Java object graph from the JSON plan.
   
   ```
   @ExecNodeMetadata(name="myNode", version=1)
   public class MyNode {}
   
   @ExecNodeMetadata(name="myNode", version=2)
   public class MyNodeWithNewFunctionality extends MyNode {}
   ```


-- 
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-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org