Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9805 )
Change subject: IMPALA-6649: Add fine-grained ALTER privilege ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/9805/4/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/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1400 PS4, Line 1400: remove newline (it seems to make no sense) http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1409 PS4, Line 1409: AuthzOk("ALTER TABLE functional.table_no_newline_part RENAME TO functional_seq_snap.t1"); Add a negative case where the user doesn't have CREATE on the target DB. Or do we have that already? http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1418 PS4, Line 1418: remove newline http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1422 PS4, Line 1422: // Alter table and set location to a path the user does not have access to. Mention what is needed here. ALL on the URI? http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@1575 PS4, Line 1575: AuthzOk("ALTER VIEW functional.alltypes_view rename to functional_seq_snap.view_view_1"); Also add negative test where user doesn't have CREATE on the target DB (or point me to the line with the existing test) http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2705 PS4, Line 2705: remove newline http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/resources/authz-policy.ini.template File fe/src/test/resources/authz-policy.ini.template: http://gerrit.cloudera.org:8080/#/c/9805/4/fe/src/test/resources/authz-policy.ini.template@31 PS4, Line 31: alter_functional_table_no_newline_part, alter_functional_alltypes_view,\ table_no_newline_part is a really odd table, can we use a more "regular" table instead? The reason why I want to avoid relying on "table_no_newline_part" here is the following. If we ever decide to clean up our data loading to not pollute the snapshot with extremely specific tables like "table_no_newline_part", then we'll also need to fix these auth tests which really are completely unrelated to the purpose of "table_no_newline_part". -- 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: 4 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Comment-Date: Thu, 29 Mar 2018 23:46:27 +0000 Gerrit-HasComments: Yes
