impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Huaisi Xu (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-3711: Remove unnecessary privilege checks in getDbsMetadata()
Date Wed, 06 Jul 2016 20:42:19 GMT
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 <hxu@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Huaisi Xu <hxu@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message