Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9972 )
Change subject: IMPALA-6804: Allow SELECT and INSERT privileges on SERVER ...................................................................... Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/9972/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/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@a614 PS1, Line 614: : : : > functional_complex_view_sub does not exist so this test is already covered We still need a test case where a user has only SELECT privilege for a view that exists. Can we update this test with that scenario instead? http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@758 PS1, Line 758: AuthzError("insert into functional.alltypes partition (month, year) select id, " + : "bool_col, tinyint_col, smallint_col, a.int_col, bigint_col, float_col, " + : "a.double_col, a.date_string_col, a.string_col, a.timestamp_col, a.year, " + : "a.month from functional_rc.alltypesagg a join (select int_col, " + : "double_col, date_string_col, string_col, timestamp_col, year, month " + : "from functional_seq.alltypes) b on (a.int_col = b.int_col)", "User '%s' does " + : "not have privileges to execute 'SELECT' on: functional_rc.alltypesagg"); > The sql as written would not execute successfully even if authorization was Can we format this SQL to make it more readable? http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2750 PS1, Line 2750: // ensure they succeed here. Some tests that fail will fail here with nit: remove extra space bore "Some" http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2758 PS1, Line 2758: AuthzOk("insert into functional.alltypes partition (month, year) select id, " + nit: format this SQL to be more readable. http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2772 PS1, Line 2772: AuthzOk("insert into functional.alltypes partition (month, year) " + Add tests for any statements that require VIEW_METADATA. http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2774 PS1, Line 2774: nit: remove new line. http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2809 PS1, Line 2809: + "join functional.alltypes b ON (a.id = b.id)"); nit: fix indentation http://gerrit.cloudera.org:8080/#/c/9972/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@2835 PS1, Line 2835: AuthzOk("select a.* from functional.alltypesagg a cross join " + Add tests for any statements that require VIEW_METADATA. -- To view, visit http://gerrit.cloudera.org:8080/9972 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If6510b4d54c9902029a357f1aebb2570cd75855d Gerrit-Change-Number: 9972 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley <[email protected]> Gerrit-Reviewer: Adam Holley <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Comment-Date: Tue, 10 Apr 2018 17:33:12 +0000 Gerrit-HasComments: Yes
