twalthr commented on code in PR #26601:
URL: https://github.com/apache/flink/pull/26601#discussion_r2108831389


##########
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/operations/SinkModifyOperation.java:
##########
@@ -98,7 +100,7 @@ public SinkModifyOperation(
             boolean overwrite,
             Map<String, String> dynamicOptions,
             ModifyType modifyType) {
-        this.contextResolvedTable = contextResolvedTable;
+        this.contextResolvedTable = withDynamicOptions(contextResolvedTable, 
dynamicOptions);

Review Comment:
   It seems we are mixing the `api` module with the `planner` module here. 
Ideally, all the logic should go into the planner. Can we perform the merging 
of options there?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/api/CompiledPlanITCase.java:
##########
@@ -101,6 +101,19 @@ void testSourceTableWithHints() {
                 .isEqualTo(expected);
     }
 
+    @Test
+    void testSinkTableWithHints() {
+        CompiledPlan compiledPlan =
+                tableEnv.compilePlanSql(
+                        "INSERT INTO MySink "
+                                + "/*+ OPTIONS('sink.parallelism'='2', 
'sink-insert-only'='false') */ "
+                                + "SELECT * FROM MyTable");
+
+        String expected = 
TableTestUtil.readFromResource("/jsonplan/testSinkTableWithHints.out");

Review Comment:
   maybe rename the file for the source above to be symmetric?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to