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


##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/calcite/FlinkCalciteSqlValidatorTest.java:
##########
@@ -63,6 +64,55 @@ void testInsertInto2() {
                 .hasMessageContaining(" Number of columns must match number of 
query columns");
     }
 
+    @Test
+    void testInsertInto3() {
+        assertThatThrownBy(
+                        () ->
+                                plannerMocks
+                                        .getParser()
+                                        .parse("INSERT INTO t2 (a,b) VALUES 
(1,2), (3)"))
+                .isInstanceOf(ValidationException.class)
+                .hasMessageContaining(" Number of columns must match number of 
query columns");
+    }
+
+    @Test
+    void testInsertInto4() {
+        assertThatThrownBy(
+                        () ->
+                                plannerMocks
+                                        .getParser()
+                                        .parse("INSERT INTO t2 (a,b) VALUES 
(1), (2,3)"))
+                .isInstanceOf(ValidationException.class)
+                .hasMessageContaining(" Number of columns must match number of 
query columns");
+    }
+
+    @Test
+    void testInsertInto5() {
+        assertThatThrownBy(
+                        () ->
+                                plannerMocks
+                                        .getParser()
+                                        .parse("INSERT INTO t2 (a,b) VALUES 
(1,2), (3,4,5)"))
+                .isInstanceOf(ValidationException.class)
+                .hasMessageContaining(" Number of columns must match number of 
query columns");
+    }
+
+    @Test
+    void testInsertInto6() {
+        assertDoesNotThrow(
+                () -> {
+                    plannerMocks.getParser().parse("INSERT INTO t2 (a,b) 
VALUES (1,2), (3,4)");
+                });
+    }
+
+    @Test
+    void testInsertInto7() {
+        assertDoesNotThrow(
+                () -> {
+                    plannerMocks.getParser().parse("INSERT INTO t2 (a,b) 
Select 1, 2");

Review Comment:
   Could you also add a test for `INSERT INTO t2 (a) VALUES (1), (3)`?



##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/calcite/FlinkCalciteSqlValidatorTest.java:
##########
@@ -63,6 +64,55 @@ void testInsertInto2() {
                 .hasMessageContaining(" Number of columns must match number of 
query columns");
     }
 
+    @Test
+    void testInsertInto3() {

Review Comment:
   Tests should have a descriptive name. E.g. 
`testInsertIntoValuesColumnsMismatch`



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