Zoltan Borok-Nagy 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: (5 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@92 PS1, Line 92: private static final long MAJOR_VERSION = 3; nit: HIVE_MAJOR_VERSION? http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@437 PS1, Line 437: * Set impala capabilities to hive client nit: could you elaborate a little? E.g. why is it needed, what are these capabilities? http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java File fe/src/main/java/org/apache/impala/analysis/Analyzer.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@129 PS1, Line 129: public static final byte REQUIRE_READ = (byte)2; > They are consistent with hive thrift defined constant for access type, and Why not import hive_metastoreConstants access types? http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@150 PS1, Line 150: "Table %s not supported. Bucketed tables are only supported " + : "for read." nit: How about "%s is a bucketed table. Only read operations are supported on such tables." http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java: http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@876 PS1, Line 876: if (MetastoreShim.getMajorVersion() > 2) { Maybe you could add tests for the other statements as well, e.g. COMPUTE STATS, CREATE TABLE LIKE, DROP, etc. -- 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 <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Sudhanshu Arora <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Reviewer: Yongzhi Chen <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 11 Jun 2019 15:12:18 +0000 Gerrit-HasComments: Yes
