You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yongzhi Chen (Code Review)" <ge...@cloudera.org> on 2019/06/07 13:40:34 UTC

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Yongzhi Chen has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13558


Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a  method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests.

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M bin/impala-config.sh
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
19 files changed, 311 insertions(+), 13 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/1
-- 
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: newchange
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 1
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4:

(14 comments)

Patch set 4 addresses the issue and passed core tests for Hive2

http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG@9
PS3, Line 9:  
> nit: extra space
Done


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@60
PS3, Line 60: import org.apache.impala.common.Imp
> I didn't find where we use this.
removed


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS3, Line 64: import org.apache.impala.service.MetadataOp;
> This is only mentioned in comment, but is not used.
removed


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@438
PS3, Line 438:    * - insert-only Acid table read
             :    * - virtual view read
             :    * - materialized view read
             :    */
> Can you make a bullet list from this?
Done


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@445
PS3, Line 445: ame = I
> Unused variable.
Done


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@451
PS3, Line 451:     // Use MAJOR_VERSION for now
             :     // TODO: Change to impala build version when the functions are fixed.
             :     String impalaId = St
> I do not understand the intention here.
Explain why I am using major version not impala version for impala id: The function available in Impala does not work for catalog front end and causes for troubles. For the issue does not affect the feature(any name will be fine), comment here, we can change the name after the related functions fixed.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@454
PS3, Line 454:     String[] capabilities = new String[] {
             :         EXTWRITE, // External tab
> nit: one line
Done


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@466
PS3, Line 466:         };
> nit: missing "is"
I think the "is" can be omitted, but I will add it anyway.


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@300
PS3, Line 300: Check if the table type is supported
> nit: "Check if the table type is supported" would be better.
Done


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@311
PS3, Line 311:         // the operations are not supported, we will generate error messages
> nit: missing "are"
This can be omitted too.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@319
PS3, Line 319:       ensureTableNotTransactional(ta
> Does this make sense in the MetastoreShim.getMajorVersion() == 2 case?
It is just a direct translation of the original code where ensureTableNotFullAcid(table) used, for version 2, need check if it is transactional. I will change to ensureTableNotTransactional(table)
And this kind of check is needed for transactional table use table properties, it can go into version 2 DB.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@365
PS3, Line 365:     analyzer.ensureTableNotTransactional(table_);
> I also mentioned in AnalyzerTest.java that prohibiting COMPUTE STATS is str
Done, change back to the original implementation. I thought we cannot compute accurate stats for the tables that we do not know how to write.


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@897
PS3, Line 897:         "is a bucketed table. Only read operations are supported on such tables
> It is strange for me that DROP STATS is allowed, while COMPUTE STATS is not
Fixed.


http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql@2162
PS3, Line 2162: ---- DATASET
> Does this have to be an Orc? If not, then STORED AS {file_format} could be 
Yes, it is a transactional bucketed table which very limited file formats are satisfied. I have a non-transactional bucketed table test cover all of the supported bucketed types.



-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Jun 2019 11:37:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 11: Code-Review+2

(1 comment)

Lgtm! Thanks for applying the changes!

http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@46
PS11, Line 46: 
nit: leftover blank line from previous PS



-- 
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: 11
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jul 2019 16:53:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 13: Verified+1


-- 
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: 13
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jul 2019 15:32:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 2:

(12 comments)

Patch 3 will address the issues.

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();
> Just to be clear I was saying to do the following
These strings are only used to be packed into a string array and sent to hive as capabilities. I will use enum in the future if we need enum operations. 
I will remove the intern (just copied from hive code)


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?
It is Impala shim's 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 ca
Done


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_);
> 1) If Hive does not define Metadata_modification and we want to fold this o
Bitwise can combine two checks into one (for example check write along with read/write).  The major purpose is to integrate with hive's translation layer, hive access type is in byte, bitwise operations work fine with it and may have better performance and simpler (save a couple of switch statements) 
This function is frequently called,  checking annotation value for operation type may be roundabout.  I will accept your first option to use enum .


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;
> Why not import hive_metastoreConstants access types?
It is auto-generated code and Hive 2 does not have these constant. We do not want to break hive 2 impala.


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 
Done


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 ST
Done


http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql@2183
PS1, Line 2183: ---- LOAD
              : INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name}
              : SELECT id, int_col from functional.alltypes;
> This will only run for the non-text versions of the tables. If you want the
Done


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2172
PS2, Line 2172: CLUSTERED BY (col1) INTO 5 BUCKETS
> Without STORED AS {file_format} all tables will be text files.
Done


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2173
PS2, Line 2173: LOCATION '/test-warehouse/{table_name}';
> Adding {db_name}{db_suffix} would ensure that tables in different databases
Done


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2182
PS2, Line 2182: CLUSTERED BY (col1) INTO 5 BUCKETS;
> Same as line 2172.
Done


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2183
PS2, Line 2183: ---- LOAD
              : INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name}
              : SELECT id, int_col from functional.alltypes;
> This only loads the data to the text table, the other formats won't contain
Done



-- 
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: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 18:15:25 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 11:

(2 comments)

Submit patch 11 with:
1. Use Impala build number by calling BackendConfig.
2. Add ToDo for alter table (nonacid to acid) table properties changes.
3. Rebase and re-run Hive 2 and Hive 3 core 
Hive 2 all passed:
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5404/testReport/
Hive 3 7 failures, no regressions:
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5402/testReport/

http://gerrit.cloudera.org:8080/#/c/13558/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/10//COMMIT_MSG@26
PS10, Line 26: When upgrade tables from non-acid to acid, the default
> Can you add a TODO about ugpraded tables? My impression is that creating a 
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   // Key for config option read from hive-site.xml
            :   private static final String HIVE_METASTORE_CNXN_DELAY_MS_CONF =
            :       "impala.catalog.metastore.cnxn.creation.delay.ms";
            :   private static final int DEFAULT_HIVE_METASTORE_CNXN_DELAY_MS_CONF = 0;
            :   /
> Here to make sure the client capabilities are set before the pool start to 
new patch moves the code out of the static block.



-- 
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: 11
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Jul 2019 10:55:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/2/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/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS2, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS2, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS2, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/2/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS2, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 20:52:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3836/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Jul 2019 14:14:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 6:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/6/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/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS6, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS6, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS6, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS6, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 03:49:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py@669
PS7, Line 669:     if HIVE_MAJOR_VERSION > 2:
             :       del properties['OBJCAPABILITIES']
We could also check OBJCAPABILITIES here, as it should have a fix values.

It may also make sense to add some other tests for OBJCAPABILITIES, e.g. what is its value for insert only transactional tables or upgraded tables created by Impala.



-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 18:45:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 12: Code-Review+2


-- 
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: 12
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jul 2019 18:48:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: eUtils.valida
> Yeah, sorry about that, I thought MAJOR_VERSION is only used to generate th
I will change back.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@532
PS7, Line 532: 
> sorry, hive.
I will change to hive.


http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@270
PS7, Line 270:         break;
             :       case READ:
             :       case ANY:
             :       default:
> Yeah, but here we want to check if the table has READ access type, right?
Here is simplfied. Assume read, read/write and write all have read permission.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   static {
            :     if (MetastoreShim.getMajorVersion() > 2) {
            :       MetastoreShim.setHiveClientCapabilities();
            :     }
            :   }
> But if you only add it to MetastoreShim 3, then it should be fine, right?
Here to make sure the client capabilities are set before the pool start to create hms client. 
I tried place BackendConfig inside MetastoreClient creation around line 99, but it is still with null value.



-- 
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: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 18:23:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Sudhanshu Arora (Code Review)" <ge...@cloudera.org>.
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 <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 16:04:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3534/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 14:22:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Sudhanshu Arora (Code Review)" <ge...@cloudera.org>.
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:

(4 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();
How about using enum for this?


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_);
Nit: Another way to do this is to introduce another enum say OperationType. 

public enum OperationType {
  READ,
  WRITE,
  METADATA_MODIFICATION
}

And then mark Alter Table as METADATA_MODIFICATION.
Then we can have a function in Analyzer that takes the operationType and tells whether the operation is allowed on the table or not.

That will make this cleaner I think

Or we can introduce annotations like
@SupportedTableTypes
@OperationType
and we can say that every stmt node must be annotated with these. That will make it clear.


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;
If we create an enum then these values can go into enum itself


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)"
Wouldn't this fail during analysis?



-- 
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 <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Jun 2019 20:54:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 3:

(14 comments)

http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/3//COMMIT_MSG@9
PS3, Line 9:  
nit: extra space


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@60
PS3, Line 60: import org.apache.impala.common.Id;
I didn't find where we use this.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS3, Line 64: import org.apache.impala.service.BackendConfig;
This is only mentioned in comment, but is not used.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@438
PS3, Line 438:    * Impala supports external table read/write
             :    * insert-only Acid table read
             :    * virtual view read
             :    * materialized view read
Can you make a bullet list from this?
e.g:
   * Impala supports:
   * - external table read/write
   * - insert-only Acid table read
   * - virtual view read
   * - materialized view read


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@445
PS3, Line 445: version
Unused variable.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@451
PS3, Line 451:     // BackendConfig.INSTANCE.getImpalaBuildVersion() is NULL or
             :     // BackendConfig.INSTANCE is NULL when the method is called.
             :     // Use MAJOR_VERSION
I do not understand the intention here.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@454
PS3, Line 454:     String impalaId = String.format("Impala%s@%s",
             :         MAJOR_VERSION, hostName);
nit: one line


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@466
PS3, Line 466:                     // number -1. It makes clients unable to know it a bucketed table.
nit: missing "is"


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@300
PS3, Line 300: Check if support the table type or not
nit: "Check if the table type is supported" would be better.


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@311
PS3, Line 311:         // the operations not supported, we will generate error messages
nit: missing "are"


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@319
PS3, Line 319:       ensureTableNotFullAcid(table);
Does this make sense in the MetastoreShim.getMajorVersion() == 2 case?


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@365
PS3, Line 365:     analyzer.checkTableCapability(table_, Analyzer.OperationType.WRITE);
I also mentioned in AnalyzerTest.java that prohibiting COMPUTE STATS is strange for me. Impala can generally do COMPUTE STATS for tables it cannot write, e.g. Avro tables.

I think that Transactional tables were excluded because Hive uses the stats for them to answer some queries, and we are afraid that differences in stats by Impala can lead to incorrect results in Hive.


http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java@897
PS3, Line 897:     AnalysisError("compute stats functional.bucketed_table", errorMsgBucketed);
It is strange for me that DROP STATS is allowed, while COMPUTE STATS is not for bucketed tables. Is this intentional?


http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/3/testdata/datasets/functional/functional_schema_template.sql@2162
PS3, Line 2162: STORED AS ORC
Does this have to be an Orc? If not, then STORED AS {file_format} could be used instead.



-- 
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: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Jun 2019 15:52:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 9:

(10 comments)

Patch set 9 addresses review issues and adds more tests.

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS7, Line 78:   private static final String EXTWRITE = "EXTWRITE";
            :   private static final String EXTREAD = "EXTREAD";
> nit: unused
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@83
PS7, Line 83:   private static final String HIVEMANAGEDINSERTREAD = "HIVEMANAGEDINSERTREAD";
            :   private static final String HIVEMANAGEDINSERTWRITE = "HIVEMANAGEDINS
> nit: unused
these are some possible capabilities we may support.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@87
PS7, Line 87:   private static final String HIVEMQT = "HIVEMQT";
> nit: unused
We may need it in the future.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: eUtils.valida
> nit: Technically this class doesn't have a version number, so maybe we shou
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@470
PS7, Line 470:         HIVESQL,
> nit: maybe add TODO for HIVEMANAGEDINSERTWRITE once IMPALA-8636 goes in.
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@532
PS7, Line 532: 
> nit: impala
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@130
PS7, Line 130:   public static final byte ACCESSTYPE_READ = (byte)2;
             :   public static final byte ACCESSTYPE_WRITE = (byte)4;
             :   public static final byte ACCESSTYPE_READWRITE = (byte
> nit: maybe we could use the same naming as in Hive, i.e. ACCESSTYPE_READONL
Done


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@270
PS7, Line 270:       case READ:
             :       case ANY:
             :       default:
             :         ensureTableSupported(table);
> nit: ensureTableSupported() also succeeds if the table has ACCESSTYPE_WRITE
write only belong to the operation WRITE. In the code ensureTableSupported, require_write(write only) is already included.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   static {
            :     if (MetastoreShim.getMajorVersion() > 2) {
            :       MetastoreShim.setHiveClientCapabilities();
            :     }
            :   }
> nit: Can you place this to MetastoreShim 3 only?
The setHiveClientCapabilities is only implemented in MetasoreShim 3, in Shim2 the method just throws Unsupported exception.  add the if statement to avoid thrown unsupported Exception in Hive 2


http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/13558/7/tests/metadata/test_ddl.py@669
PS7, Line 669:     if HIVE_MAJOR_VERSION > 2:
             :       assert properties['OBJCAPABILITIE
> We could also check OBJCAPABILITIES here, as it should have a fix values.
Done



-- 
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: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 14:49:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#6).

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 392 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/6
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen 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:

> Patch Set 1:
> 
> (4 comments)

Do we really want to put import into two lines? I will keep them as is.


-- 
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: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 13:46:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

The code looks good to me, but I am still trying to grasp the details Hive capabilities concept.

For example I saw a line in HMS that suggests that managed tables without ACID will be treated as read only:
https://github.com/apache/hive/blob/ec3779978797051fdb345172536aafcd50f1b4ae/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetastoreDefaultTransformer.java#L130
Can't this break some Impala tests?

http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java@599
PS4, Line 599: catlaog
typo: catalog



-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Jul 2019 16:06:29 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen 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:

(4 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();
> How about using enum for this?
These are hive defined strings for capabilities, they only recognize these strings.


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_);
> Nit: Another way to do this is to introduce another enum say OperationType.
Hive defines access types as byte,  we combine the access type check in MetastoreShim and try not to spread the definition to so many places and take advantage of bitwise operations.  There is no METADATA_MODIFICATION in Hive, only read, write, read/write: We need the write operation(write or read/write) for it, this method includes both.


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;
> If we create an enum then these values can go into enum itself
They are consistent with hive thrift defined constant for access type, and byte type has one more advantage: it can  run bitwise operations like  |  (bitwise logical or)


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)"
> Wouldn't this fail during analysis?
Parsing happens before analysis. Impala does not recognize clustered by, so  it failed with the Syntax error



-- 
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 <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 12:02:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3868/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 15:29:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 13:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4605/ DRY_RUN=false


-- 
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: 13
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jul 2019 09:16:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2172
PS2, Line 2172: CLUSTERED BY (col1) INTO 5 BUCKETS
Without STORED AS {file_format} all tables will be text files.


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2173
PS2, Line 2173: LOCATION '/test-warehouse/{table_name}';
Adding {db_name}{db_suffix} would ensure that tables in different databases use different locations. This probably doesn't matter if the table is empty, but it is still weird if e.g. a Parquet and a text table use the same directory.


http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2182
PS2, Line 2182: CLUSTERED BY (col1) INTO 5 BUCKETS;
Same as line 2172.



-- 
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: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 15:40:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 12:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/12/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/12/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS12, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/12/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS12, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/12/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS12, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/12/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS12, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 12
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jul 2019 18:22:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 10:

(1 comment)

Add one missing comment.

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@198
PS7, Line 198: Operation typ
> nit: please add comment
Done



-- 
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: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 16:04:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 5: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2208
PS5, Line 2208:         }
What should happen if it is a full acid tables? If it shouldn't be possible, that a precondition could be used instead of the "if".


http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS5, Line 2210: !KuduTable.isKuduTable(tbl)
A comment could be added about the lack of capabilities for Kudu tables.



-- 
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: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 18:47:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins 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:

(4 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@20
PS1, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS1, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS1, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/1/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS1, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 13:41:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3863/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 15:28:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3790/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Jun 2019 12:12:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/2/testdata/datasets/functional/functional_schema_template.sql@2183
PS2, Line 2183: ---- LOAD
              : INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name}
              : SELECT id, int_col from functional.alltypes;
This only loads the data to the text table, the other formats won't contain any data.

You'll need a DEPENDENT_LOAD_HIVE section with an 'INSERT OVERWRITE TABLE {db_name}{db_suffix}.{table_name} SELECT * FROM functional.{table_name};' statement. This copies data to the other formats from the text table. See 'complextypes_fileformat' that Joe mentioned.

Of course, you could also use the same statement in DEPENDENT_LOAD_HIVE that you use in LOAD, but that way you'd need to modify the INSERT statement at both places whenever you want to make changes.



-- 
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: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 09:39:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/3/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/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS3, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS3, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS3, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/3/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS3, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 18:16:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3882/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 12
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jul 2019 19:02:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4:

Hive will treat all acid table as managed and all managed are needed to be transactional. Impala created tables will be external tables. The changing and discussion is still going on. This patch is just first step, we can make more
fixes later.
 
Following are related docs for HMS translation layer:
Hive:
https://docs.google.com/document/d/1ZmtMlKvnSz-EFGuCuYfPVA-2kfgHOWenL8sW47vkhnw/edit#
Impala:
https://docs.google.com/document/d/1lgcD_ZqIFVydLdRPQhkYBdFu2C0mH1A8VnXpcSs9psA/edit#heading=h.l9b51051hhma

Three engines: 
https://docs.google.com/document/d/1fPIn1WW-tRRSmM-aGEVl0DI-TByIswo5HrLdgSPcw9s/edit?ts=5d1baa75


-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Jul 2019 12:14:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 6:

(2 comments)

Patch set 6 addressed the review issues and was rebased to the latest hive bump.
Run cdp all core tests, the failures are the same as before the change to upstream master. Both have 12 failures:
My private cdp build:
https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5393/testReport/

upstream without the patch:
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/23/testReport/

So no regressions.

http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2208
PS5, Line 2208:       } e
> What should happen if it is a full acid tables? If it shouldn't be possible
Done


http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS5, Line 2210: 1. The property is not stor
> A comment could be added about the lack of capabilities for Kudu tables.
Done



-- 
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: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 04:00:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 5:

Ran Hive 2 and Hive 3 core tests. For hive 3 tests, there are 4 failures and are the same failures as upstream master :
	Test Result (4 failures / -7)
custom_cluster.test_hive_parquet_codec_interop.TestParquetInterop.test_hive_impala_interop[protocol: beeswax | exec_option: {'sync_ddl': 1, 'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format: parquet/none]
generate_junitxml.buildall.run-custom-cluster-tests
org.apache.impala.catalog.events.MetastoreEventsProcessorTest.testInsertEventOnRemovedTable
org.apache.impala.catalog.events.MetastoreEventsProcessorTest.testInsertEvents

https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5367/#showFailuresLink

https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/20/#showFailuresLink

So there is no regression.
Because of recent cdp bumping from 107 to 195, Impala has to add capabilities when creating tables. Most impala created table have OBJCAPABILITIES except for kudu tables (seems that kudu managed table does not store extra table properties). Therefore, kudu table will not check capabilities for now.


-- 
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: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Jul 2019 13:48:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 13: Code-Review+2


-- 
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: 13
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Jul 2019 09:16:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell 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:

(1 comment)

Drive-by comment. Hope this helps!

http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/13558/1/testdata/datasets/functional/functional_schema_template.sql@2183
PS1, Line 2183: ---- DEPENDENT_LOAD_HIVE
              : INSERT INTO TABLE {db_name}{db_suffix}.{table_name}
              : SELECT id, int_col from functional.alltypes;
This will only run for the non-text versions of the tables. If you want the same thing for text, you would want a LOAD section (which always runs in Hive). See, for example, complextypes_fileformat for a table that has an insert in the LOAD section.



-- 
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 <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 19:49:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4: Code-Review+2

I give +2, but I would prefer to run https://jenkins.impala.io/job/ubuntu-16.04-from-scratch-cdp-hive/ with the change before merging it. The cdp build is still red, but there are only a few ( 3 ) errors left on master.


-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Jul 2019 12:56:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#8).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 404 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/8
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#2).

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a  method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests.

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M bin/impala-config.sh
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
19 files changed, 311 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/2
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 12:

(1 comment)

> Patch Set 11: Code-Review+2
> 
> (1 comment)
> 
> Lgtm! Thanks for applying the changes!

Zoltan and Csaba, Thanks for the review!

http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@46
PS11, Line 46:   // Key for config option read from hive-site.xml
> nit: leftover blank line from previous PS
Done



-- 
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: 12
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Jul 2019 18:24:09 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7:

(3 comments)

Patch set 7 addresses review issues.

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG@7
PS6, Line 7: Support table capabilities handling with Hive
> I think that bucketed tables are now a secondary feature compared to capabi
Done


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS6, Line 2210: he table cannot be found after created.
> This means that HMS doesn't return Kudu tables where OBJCAPABILITIES is set
Done


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS6, Line 2211: ed jira
> typo: adding
Done



-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 14:50:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#5).

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 390 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/5
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3861/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 04:28:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 6: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/6//COMMIT_MSG@7
PS6, Line 7: Prohibit write operations for bucketed tables
I think that bucketed tables are now a secondary feature compared to capability handling. Maybe something like "Support table capabilities handling with Hive 3" would be more descriptive.


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:

http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2210
PS6, Line 2210: The table cannot be found after created.
This means that HMS doesn't return Kudu tables where OBJCAPABILITIES is set? If there is a jira for this, can you mention it in the comment?


http://gerrit.cloudera.org:8080/#/c/13558/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2211
PS6, Line 2211: addiing
typo: adding



-- 
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: 6
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 12:05:24 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 11:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3876/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 11
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Jul 2019 11:24:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#12).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.
When upgrade tables from non-acid to acid, the default
capabilities should be changed too. Currently, use the
workaround by explicitly setting tblproperties OBJCAPABILITIES
with the acid properties.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
21 files changed, 418 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/12
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 12
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 11:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/11/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/11/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS11, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS11, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS11, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/11/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS11, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 11
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Jul 2019 10:45:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7: Code-Review+1

(10 comments)

Lgtm, found some nits, but nothing serious.

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@78
PS7, Line 78:   private static final String CONNECTORREAD = "CONNECTORREAD";
            :   private static final String CONNECTORWRITE = "CONNECTORWRITE";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@83
PS7, Line 83:   private static final String HIVEFULLACIDREAD = "HIVEFULLACIDREAD";
            :   private static final String HIVEFULLACIDWRITE = "HIVEFULLACIDWRITE";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@87
PS7, Line 87:   private static final String HIVEMANAGESTATS = "HIVEMANAGESTATS";
nit: unused


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: MAJOR_VERSION
nit: Technically this class doesn't have a version number, so maybe we should rename it to IMPALA_MAJOR_VERSION


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@470
PS7, Line 470:         HIVEMANAGEDINSERTREAD,
nit: maybe add TODO for HIVEMANAGEDINSERTWRITE once IMPALA-8636 goes in.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@532
PS7, Line 532: shim
nit: impala


http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@130
PS7, Line 130:   public static final byte REQUIRE_READ = (byte)2;
             :   public static final byte REQUIRE_WRITE = (byte)4;
             :   public static final byte REQUIRE_READWRITE = (byte)8;
nit: maybe we could use the same naming as in Hive, i.e. ACCESSTYPE_READONLY, etc.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@198
PS7, Line 198: OperationType
nit: please add comment


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@270
PS7, Line 270:       case READ:
             :       case ANY:
             :       default:
             :         ensureTableSupported(table);
nit: ensureTableSupported() also succeeds if the table has ACCESSTYPE_WRITEONLY, however having a write only table might not be a real life scenario.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   static {
            :     if (MetastoreShim.getMajorVersion() > 2) {
            :       MetastoreShim.setHiveClientCapabilities();
            :     }
            :   }
nit: Can you place this to MetastoreShim 3 only?

 static {
   setHiveClientCapabilities();
 }



-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 17:45:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#7).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 393 insertions(+), 9 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/7
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: MAJOR_VERSION
> This change doesn't seem logical to me - the major version that getMajorVer
Yeah, sorry about that, I thought MAJOR_VERSION is only used to generate the variable 'impalaId' in L466.

So this is rather HIVE_MAJOR_VERSION I think.

setHiveClientCapabilites() is currently called in a static initializer block which is called by the Java runtime when the class MetastoreClientPool gets loaded. Maybe we should have a static method in the MetastoreShims and call it explicitly.


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@532
PS7, Line 532: shim
> Done
sorry, hive.


http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@270
PS7, Line 270:       case READ:
             :       case ANY:
             :       default:
             :         ensureTableSupported(table);
> write only belong to the operation WRITE. In the code ensureTableSupported,
Yeah, but here we want to check if the table has READ access type, right?

ensureTableSupported() also succeeds if the table doesn't have READ access type, only WRITE. (might not be a typical scenario to have a write-only table, but in theory it's possible).


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
File fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java:

http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java@47
PS7, Line 47:   static {
            :     if (MetastoreShim.getMajorVersion() > 2) {
            :       MetastoreShim.setHiveClientCapabilities();
            :     }
            :   }
> The setHiveClientCapabilities is only implemented in MetasoreShim 3, in Shi
But if you only add it to MetastoreShim 3, then it should be fine, right?

Anyway, until we use this static initializer block the Impala BackendConfig won't be initialized before setHiveClientCapabilities() is called. Maybe we should initialize the MetastoreShim explicitly.



-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 17:47:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 4:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/4/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/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS4, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS4, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS4, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/4/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS4, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Jun 2019 11:33:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/5/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/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS5, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS5, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS5, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/5/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS5, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 5
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Jul 2019 13:34:27 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#9).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 404 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/9
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Yongzhi Chen 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:

Please help me review the patch. Thanks


-- 
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 <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 10 Jun 2019 10:11:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3581/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Jun 2019 18:57:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#10).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
19 files changed, 405 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/10
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#4).

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests.

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
17 files changed, 346 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/4
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 4
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 8:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/13558/8/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/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS8, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS8, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS8, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS8, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/8/tests/metadata/test_ddl.py
File tests/metadata/test_ddl.py:

http://gerrit.cloudera.org:8080/#/c/13558/8/tests/metadata/test_ddl.py@30
PS8, Line 30: ,
flake8: E231 missing whitespace after ','



-- 
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: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 14:41:55 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13558/10//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13558/10//COMMIT_MSG@26
PS10, Line 26: 
Can you add a TODO about ugpraded tables? My impression is that creating a normal table then altering it to insert only ACID will leave the capabilities in the old state.


http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@92
PS7, Line 92: eUtils.valida
> Done
This change doesn't seem logical to me - the major version that getMajorVersion() returns should be the Hive version, while IMPALA_MAJOR_VERSION should be the Impala version. It is just a coincidence that both are 3, Impala's version could be raised to 4 any time, and if getMajorVersion() would also return 4, it would break some existing code.

The best would be to initialize this after BackendConfig - as far as I understand this happens extremely early and frontend code generally relies on it.



-- 
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: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 17:17:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 9:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/9/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/9/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS9, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/9/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS9, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/9/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS9, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/9/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS9, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 9
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 14:48:19 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Sahil Takiar, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#11).

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.
When upgrade tables from non-acid to acid, the default
capabilities should be changed too. Currently, use the
workaround by explicitly setting tblproperties OBJCAPABILITIES
with the acid properties.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
22 files changed, 419 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/11
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 11
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/7/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/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS7, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS7, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS7, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS7, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 7
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 14:48:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3867/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 8
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 15:21:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 10:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13558/10/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/10/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@20
PS10, Line 20: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_NONE;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/10/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@21
PS10, Line 21: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READONLY;
line too long (95 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/10/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@22
PS10, Line 22: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_READWRITE;
line too long (96 > 90)


http://gerrit.cloudera.org:8080/#/c/13558/10/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@23
PS10, Line 23: import static org.apache.hadoop.hive.metastore.api.hive_metastoreConstants.ACCESSTYPE_WRITEONLY;
line too long (96 > 90)



-- 
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: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 16:03:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Yongzhi Chen (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Joe McDonnell, Csaba Ringhofer, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13558

to look at the new patch set (#3).

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................

IMPALA-8593: Prohibit write operations for bucketed tables

This patch adds a  method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests.

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
---
M bin/impala-config.sh
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/MetaStoreClientPool.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
19 files changed, 354 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/13558/3
-- 
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: newpatchset
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 3
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................

IMPALA-8593: Support table capabilities handling with Hive 3

This patch adds a method to check if a table bucketed.
For Hive 3, integrates with HMS translation layer for
capabilities checks.
Implements methods ensureTableWriteSupported and
ensureTableReadSupported.
Set default capabilities for tables.

Tests:
Added unit tests to ParserTest and AnalyzerTest.
Added bucketed tables which are required by IMPALA-8439.
Ran core tests(Hive 2 and Hive 3)

ToDo:
Integrate checking bucketed tables capabilities and creating
error messages with HMS translation after Hive provides the
required functions.
Enable capabilities checking for Kudu tables.
When upgrade tables from non-acid to acid, the default
capabilities should be changed too. Currently, use the
workaround by explicitly setting tblproperties OBJCAPABILITIES
with the acid properties.

Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Reviewed-on: http://gerrit.cloudera.org:8080/13558
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniCatalog.java
M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/testutil/CatalogServiceTestCatalog.java
M testdata/datasets/functional/functional_schema_template.sql
M testdata/datasets/functional/schema_constraints.csv
M tests/metadata/test_ddl.py
M tests/metadata/test_show_create_table.py
21 files changed, 418 insertions(+), 11 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
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: merged
Gerrit-Change-Id: Ia08d01168660830b6e0d08b55a95eac129889cec
Gerrit-Change-Number: 13558
Gerrit-PatchSet: 14
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Prohibit write operations for bucketed tables
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3566/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 2
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 21:36:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Support table capabilities handling with Hive 3

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13558 )

Change subject: IMPALA-8593: Support table capabilities handling with Hive 3
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3869/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
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: 10
Gerrit-Owner: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jul 2019 16:43:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8593: Prohibit write operations for bucketed tables

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 <yc...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sudhanshu Arora <su...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yongzhi Chen <yc...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Jun 2019 15:12:18 +0000
Gerrit-HasComments: Yes