> On Sept. 13, 2014, 1:28 a.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java, > > line 276 > > <https://reviews.apache.org/r/24961/diff/2/?file=676637#file676637line276> > > > > Could you please add some more test cases on partial privileges, eg > > remove insert from all. That part of code is bit messy, so it would be good > > to have a bit more tests for that. Thanks!
I have test cases to handle partial privileges, pls take a look at TestSentryStore.testGrantRevokePrivilegeWithColumn() > On Sept. 13, 2014, 1:28 a.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 545 > > <https://reviews.apache.org/r/24961/diff/2/?file=676634#file676634line545> > > > > I guess if this is a server level privilege, then both will be null. Do > > we have to add that check as well ? Yes, I will fix it. > On Sept. 13, 2014, 1:28 a.m., Prasad Mujumdar wrote: > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift, > > line 48 > > <https://reviews.apache.org/r/24961/diff/2/?file=676635#file676635line48> > > > > I don't think we should change the sequence of fields. Thrift use that > > to maintain the backward compatibility. > > The columnName should be added at the end as 11th field. Yes, if we just upgrade the sentry server library, it will has compartible issues with client side in thrift. I will fix it. - Xiaomeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24961/#review53257 ----------------------------------------------------------- On Sept. 4, 2014, 10:23 a.m., Xiaomeng Huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24961/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2014, 10:23 a.m.) > > > Review request for sentry, Arun Suresh, Prasad Mujumdar, and Sravya > Tirukkovalur. > > > Repository: sentry > > > Description > ------- > > rename table should get child privileges firstly, then add new child > privileges with new tablename > > > Diffs > ----- > > > sentry-core/sentry-core-model-db/src/main/java/org/apache/sentry/core/model/db/DBModelAuthorizable.java > de35bfa > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryAuthorizable.java > 59418a3 > > sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TSentryPrivilege.java > 54b6204 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 33600e9 > > sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift > b14616b > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryPrivilege.java > 91d3171 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > 985a73d > > Diff: https://reviews.apache.org/r/24961/diff/ > > > Testing > ------- > > test case are included. > > > Thanks, > > Xiaomeng Huang > >
