Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 )
Change subject: IMPALA-6802 (part 4): Clean up authorization tests ...................................................................... Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java: http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@104 PS1, Line 104: private static final String[] ALLTYPES_COLUMNS = (String []) ArrayUtils.addAll( nit: String[] http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@923 PS1, Line 923: .ok(onServer(TPrivilegeLevel.INSERT)) We need to add REFRESH since VIEW_METADATA consists of SELECT, INSERT, and REFRESH http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@927 PS1, Line 927: .error(accessError("functional")); Missing check .error(accessError("functional"), onDatabase...) http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@933 PS1, Line 933: TTableName tableName = new TTableName("functional","alltypes"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@935 PS1, Line 935: authorize("describe functional.alltypes") describe <table> uses ANY privilege, we need more test cases with all other privileges. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@961 PS1, Line 961: String [] locationString = new String[]{"Location:"}; nit: no space for array declaration http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@989 PS1, Line 989: tableName = new TTableName("functional","alltypes_view"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1013 PS1, Line 1013: tableName = new TTableName("functional","alltypes_view"); nit: space after comma http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1018 PS1, Line 1018: viewStrings); join L1018 with L1017 http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1040 PS1, Line 1040: authorize("describe functional.allcomplextypes.int_struct_col") describe <table> uses ANY privilege, we need more test cases with all other privileges. http://gerrit.cloudera.org:8080/#/c/10442/1/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1183 PS1, Line 1183: String [] requiredStrings, String [] excludedStrings, nit: fix indentation -- To view, visit http://gerrit.cloudera.org:8080/10442 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4cf3a4751b41908ef81ec35d89a2713d9fa0dc4 Gerrit-Change-Number: 10442 Gerrit-PatchSet: 1 Gerrit-Owner: Adam Holley <g...@holleyism.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Comment-Date: Tue, 22 May 2018 23:11:45 +0000 Gerrit-HasComments: Yes
