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)

Taking this over since Adam is out.

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[]
Done


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
Done


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...)
Done


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
Done


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
Done


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
Done


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
Done


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
Done


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
Done


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
Done


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
Done



--
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 <[email protected]>
Gerrit-Reviewer: Adam Holley <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Comment-Date: Wed, 23 May 2018 14:29:55 +0000
Gerrit-HasComments: Yes

Reply via email to