> On 十二月 2, 2014, 2:03 a.m., Lenni Kuff wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java, > > line 591 > > <https://reviews.apache.org/r/28547/diff/1/?file=778793#file778793line591> > > > > I thought about this more over the weekend, I think this behavior is a > > bit confusing. > > Consider the case where you have > > > > Table -> SELECT > > Column -> INSERT > > > > If you call "populateChildren" you will get back all the column > > privileges, with their action set to SELECT! If someone did not understand > > this very subtle behavior it would be easy for them to write buggy code > > that went through and removed all child privileges. > > I think the correct fix is to mix what you are doing here along with my > > change in SENTRY-543 it will handle the downgrade and also properly use > > implies.
Hello Lenni, first I am very thankful that you help to review even at weekends. About the fix, I'm agreed with you. I also think put our fix together is the best way. I will bring your fix to this one. - Dapeng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28547/#review63376 ----------------------------------------------------------- On 十二月 1, 2014, 3:45 p.m., Dapeng Sun wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28547/ > ----------------------------------------------------------- > > (Updated 十二月 1, 2014, 3:45 p.m.) > > > Review request for sentry and Lenni Kuff. > > > Bugs: SENTRY-552 > https://issues.apache.org/jira/browse/SENTRY-552 > > > Repository: sentry > > > Description > ------- > > Updated the patch accroding Lenni's feedback > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 073bb33 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > e8be81f > > Diff: https://reviews.apache.org/r/28547/diff/ > > > Testing > ------- > > UnitTests in local > > > Thanks, > > Dapeng Sun > >
