Sudhanshu Arora has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )
Change subject: IMPALA-8593: Prohibit write operations for bucketed tables ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78 PS1, Line 78: private static final String CONNECTORREAD = "CONNECTORREAD".intern(); > These are hive defined strings for capabilities, they only recognize these Just to be clear I was saying to do the following public enum Capability { CONNECTOREAD("CONNECTTOREAD") private final String capabilityName; private final getName() { return capabilityName; } } And we can use the getName to get the String. If you still feel using String constants is better, please go ahead with that. Also what's the benefit of calling intern on the string constants? http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@93 PS1, Line 93: analyzer.ensureTableWriteSupported(table_); > Hive defines access types as byte, we combine the access type check in Met 1) If Hive does not define Metadata_modification and we want to fold this operation under write. We can do that, i.e. we could still define @OperationType(WRITE) 2) What is the advantage of using bitwise operators? I do think annotations are introducing slightly more work. And before we make the change we should agree that it offers more clarity than the current changes. http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@4021 PS1, Line 4021: ParserError("Create table bucketed_tbl(order_id int, order_name string)" > Parsing happens before analysis. Impala does not recognize clustered by, so My bad. I don't know what I was thinking :). -- To view, visit http://gerrit.cloudera.org:8080/13558 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec Gerrit-Change-Number: 13558 Gerrit-PatchSet: 1 Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sudhanshu Arora <sudhan...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 11 Jun 2019 16:04:26 +0000 Gerrit-HasComments: Yes