> On Nov. 16, 2017, 8:14 p.m., Vadim Spector wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > > Line 3251 (original), 3251 (patched) > > <https://reviews.apache.org/r/63886/diff/1/?file=1894544#file1894544line3251> > > > > So, it means this code only worked for MAuthzPathsMapping type. > > > > Which implies we've never tested it for any other type - shouldn't we > > add such a test now? > > Arjun Mishra wrote: > It is acutally usd by isHmsNotificationEmpt() method, which passes in > MSentryHmsNotification.class. I am not sure why an exception was never thrown > because this is the first method we execute before taking a full snapshot for > the first time. > > Test cases were included in TestHMSFollower and they seem to pass. Did > you still want me to add tests?
No, actually, nevermind, the way Java generics work, the old casting works even if the type is wrong, because it is a collection. The fix is ok. - Vadim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63886/#review191238 ----------------------------------------------------------- On Nov. 16, 2017, 7:29 p.m., Arjun Mishra wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63886/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2017, 7:29 p.m.) > > > Review request for sentry, kalyan kumar kalvagadda, Na Li, Sergio Pena, and > Vadim Spector. > > > Repository: sentry > > > Description > ------- > > Current isTableEmpty implementation is below > private boolean isTableEmptyCore(PersistenceManager pm, Class clazz) { > Query query = pm.newQuery(clazz); > query.addExtension(LOAD_RESULTS_AT_COMMIT, "false"); > // setRange is implemented efficiently for MySQL, Postgresql (using the > LIMIT SQL keyword) > // and Oracle (using the ROWNUM keyword), with the query only finding the > objects required > // by the user directly in the datastore. For other RDBMS the query will > retrieve all > // objects up to the "to" record, and will not pass any unnecessary > objects that are before > // the "from" record. > query.setRange(0, 1); > return ((List<MAuthzPathsMapping>) query.execute()).isEmpty(); > } > We seem to be casting query.execute to a List<MAuthzPathsMapping> when there > is no need for it > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 7217dea7a > > > Diff: https://reviews.apache.org/r/63886/diff/1/ > > > Testing > ------- > > mvn -f sentry-tests/sentry-tests-hive/pom.xml test > -Dtest=TestPrivilegesAtTableScopePart1 > > > Thanks, > > Arjun Mishra > >
