[GitHub] [flink] bowenli86 commented on a change in pull request #8458: [FLINK-12452][table][hive] alterTable() should ensure existing base table and the new one are of the same type

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

bowenli86 commented on a change in pull request #8458: [FLINK-12452][table][hive] alterTable() should ensure existing base table and the new one are of the same type
URL: https://github.com/apache/flink/pull/8458#discussion_r284488368
 
 

 ##########
 File path: flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/catalog/GenericInMemoryCatalog.java
 ##########
 @@ -209,11 +209,17 @@ public void alterTable(ObjectPath tablePath, CatalogBaseTable newTable, boolean
 		checkNotNull(tablePath);
 		checkNotNull(newTable);
 
-		// TODO: validate the new and old CatalogBaseTable must be of the same type. For example, this doesn't
-		//		allow alter a regular table to partitioned table, or alter a view to a table, and vice versa.
-		//		And also add unit tests.
-
 		if (tableExists(tablePath)) {
+			CatalogBaseTable oldTable = tables.get(tablePath);
+
+			// Theoretically, in-memory catalog can store any tables and views implementations
+			// thus, we need to check they are of the same class
 
 Review comment:
   there's a subtle difference between checks in in-memory catalog and HiveCatalogBase.
   
   HiveCatalogBase can only handle HiveCatalogTable/View (HiveCatalog) or GenericCatalogTable/View (GenericHiveMetastoreCatalog), and user input is already validated in validateCatalogBaseTable(). Thus, we only need to check if the input is CatalogTable or CatalogView.
   
   However, in-memory catalog can host any implementations of table and view, thus we need to check here that they are of the same impl class, not just whether it's a table or view.

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