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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
