xuefuz 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_r284859760
########## 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: I guess you didn't get what I meant. I was saying the comment doesn't seem making much sense. It doesn't explain why we put checks and why we are mentioning in-memory catalog here. There is a "thus", but I don't understand the cause/result relationship that you're trying to explain. ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services