impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Taras Bobrovytsky (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4000: Restricted Sentry authorization for Kudu Tables
Date Thu, 24 Nov 2016 01:26:18 GMT
Taras Bobrovytsky has posted comments on this change.

Change subject: IMPALA-4000: Restricted Sentry authorization for Kudu Tables
......................................................................


Patch Set 4:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/5047/4//COMMIT_MSG
Commit Message:

Line 12: - Column level permissions are not supported.
> ... on Kudu tables
Done


Line 13: - Permissions on certain operations (e.g. only SELECT) are not
> Move this point to the top and rephrase to:
Done


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

Line 247:     // See https://issues.cloudera.org/browse/IMPALA-4000 for details.
> no need for the whole link, just "See IMPALA-4000 for details."
Done


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

Line 287:       // At this time Kudu tables only support the table level ALL privilege.
> slightly rephrase:
Done


Line 291:       } else if (scope_ == TPrivilegeScope.COLUMN) {
> logic is a little simpler to me if you make this an independent "if" (and n
Done


http://gerrit.cloudera.org:8080/#/c/5047/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java:

Line 164:       AnalysisError(String.format(
> Write a loop somewhere that goes over all privilege levels to make sure the
Since there are only two other privilege levels (INSERT AND SELECT), it's simpler to just
have two test cases.


Line 204:       AnalysisError(String.format("%s SELECT (id, bool_col) ON TABLE " +
> use ALL here to get the other err msg
Yeah, I thought about it, but it doesn't work either, the error would be:
Only 'SELECT' privileges are allowed in a column privilege spec.


http://gerrit.cloudera.org:8080/#/c/5047/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

Line 2168:     // IMPALA-4000: Only users with ALL privileges on SERVER may create external
Kudu
> remove comment (this is a test for admin users)
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I183f08ad8ce80deee011a6b90ad67b9cefc0452c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message