Fang-Yu Rao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/17640 )

Change subject: IMPALA-10436: Support storage handler privileges for external 
Kudu table creation
......................................................................


Patch Set 10:

(5 comments)

Hi all, I have slightly revised the previous patch set according to Quanlong's 
suggestions. Let me know if you have any other comment on the patch. Thanks!

http://gerrit.cloudera.org:8080/#/c/17640/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17640/9//COMMIT_MSG@14
PS9, Line 14: supported by Apache Ranger once RANGER-3281 is resolved, which in 
turn
            : depends on the release of Apache Hive 4.0 that consists of 
HIVE-24705.
> I see HIVE-24705 is resolved but RANGER-3281 is still open. Do we need to b
Thanks Quanlong!

We do not have to bump up the build number for this patch in that RANGER-3281 
is already shipped in Cloudera's distribution of Ranger.

My current understanding is that Apache Ranger relies on the official 
release(s) of Apache Hive's API's. Since HIVE-24705 exists only on Apache 
Hive's master, which has not yet been officially released, RANGER-3281 could 
not be committed to  Apache Ranger's master at the moment. Thus RANGER-3281 is 
still open.

At https://hive.apache.org/downloads.html we can see that 4.0.0-alpha-1 and 
3.1.3 of Hive were released in the past March and April, respectively. We will 
have to wait for Hive 4 until we can resolve RANGER-3281 .


http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@61
PS9, Line 61: import com.google.common.primitives.Ints;
> nit: move this to line 59 to keep the import list sorted
Done


http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@369
PS9, Line 369:       boolean isExternal = tableDef_.isExternal() ||
             :           MetaStoreUtil.findTblPropKeyCaseInsensitive(
             :               getTblProperties(), "EXTERNAL") != null;
             :       if (isExternal) {
> Should we update these?
Thanks Quanlong!

I will remove this stale code comment. :-)


http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/StorageHandlerUri.java
File fe/src/main/java/org/apache/impala/analysis/StorageHandlerUri.java:

http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/StorageHandlerUri.java@37
PS9, Line 37: storageHandlerUri.equals("*://*")) {
> nit: we can use equals() directly
Done


http://gerrit.cloudera.org:8080/#/c/17640/9/fe/src/main/java/org/apache/impala/analysis/StorageHandlerUri.java@47
PS9, Line 47: equals("*")) {
> nit: use equals() directly
Done



--
To view, visit http://gerrit.cloudera.org:8080/17640
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7936e1d8c48696169f7ad7ad92abe44a26eea3c4
Gerrit-Change-Number: 17640
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Mon, 31 Oct 2022 17:14:16 +0000
Gerrit-HasComments: Yes

Reply via email to