[GitHub] [flink] KurtYoung commented on a change in pull request #11276: [FLINK-16029][table-planner-blink] Remove register source and sink in test cases of blink planner

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

KurtYoung commented on a change in pull request #11276:
URL: https://github.com/apache/flink/pull/11276#discussion_r421350537



##########
File path: flink-table/flink-table-api-java-bridge/src/main/java/org/apache/flink/table/sinks/CsvTableSinkFactoryBase.java
##########
@@ -77,7 +81,12 @@
 		properties.add(FORMAT_FIELDS + ".#." + DescriptorProperties.TABLE_SCHEMA_NAME);
 		properties.add(FormatDescriptorValidator.FORMAT_DERIVE_SCHEMA);
 		properties.add(FORMAT_FIELD_DELIMITER);
+		properties.add(FORMAT_WRITE_MODE);
+		properties.add(FORMAT_NUM_FILES);

Review comment:
       duplicated

##########
File path: flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/api/TableEnvironmentITCase.scala
##########
@@ -72,13 +69,20 @@ class TableEnvironmentITCase(tableEnvName: String, isStreaming: Boolean) extends
           StreamExecutionEnvironment.getExecutionEnvironment, settings)
       case _ => throw new UnsupportedOperationException("unsupported tableEnvName: " + tableEnvName)
     }
-    tEnv.registerTableSource("MyTable", getPersonCsvTableSource)
+    TestTableSourceSinks.createPersonCsvTemporaryTable(tEnv, "MyTable")
   }
 
   @Test
   def testExecuteTwiceUsingSameTableEnv(): Unit = {
-    val sink1Path = registerCsvTableSink(tEnv, Array("first"), Array(STRING), "MySink1")
-    val sink2Path = registerCsvTableSink(tEnv, Array("last"), Array(STRING), "MySink2")
+    val sink1Path = _tempFolder.newFile().getAbsolutePath
+    TestTableSourceSinks.createCsvTemporarySinkTable(

Review comment:
       We could let `createCsvTemporarySinkTable` method create the temporary path and then return it

##########
File path: flink-table/flink-table-planner-blink/src/test/scala/org/apache/flink/table/planner/runtime/batch/sql/TimestampITCase.scala
##########
@@ -41,12 +40,12 @@ class TimestampITCase extends BatchTestBase {
       Array(
         DataTypes.INT(),
         DataTypes.BIGINT(),
-        DataTypes.TIMESTAMP(9).bridgedTo(classOf[LocalDateTime]),
-        DataTypes.TIMESTAMP(9).bridgedTo(classOf[Timestamp]),
-        DataTypes.TIMESTAMP(3).bridgedTo(classOf[LocalDateTime]),
-        DataTypes.TIMESTAMP(3).bridgedTo(classOf[Timestamp]),
-        DataTypes.TIMESTAMP_WITH_LOCAL_TIME_ZONE(9).bridgedTo(classOf[Instant]),
-        DataTypes.TIMESTAMP_WITH_LOCAL_TIME_ZONE(9).bridgedTo(classOf[Instant]))
+        DataTypes.TIMESTAMP(9),
+        DataTypes.TIMESTAMP(9),
+        DataTypes.TIMESTAMP(3),
+        DataTypes.TIMESTAMP(3),
+        DataTypes.TIMESTAMP_WITH_LOCAL_TIME_ZONE(9),
+        DataTypes.TIMESTAMP_WITH_LOCAL_TIME_ZONE(9))

Review comment:
       Why change these?




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org