[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/20077 )
Change subject: IMPALA-11013: Support 'MIGRATE TABLE' for external HDFS tables ...................................................................... Patch Set 5: (2 comments) Thank you very much for your work! I took a quick look at the code and left some comments. http://gerrit.cloudera.org:8080/#/c/20077/5/fe/src/main/java/org/apache/impala/analysis/ConvertTableToIcebergStmt.java File fe/src/main/java/org/apache/impala/analysis/ConvertTableToIcebergStmt.java: http://gerrit.cloudera.org:8080/#/c/20077/5/fe/src/main/java/org/apache/impala/analysis/ConvertTableToIcebergStmt.java@138 PS5, Line 138: .table(table.getFullName()) To ensure that the hdfs table is a pure external table, whether the table is RENAME or DELETE, its datafile path does not change, should we make sure that "Table.TBL_PROP_EXTERNAL_TABLE" is "TRUE"? http://gerrit.cloudera.org:8080/#/c/20077/5/fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java File fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java: http://gerrit.cloudera.org:8080/#/c/20077/5/fe/src/main/java/org/apache/impala/util/MigrateTableUtil.java@235 PS5, Line 235: Math.min This "migration" action, I think, it is IO-intensive, so the concurrency should be greater than the number of cpu cores. Maybe we should use "Math.min": The minimum concurrency is 1. By default, the number of cup cores should be used. When the maximum concurrency is configured, the maximum concurrency is used first. -- To view, visit http://gerrit.cloudera.org:8080/20077 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iacdad996d680fe545cc9a45e6bc64a348a64cd80 Gerrit-Change-Number: 20077 Gerrit-PatchSet: 5 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Mon, 19 Jun 2023 10:14:46 +0000 Gerrit-HasComments: Yes
