[GitHub] [flink] dawidwys commented on a change in pull request #10059: [FLINK-14543][table] Support partition for temporary table

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

dawidwys commented on a change in pull request #10059: [FLINK-14543][table] Support partition for temporary table
URL: https://github.com/apache/flink/pull/10059#discussion_r347253491
 
 

 ##########
 File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/descriptors/ConnectTableDescriptor.java
 ##########
 @@ -136,14 +137,24 @@ public void createTemporaryTable(String path) {
 					" use registerTableSource/registerTableSink/registerTableSourceAndSink.");
 		}
 
+		Map<String, String> properties = new HashMap<>(toProperties());
+
+		// handle schema
 		Map<String, String> schemaProperties = schemaDescriptor.toProperties();
 		TableSchema tableSchema = getTableSchema(schemaProperties);
-
-		Map<String, String> properties = new HashMap<>(toProperties());
 		schemaProperties.keySet().forEach(properties::remove);
 
+		// handle partition keys
+		DescriptorProperties descriptor = new DescriptorProperties();
 
 Review comment:
   Why do you need this code in the first place?
   
   As far as I can tell there is no way to pass partitionKeys to the `ConnectTableDescriptor`. They will never be in the properties. We need a method for doing it. Once you add one you don't need to put the keys into properties. You may just pass them explicitly to the `CatalogTable`.
   
   The reason why its different for the `TableSchema` is backwards compatibility. The schema was part of properties before and was passed as part of `SchemaDescriptor`. This is why we had to extract and remove it from the properties.
   
   Moreover I am not sure if it makes sense to have the `CatalogTableBuilder`. It duplicates the functionality of `ConnectTableDescriptor` and actually was never part of the public API. This is a different case though.

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


With regards,
Apache Git Services