impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
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:


Thanks Huaisi, it looks much better now. Can you please update all the comments? I will then
do a final review.
File fe/src/main/java/com/cloudera/impala/service/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I17d8c5b9fb12483e4b01b819fba48b6849311a14
Gerrit-PatchSet: 7
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Huaisi Xu <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Huaisi Xu <>
Gerrit-HasComments: Yes

View raw message