Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10442 )
Change subject: IMPALA-6802 (part 4): Clean up authorization tests ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/10442/3/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/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@918 PS3, Line 918: @Test perhaps add a comment explaining what's being tested... from a first glance, it looks like certain privs can see certain fields, and others cannot? if there are docs or jiras that explain this more, pls reference them here. http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@922 PS3, Line 922: consistent spacing (see comment below) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@938 PS3, Line 938: and several more places below http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@946 PS3, Line 946: allExcept( : TPrivilegeLevel.ALL, TPrivilegeLevel.SELECT) factor out (repeated 3 times in this block, so it adds too much noise). http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@952 PS3, Line 952: ng[]{"id"}, explain what is being tested here (missing why "id" is special) http://gerrit.cloudera.org:8080/#/c/10442/3/fe/src/test/java/org/apache/impala/analysis/AuthorizationTestV2.java@1181 PS3, Line 1181: use consistent spacing for this... from a brief look, seems like no space is the preference. -- 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: 3 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-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Fri, 08 Jun 2018 22:02:14 +0000 Gerrit-HasComments: Yes
