zentol commented on code in PR #20555:
URL: https://github.com/apache/flink/pull/20555#discussion_r944337888


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/ExecNodeBase.java:
##########
@@ -49,6 +49,13 @@
 @JsonIgnoreProperties(ignoreUnknown = true)
 public abstract class ExecNodeBase<T> implements ExecNode<T> {
 
+    /**
+     * The default value is chosen for the JSON deserialization case. Other 
cases must set this flag
+     * accordingly via {@link #setCompiled(boolean)}. It is not exposed via a 
constructor arg to
+     * avoid complex constructor overloading for all {@link ExecNode}s.
+     */
+    private transient boolean isCompiled = true;

Review Comment:
   Would it be useful to not have a general default and force all manual 
constructions to set this, to make sure this isn't accidentally missed?
   
   For the Json deserialization case you could try adding an `StdConverter` 
with `@JsonDeserialize` that sets this to `true` post deserialization.
   
   ```
   @JsonDeserialize(converter = ExecNodeBase.IsCompiledFlagDefaultSetter.class)
   public abstract class ExecNodeBase<T> implements ExecNode<T> {
   
       public static class IsCompiledFlagDefaultSetter extends 
StdConverter<ExecNodeBase<?>, ExecNodeBase<?>> {
           @Override
           public ExecNodeBase<?> convert(ExecNodeBase<?> execNodeBase) {
               execNodeBase.setCompiled(true);
               return execNodeBase;
           }
       }
   ```



-- 
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

Reply via email to