Huaisi Xu has posted comments on this change.

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


Patch Set 19: Code-Review+2

(3 comments)

thanks. carry Henry's +2

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

PS19, Line 210: Implement Hive's pattern-matching semantics for "SHOW TABLE 
[[LIKE] 'pattern']", and
              :    * return a list of table names matching an optional pattern.
              :    * The only metacharacters are '*' which matches any string 
of characters, and '|'
              :    * which denotes choice.  Doing the work here saves loading 
tables or databases from the
              :    * metastore (which Hive would do if we passed the call 
through to the metastore
              :    * client). If the pattern is null, a
> This comment seems like it would be better on PatternMatcher.createHivePatt
this is exactly what "show table ..." uses. createHivePatternMatcher() can be 
called for many places, i.e. "show functions ..", "show databases ...". I think 
it is better to have an 1:1 matching to the corresponding sql statement. 
actually I think we should do this kind of comment on all such thrift api?


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

PS19, Line 227: matcheres
> matchers
Done


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

PS19, Line 1544: assertEquals(matchAllMatcher1, matchAllMatcher2);
> this test should confirm that the matchers have the same behaviour, not tha
Done. changed all occurrence to check behavior.


-- 
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: 19
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