Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9805 )
Change subject: IMPALA-6649: Add fine-graned ALTER privilege ...................................................................... Patch Set 1: (16 comments) http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-6649: Add fine-graned ALTER privilege typo: grained http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@9 PS1, Line 9: Updated support and analysis files to provide ALTER privilege. We need ALTER on server scope as well. http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@11 PS1, Line 11: Example statements: Please mention the privilege scope. http://gerrit.cloudera.org:8080/#/c/9805/1//COMMIT_MSG@19 PS1, Line 19: Added ALTER tests to cover scope of existing ALTER tests but in Make sure to run all front-end tests. http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java@250 PS1, Line 250: AnalyzesOk(String.format("%s ALTER ON DATABASE functional %s myrole", formatArgs)); Add test for server scope. http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java: http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@338 PS1, Line 338: sentryService.grantRolePrivilege(USER, roleName, privilege); Add TestServerLevelAlter similar to TestServerLevelCreate http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1387 PS1, Line 1387: AuthzOk("ALTER TABLE functional_seq_snap.alltypes RENAME TO functional_seq_snap.t1"); I think for RENAME TO, we need a CREATE privilege at the database-level and ALTER privilege at the table-level. Please add the comment. http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1434 PS1, Line 1434: "'hdfs://localhost:20500/test-warehouse/no_access'", nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1438 PS1, Line 1438: "'/test-warehouse/no_access'", nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1442 PS1, Line 1442: "SET LOCATION '/test-warehouse/no_access'", nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1472 PS1, Line 1472: "PARTITION(year=2011, month=3) " + nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1473 PS1, Line 1473: "PARTITION(year=2011, month=4) LOCATION '/test-warehouse/no_access'", nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1475 PS1, Line 1475: "hdfs://localhost:20500/test-warehouse/no_access"); nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1478 PS1, Line 1478: "'hdfs://localhost:20510/test-warehouse/new_table'", nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1480 PS1, Line 1480: "hdfs://localhost:20510/test-warehouse/new_table"); nit: formatting http://gerrit.cloudera.org:8080/#/c/9805/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1572 PS1, Line 1572: // ALTER privilege on view only. Update the comment about required privileges to do RENAME TO -- To view, visit http://gerrit.cloudera.org:8080/9805 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0b25d10a8634829fbe90e308dfc7efc8182fef2d Gerrit-Change-Number: 9805 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Comment-Date: Mon, 26 Mar 2018 19:55:06 +0000 Gerrit-HasComments: Yes
