Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/10030 )
Change subject: IMPALA-6451: AuthorizationException in CTAS for Kudu tables ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/10030/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: http://gerrit.cloudera.org:8080/#/c/10030/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@171 PS5, Line 171: removeHiddenTableProperties(properties); I feel we should have all the property-removal logic inside removeHiddenTableProperties(). The logic here seems correct, but a little tricky to understand why. What we really want os that external Kudu tables keep the table name, and managed Kudu tables have the name hidden. So it's better to express that directly imo. It's ok to add more args to removeHiddenTableProperties() but I think the properties should have all the info you need (external or managed table). -- To view, visit http://gerrit.cloudera.org:8080/10030 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iac3bbe4dceb80dfefd9756bc322f8c10ceab9f49 Gerrit-Change-Number: 10030 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Adam Holley <g...@holleyism.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Comment-Date: Fri, 13 Apr 2018 15:59:24 +0000 Gerrit-HasComments: Yes
