> On Jan. 5, 2017, 3:49 a.m., Hao Hao wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 1624 > > <https://reviews.apache.org/r/54526/diff/2/?file=1596329#file1596329line1624> > > > > Do you know why isMultiActionsSupported is needed? The existing comment > > is not quite clear to me. > > Vamsee Yarlagadda wrote: > Updated the description of the review to address your question.
This method is used to check which entities support multiple actions (QUERY, UPDATE, ALL). As per the comment, it looks like Database and Table are the ones that support all three. Do you think even Server, Column support all actions? If so, we can get rid of the entire method. - Vamsee ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54526/#review160558 ----------------------------------------------------------- On Jan. 9, 2017, 8:15 p.m., Vamsee Yarlagadda wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54526/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2017, 8:15 p.m.) > > > Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar > kalvagadda. > > > Repository: sentry > > > Description > ------- > > 1. > isMultiActionsSupported used to return true for all invocations as > tPrivilege.getDbName() is never set to NULL. > So we should change it instead to do a proper check for null - > SentryStore.isNull() instead. > > 2. Currently this method is only dealing database name string but it should > also check for table name strings as mentioned in the comments of the above > method "/ Currently INSERT/SELECT/ALL are supported for Table and DB level > privileges". So fixes the method to handle tables as well. > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 3f3afb7080ee330fedd48f4d400553fe14d57deb > > Diff: https://reviews.apache.org/r/54526/diff/ > > > Testing > ------- > > ```bash > vamsee-MBP:sentry-service-server vamsee$ mvn clean test > -Dtest=TestSentryStore -DfailIfNoTests=false > ... > ------------------------------------------------------- > T E S T S > ------------------------------------------------------- > Running org.apache.sentry.provider.db.service.persistent.TestSentryStore > 2016-12-08 00:08:59.772 java[5207:544535] Unable to load realm info from > SCDynamicStore > Tests run: 41, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 18.302 sec - > in org.apache.sentry.provider.db.service.persistent.TestSentryStore > > Results : > > Tests run: 41, Failures: 0, Errors: 0, Skipped: 2 > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 44.421s > [INFO] Finished at: Thu Dec 08 00:09:18 PST 2016 > [INFO] Final Memory: 67M/687M > [INFO] > ------------------------------------------------------------------------ > ``` > > > Thanks, > > Vamsee Yarlagadda > >
