> On May 26, 2017, 8:13 p.m., Sergio Pena wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Lines 2890 (patched) > > <https://reviews.apache.org/r/59510/diff/2/?file=1732547#file1732547line2890> > > > > The name of the method does not match with the result. It says > > 'getFirst..' but the caller might expect more than 1 result due to the > > list? We should only return the one object instead. Notice that the object > > might be null. > > > > Another idea is just return True or False if at least 1 row is found. > > Do you expect that there are other callers which require only one row of > > the table? If not, why not keeping this block of code inside > > isAuthzPathsMappingEmpty()? It is only a 2-3 lines of code.
with the query.setRange(0, 1) the query only returns the first record even if there are more. The returned list could either be empty OR have one entry in it. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59510/#review176222 ----------------------------------------------------------- On May 25, 2017, 1:41 p.m., kalyan kumar kalvagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59510/ > ----------------------------------------------------------- > > (Updated May 25, 2017, 1:41 p.m.) > > > Review request for sentry, Alexander Kolbasov, Brian Towles, Hao Hao, Na Li, > Sergio Pena, Vamsee Yarlagadda, and Vadim Spector. > > > Bugs: SENTRY-1774 > https://issues.apache.org/jira/browse/SENTRY-1774 > > > Repository: sentry > > > Description > ------- > > HMSFollower should always depend on persisted information to decide is full > snapshot is needed instead of depending on in-memory information. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 8450402 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/HMSFollower.java > 58e8881 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 440b0e9 > > > Diff: https://reviews.apache.org/r/59510/diff/2/ > > > Testing > ------- > > Added new test to check new functionality added and also ran HMSFollower tests > > > Thanks, > > kalyan kumar kalvagadda > >
