Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13465 )
Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/13465/2/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/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@118 PS2, Line 118: // Checking for 'EXTERNAL' is case-insensitive, see IMPALA-5637. : String keyForExternalProperty = : MetaStoreUtil.findTblPropKeyCaseInsensitive(tblProperties_, "EXTERNAL"); > nit: while you are here, maybe move this under the 'if (authzConfig.isEnabl Done http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@132 PS2, Line 132: manually > nit: what does 'manually' means in this context? Is it possible to set it Done http://gerrit.cloudera.org:8080/#/c/13465/2/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@132 PS2, Line 132: > nit: drop the extra space or drop the space and the period altogether. Done -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <tmarsh...@cloudera.com> Gerrit-Comment-Date: Tue, 04 Jun 2019 18:19:41 +0000 Gerrit-HasComments: Yes
