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

Reply via email to