Huaisi Xu has posted comments on this change.

Change subject: IMPALA-3711: Remove unnecessary privilege checks in 
getDbsMetadata()
......................................................................


Patch Set 18:

(10 comments)

Thanks! used similar implementation as java's immutable Collections.EMPTY_LIST

http://gerrit.cloudera.org:8080/#/c/3371/18/fe/src/main/java/com/cloudera/impala/catalog/Db.java
File fe/src/main/java/com/cloudera/impala/catalog/Db.java:

PS18, Line 432: If matcher is null, return an empty list.
> will do
Done


Line 436:     if (matcher == null) return Collections.emptyList();
> will do checknotnull(matcher) after review.
Done


http://gerrit.cloudera.org:8080/#/c/3371/18/fe/src/main/java/com/cloudera/impala/service/Frontend.java
File fe/src/main/java/com/cloudera/impala/service/Frontend.java:

Line 593:    * accessible to 'user'. Returns an empty list if 'matcher' is null.
updated


Line 619:     if (matcher == null) return Collections.emptyList();
> will do checknotnull(matcher) after review
Done


Line 637:    * accessible to 'user'. Returns an empty list if 'matcher' is null.
updated


http://gerrit.cloudera.org:8080/#/c/3371/18/fe/src/main/java/com/cloudera/impala/service/MetadataOp.java
File fe/src/main/java/com/cloudera/impala/service/MetadataOp.java:

Line 22: import org.apache.hadoop.yarn.webapp.hamlet.HamletSpec;
> will remove after review
Done


Line 228:    * associated search patterns in 'metadataParams'. All patterns are 
treated as JDBC
> will update after review.
Done


http://gerrit.cloudera.org:8080/#/c/3371/18/fe/src/main/java/com/cloudera/impala/util/PatternMatcher.java
File fe/src/main/java/com/cloudera/impala/util/PatternMatcher.java:

PS18, Line 46: public static final PatternMatcher MATCHER_MATCH_ALL = new 
PatternMatcher();
             :   public static final PatternMatcher MATCHER_MATCH_NONE;
             :   static {
             :     MATCHER_MATCH_NONE = new PatternMatcher();
             :     MATCHER_MATCH_NONE.patterns_ = Collections.emptyList();
             :   }
> That's the point: the class implementation needs to stay read-only. 
Done


http://gerrit.cloudera.org:8080/#/c/3371/18/fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java
File fe/src/test/java/com/cloudera/impala/analysis/AuthorizationTest.java:

Line 31: import org.apache.hadoop.yarn.webapp.hamlet.HamletSpec;
> will remove after review.
Done


Line 1581:         PatternMatcher.createHivePatternMatcher(null), USER);
> will remove after review.
Done


-- 
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: 18
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Huaisi Xu <[email protected]>
Gerrit-HasComments: Yes

Reply via email to