> On Jan. 9, 2017, 6:27 p.m., Mat Crocker wrote:
> > should your || be an && ?
> > this logic suggests its ok if the db is null, but the table name isn't

The comment for this function says: "// Currently INSERT/SELECT/ALL are 
supported for Table and DB level privileges"

So if the privilege has either DB or Table set, then they support multiple 
actions. So that's why it has to be || instead of &&.


- Vamsee


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54526/#review160920
-----------------------------------------------------------


On Jan. 5, 2017, 12:53 a.m., Vamsee Yarlagadda wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54526/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 12:53 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, Hao Hao, and kalyan kumar 
> kalvagadda.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> Renames the method isMultiActionsSupported to isDBNull to reflect the right 
> action being performed.
> Change based on: https://reviews.apache.org/r/54525/
> 
> Should we remove the method as the whole and do the DB null check directly at 
> the if clauses?
> 
> 
> 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
> 
>

Reply via email to