Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14397 )
Change subject: IMPALA-9030: Handle translated external Kudu tables ...................................................................... Patch Set 8: (3 comments) I think HDFS and HBase tables also have this issue. For example, creating a non-transactional HDFS table will finally create an external table. Then in DROP TABLE we won't drop HDFS files. However, let's keep this fix simple and focus on kudu first. http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@341 PS8, Line 341: // External table cannot have 'external.table.purge' property set, which is considered : // equivalent to managed table. : if (Boolean.parseBoolean( : getTblProperties().get(KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE))) { : throw new AnalysisException(String.format("Table property '%s' cannot be set to " + : "true with an external Kudu table.", KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE)); : } I think manually changing 'external.table.purge' should not be allowed. We should also check this in AlterTableSetTblProperties#analyze(). Please also add a test to cover it. Fortunately, CREATE TABLE LIKE statement don't allow setting tblproperties and won't copy them. I think no other statements will modify the tblproperties. http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/catalog/KuduTable.java File fe/src/main/java/org/apache/impala/catalog/KuduTable.java: http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/main/java/org/apache/impala/catalog/KuduTable.java@140 PS8, Line 140: public static boolean isSynchronizedTable( Could you move these three functions into Table.java? I feel like it's not just a kudu issue. Tables on HDFS and HBase may also need these. However, let's keep this patch simple to just focus on Kudu. We can deal with HDFS, HBase issues in other patches. http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java: http://gerrit.cloudera.org:8080/#/c/14397/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java@505 PS8, Line 505: // Cannot specify the number of replicas for external Kudu tables Comment looks incorrect here. Maybe: External Kudu table is not allowed to set table property 'external.table.purges' to true. -- To view, visit http://gerrit.cloudera.org:8080/14397 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I324523361c923b7d291cb4d0f1028b1a5b653b36 Gerrit-Change-Number: 14397 Gerrit-PatchSet: 8 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Greg Solovyev <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Sat, 19 Oct 2019 06:21:38 +0000 Gerrit-HasComments: Yes
