> On March 14, 2018, 6:33 p.m., kalyan kumar kalvagadda wrote: > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > > Line 363 (original), 363 (patched) > > <https://reviews.apache.org/r/65768/diff/3/?file=1972379#file1972379line363> > > > > if changing the database is not supported hive should throw an error, > > if hive allows that command sentry should be handling it. > > 1. update the path information > > 2. change the permission > > Na Li wrote: > The function that processes the event is alterTable(), not > alterDatabase(). If user wants to change the name of a database, the event > handler should call alterDatabase(), not alterTable(). The current processing > assumes that when user calls alter table and the database name is changed, > then all tables in original database should change their database name. I > don't think this assumption is valid. It is very dangerous to do so. > > altertable should only be used to change table, not all tables in a > database. > > NotificationProcessor.processAlterTable does not do this: "when user > calls alter table and the database name is changed, then all tables in > original database should change their database name."
You are right. This part of code seems to be incorrect. you may want to fix it as well in this commit as it is related. - kalyan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65768/#review199197 ----------------------------------------------------------- On March 7, 2018, 10:39 p.m., Na Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65768/ > ----------------------------------------------------------- > > (Updated March 7, 2018, 10:39 p.m.) > > > Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and > Sergio Pena. > > > Repository: sentry > > > Description > ------- > > fix the bug that moves privilege to wrong table when DB name changes in alter > table rename > > > Diffs > ----- > > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java > 7a31a01 > > sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java > c30d982 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java > b410027 > > sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/service/thrift/TestFullUpdateModifier.java > c6be80d > > sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java > 5fe6625 > > > Diff: https://reviews.apache.org/r/65768/diff/3/ > > > Testing > ------- > > unit test succeeded > > > Thanks, > > Na Li > >
