Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3711: Check privileges when necessary ......................................................................
Patch Set 7: (2 comments) Thanks Huaisi, it looks much better now. Can you please update all the comments? I will then do a final review. http://gerrit.cloudera.org:8080/#/c/3371/7/fe/src/main/java/com/cloudera/impala/service/Frontend.java File fe/src/main/java/com/cloudera/impala/service/Frontend.java: PS7, Line 595: * @param dbName database to search for tablePattern : * @param tablePatternMatcher matcher used to match tables in dbName : * @param user user used to check privileges : * @return a list of tables names filtered by matcher and user has : * accessing privileges : * @throws ImpalaException when dbName is null or error calling user.getShortName() : * in hasAccess() We are not very consistent with our comments. Maybe remove Javadoc for now since we don't seem to use it anywhere else in this file. PS7, Line 605: List<String> tblNames = Lists.newArrayList(); : if (tablePatternMatcher == null) return tblNames; You can use this pattern instead here and everywhere else. emptyList() is a bit cheaper to construct and is immutable. if (tablePatternMatcher == null) return List.<String>emptyList(); List<String> tblNames = Lists.newArrayList(); -- To view, visit http://gerrit.cloudera.org:8080/3371 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14 Gerrit-PatchSet: 7 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
