[GitHub] [flink] wenlong88 commented on a diff in pull request #19179: [FLINK-26756][table-planner] Fix the deserialization error for match recognize

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

wenlong88 commented on code in PR #19179:
URL: https://github.com/apache/flink/pull/19179#discussion_r843729615


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/utils/JsonPlanTestBase.java:
##########
@@ -72,7 +73,9 @@ public void after() {
     protected TableResult compileSqlAndExecutePlan(String sql) {
         CompiledPlan compiledPlan = tableEnv.compilePlanSql(sql);
         checkTransformationUids(compiledPlan);
-        return compiledPlan.execute();
+        String jsonPlan = compiledPlan.asJsonString();

Review Comment:
   add some comment about the magic here is trying to validate the ser/de of plan json works well.



##########
flink-table/flink-table-planner/src/test/resources/org/apache/flink/table/planner/plan/nodes/exec/stream/MatchRecognizeJsonPlanTest_jsonplan/testMatch.out:
##########
@@ -131,10 +131,12 @@
         "kind" : "CALL",
         "syntax" : "BINARY",
         "internalName" : "$$1",
+        "sqlKind" : "PATTERN_CONCAT",

Review Comment:
   regarding such kind of operator, do we need version them? 
   on the other hand, I think it may be better to introduce a special format of InternalName, instead of a new optional field.



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