Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14750 )
Change subject: IMPALA-9092: Add support for creating external Kudu table ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/14750/6/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java: http://gerrit.cloudera.org:8080/#/c/14750/6/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@128 PS6, Line 128: // Throw error if 'external.table.purge' is manually set. : AnalysisUtils.throwIfNotNull( : tblProperties_.get(KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE), : String.format("Property '%s' cannot be altered for Kudu tables", : KuduTable.TBL_PROP_EXTERNAL_TABLE_PURGE)); > I'm not sure if we should remove this restriction. I found some use cases c Good point. I think it makes sense to add this ability so that users can switch between a synchronized table and a pure external table if they wish. Removed this check and added some test coverage for this case. http://gerrit.cloudera.org:8080/#/c/14750/6/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java File fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java: http://gerrit.cloudera.org:8080/#/c/14750/6/fe/src/test/java/org/apache/impala/catalog/local/LocalCatalogTest.java@263 PS6, Line 263: // unfortunately, we cannot reformat the below lines to make them shorter since > I believe the suggestion was something like: Ah, I see. Thanks for the clarification. Makes sense to me. Updated the patch. -- To view, visit http://gerrit.cloudera.org:8080/14750 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76f81d41db0cf2269ee1b365857164a43677e14d Gerrit-Change-Number: 14750 Gerrit-PatchSet: 6 Gerrit-Owner: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Thu, 12 Dec 2019 02:13:15 +0000 Gerrit-HasComments: Yes
