> On May 31, 2018, 9:20 p.m., Sergio Pena wrote: > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Line 792 (original), 813 (patched) > > <https://reviews.apache.org/r/67402/diff/1/?file=2033279#file2033279line813> > > > > Is there some tests that verify this method or the parent public > > method? We need to make sure this new logic works as expected.
The existing tests verify that the change does not break the functionalities. The change is in private functions. It is performance improvement and does not change behavior. If I change the implementation of findMatchPrivilege to return inputPrivilege insterad of matching privilege in entityPrivileges, 24 tests failed, like testGrantRevokePrivilege, testGrantCheckWithGroupAndUser etc. This shows if the behavior changes, many tests will fail. - Na ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67402/#review204151 ----------------------------------------------------------- On June 4, 2018, 2:57 a.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67402/ > ----------------------------------------------------------- > > (Updated June 4, 2018, 2:57 a.m.) > > > Review request for sentry, kalyan kumar kalvagadda and Sergio Pena. > > > Bugs: sentry-2244 > https://issues.apache.org/jira/browse/sentry-2244 > > > Repository: sentry > > > Description > ------- > > When granting privilege to a user or role, sentry does not need to get > individual privilege with specific action from DB again after all its > associated privileges are fetched from DB. We just just find the > corresponding privilege from list to avoid extra query to DB > > > Diffs > ----- > > > sentry-service/sentry-service-server/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 5932335 > > > Diff: https://reviews.apache.org/r/67402/diff/1/ > > > Testing > ------- > > test cases in TestSentryStore succeeded > > > Thanks, > > Na Li > >
