----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56481/#review165909 -----------------------------------------------------------
sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java (line 56) <https://reviews.apache.org/r/56481/#comment237746> Can you add more detail comments to describe the class more clearly? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java (line 82) <https://reviews.apache.org/r/56481/#comment237747> Nit: Return sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java (line 114) <https://reviews.apache.org/r/56481/#comment237755> Can you rephrase the sentense a bit? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java (line 137) <https://reviews.apache.org/r/56481/#comment237756> Hard to understand the name. By purely looking at the logic, it seems what it is doing it just construct the QueryParamBuilder from the privilege. No more privileges are popluated from the given privilege. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java (line 157) <https://reviews.apache.org/r/56481/#comment237750> typo. iff sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java (line 37) <https://reviews.apache.org/r/56481/#comment237754> What if we need subexpression for other relation such as '>' or '<='? sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java (line 40) <https://reviews.apache.org/r/56481/#comment237753> We may want to point out that default operation is AND if not specified. sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java (line 78) <https://reviews.apache.org/r/56481/#comment237751> type key3 == :val3. And why the subexpression here would be translated to OR relation? Defalut operation is AND? - Hao Hao On Feb. 9, 2017, 7:42 a.m., Alexander Kolbasov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56481/ > ----------------------------------------------------------- > > (Updated Feb. 9, 2017, 7:42 a.m.) > > > Review request for sentry, Hao Hao, kalyan kumar kalvagadda, Sergio Pena, > Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1625 > https://issues.apache.org/jira/browse/SENTRY-1625 > > > Repository: sentry > > > Description > ------- > > SENTRY-1625: PrivilegeOperatePersistence can use QueryParamBuilder > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java > b1180bff1ce8850712e842e59e0e7217273ac59c > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/model/MSentryGMPrivilege.java > 55b61ac07c6865402ffe4d0ff1690afb435deb95 > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/QueryParamBuilder.java > PRE-CREATION > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 7b926a5e546091881d4f7b4f30df4ae82dcd35b0 > > sentry-service/sentry-service-server/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 17a4a937221891a72ee44db92976cfa5cab40bc4 > > Diff: https://reviews.apache.org/r/56481/diff/ > > > Testing > ------- > > > Thanks, > > Alexander Kolbasov > >
