impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3711: Check privileges when necessary
Date Mon, 27 Jun 2016 22:39:44 GMT
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 <hxu@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Huaisi Xu <hxu@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message