You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "jnh5y (via GitHub)" <gi...@apache.org> on 2024/08/07 21:46:36 UTC

[PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

jnh5y opened a new pull request, #25171:
URL: https://github.com/apache/flink/pull/25171

   ## What is the purpose of the change
   
   * Adds Compiled Plan annotations to BatchExecMatch.
   * Tests the new annotations with the existing TestPrograms.
   
   ## Verifying this change
   
   This change adds a BatchRestoreTest to cover the new annotations and show that the batch compiled plan can be restored and executed correctly.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (yes)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (not applicable)


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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1708923277


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+// @Disabled

Review Comment:
   do we still need commented `@Disabled` here?



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1710993068


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+public class MatchRecognizeBatchRestoreTest extends BatchRestoreTestBase {
+
+    public MatchRecognizeBatchRestoreTest() {
+        super(BatchExecMatch.class);
+    }
+
+    @Override
+    public List<TableTestProgram> programs() {
+        return Arrays.asList(
+                MatchRecognizeTestPrograms.MATCH_SIMPLE,

Review Comment:
   Why are tests like `MATCH_ORDER_BY_EVENT_TIME` not part of this list? All tests currently only test processing-time but no test tests event-time.



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys merged PR #25171:
URL: https://github.com/apache/flink/pull/25171


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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1814803784


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/common/CommonExecMatch.java:
##########
@@ -86,7 +89,8 @@ public abstract class CommonExecMatch extends ExecNodeBase<RowData>
 
     public static final String MATCH_TRANSFORMATION = "match";
 
-    private final MatchSpec matchSpec;
+    @JsonProperty(FIELD_NAME_MATCH_SPEC)

Review Comment:
   This introduces circular dependency. You use a constant from a child class in the parent class.



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1814809608


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/common/MatchRecognizeTestPrograms.java:
##########
@@ -121,6 +122,27 @@ public class MatchRecognizeTestPrograms {
                     .build();
 
     static final Row[] BEFORE_DATA = {
+        Row.of("2020-10-10 00:00:01", 9, 1),
+        Row.of("2020-10-10 00:00:01", 8, 2),
+        Row.of("2020-10-10 00:00:01", 10, 3),
+        Row.of("2020-10-10 00:00:04", 7, 4),
+        Row.of("2020-10-10 00:00:06", 5, 6),
+        Row.of("2020-10-10 00:00:07", 8, 5),
+        Row.of("2020-10-10 00:00:12", 3, 7),
+        Row.of("2020-10-10 00:00:16", 4, 9),
+        Row.of("2020-10-10 00:00:32", 7, 10),
+        Row.of("2020-10-10 00:00:33", 9, 12),
+        Row.of("2020-10-10 00:00:34", 5, 11)
+    };
+
+    static final Row[] AFTER_DATA = {
+        Row.of("2020-10-10 00:00:41", 3, 13),
+        Row.of("2020-10-10 00:00:42", 11, 16),
+        Row.of("2020-10-10 00:00:43", 12, 15),
+        Row.of("2020-10-10 00:00:44", 13, 14)
+    };
+
+    static final Row[] BEFORE_DATA_WITH_OUT_OF_ORDER_DATA = {

Review Comment:
   why default scope?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/common/MatchRecognizeTestPrograms.java:
##########
@@ -181,6 +203,47 @@ public class MatchRecognizeTestPrograms {
                     .runSql(getEventTimeSql("ORDER BY rowtime, sequence_num"))
                     .build();
 
+    static final SourceTestStep SOURCE_WITH_OUT_OF_ORDER_DATA =

Review Comment:
   Why is this in the default scope? Can't it be `private` if `public` is not necessary?



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/common/CommonExecMatch.java:
##########
@@ -86,7 +89,8 @@ public abstract class CommonExecMatch extends ExecNodeBase<RowData>
 
     public static final String MATCH_TRANSFORMATION = "match";
 
-    private final MatchSpec matchSpec;
+    @JsonProperty(FIELD_NAME_MATCH_SPEC)

Review Comment:
   Could you please also deduplicate the field from `StreamExecMatch` since you're touching this anyhow. I don't think it's necessary to be a separate field in `StreamExecMatch`.



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/common/MatchRecognizeTestPrograms.java:
##########
@@ -121,6 +122,27 @@ public class MatchRecognizeTestPrograms {
                     .build();
 
     static final Row[] BEFORE_DATA = {
+        Row.of("2020-10-10 00:00:01", 9, 1),
+        Row.of("2020-10-10 00:00:01", 8, 2),
+        Row.of("2020-10-10 00:00:01", 10, 3),
+        Row.of("2020-10-10 00:00:04", 7, 4),
+        Row.of("2020-10-10 00:00:06", 5, 6),
+        Row.of("2020-10-10 00:00:07", 8, 5),
+        Row.of("2020-10-10 00:00:12", 3, 7),
+        Row.of("2020-10-10 00:00:16", 4, 9),
+        Row.of("2020-10-10 00:00:32", 7, 10),
+        Row.of("2020-10-10 00:00:33", 9, 12),
+        Row.of("2020-10-10 00:00:34", 5, 11)
+    };
+
+    static final Row[] AFTER_DATA = {

Review Comment:
   why default scope?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/common/MatchRecognizeTestPrograms.java:
##########
@@ -136,7 +158,7 @@ public class MatchRecognizeTestPrograms {
         Row.of("2020-10-10 00:00:34", 5, 11)
     };
 
-    static final Row[] AFTER_DATA = {
+    static final Row[] AFTER_DATA_WITH_OUT_OF_ORDER_DATA = {

Review Comment:
   why default scope?



##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/common/CommonExecMatch.java:
##########
@@ -86,7 +89,8 @@ public abstract class CommonExecMatch extends ExecNodeBase<RowData>
 
     public static final String MATCH_TRANSFORMATION = "match";
 
-    private final MatchSpec matchSpec;
+    @JsonProperty(FIELD_NAME_MATCH_SPEC)

Review Comment:
   This introduces circular dependency. You use a constant from a child class that you use in the parent class.



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1711482347


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+public class MatchRecognizeBatchRestoreTest extends BatchRestoreTestBase {
+
+    public MatchRecognizeBatchRestoreTest() {
+        super(BatchExecMatch.class);
+    }
+
+    @Override
+    public List<TableTestProgram> programs() {
+        return Arrays.asList(
+                MatchRecognizeTestPrograms.MATCH_SIMPLE,

Review Comment:
   The ordering for the existing event time tests use data that has out-of-order elements: https://github.com/apache/flink/blob/master/flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/stream/MatchRecognizeTestPrograms.java#L132-L133
   
   That means that the result sets are going to be different in batch mode.  
   
   I see two options:
   1. Provide separate results in TableTestPrograms when batch would return different results.
   2. Add additional test programs.
   
   I'd prefer 1 since this would also handle the situation where changelogs need to be materialized.



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1711596673


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+public class MatchRecognizeBatchRestoreTest extends BatchRestoreTestBase {
+
+    public MatchRecognizeBatchRestoreTest() {
+        super(BatchExecMatch.class);
+    }
+
+    @Override
+    public List<TableTestProgram> programs() {
+        return Arrays.asList(
+                MatchRecognizeTestPrograms.MATCH_SIMPLE,

Review Comment:
   From a chat with Timo, we decided that `common` TableTestPrograms should not contain late data so that streaming and batch execution modes return the same thing.  



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1711600842


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+public class MatchRecognizeBatchRestoreTest extends BatchRestoreTestBase {
+
+    public MatchRecognizeBatchRestoreTest() {
+        super(BatchExecMatch.class);
+    }
+
+    @Override
+    public List<TableTestProgram> programs() {
+        return Arrays.asList(
+                MatchRecognizeTestPrograms.MATCH_SIMPLE,

Review Comment:
   Plan:  Rename the existing streaming test to have "with-late-data" or similar in the name.
   
   Create new event time test cases without late data for use with Match Recognize.



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on PR #25171:
URL: https://github.com/apache/flink/pull/25171#issuecomment-2455044305

   > one final comment on an import, otherwise +1
   
   Apologies for that.  While fixing that, I updated a few things relative to https://github.com/apache/flink/pull/24699/files.
   
   Hopefully CI will start soon; if not, I'll try and remember to poke it here in a bit.


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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25171:
URL: https://github.com/apache/flink/pull/25171#issuecomment-2274408966

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "5689c2b9d1352c5bd9655f1df817bffc6d4b4876",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "5689c2b9d1352c5bd9655f1df817bffc6d4b4876",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * 5689c2b9d1352c5bd9655f1df817bffc6d4b4876 UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "jnh5y (via GitHub)" <gi...@apache.org>.
jnh5y commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1709560780


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/plan/nodes/exec/batch/MatchRecognizeBatchRestoreTest.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.table.planner.plan.nodes.exec.batch;
+
+import org.apache.flink.table.planner.plan.nodes.exec.common.MatchRecognizeTestPrograms;
+import org.apache.flink.table.planner.plan.nodes.exec.testutils.BatchRestoreTestBase;
+import org.apache.flink.table.test.program.TableTestProgram;
+
+import java.util.Arrays;
+import java.util.List;
+
+/** Batch Compiled Plan tests for {@link BatchExecMatch}. */
+// @Disabled

Review Comment:
   Good catch.  We do not.  Removing it.  



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


Re: [PR] [FLINK-36002] Add CompiledPlan annotations to BatchExecMatch [flink]

Posted by "dawidwys (via GitHub)" <gi...@apache.org>.
dawidwys commented on code in PR #25171:
URL: https://github.com/apache/flink/pull/25171#discussion_r1827346770


##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/common/CommonExecMatch.java:
##########
@@ -77,6 +79,7 @@
 import java.util.List;
 import java.util.Optional;
 
+import static org.apache.flink.table.planner.plan.nodes.exec.batch.BatchExecMatch.FIELD_NAME_MATCH_SPEC;

Review Comment:
   invalid import? We shouldn't have this constant anymore.



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