> 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

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."


- Na


-----------------------------------------------------------
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
> 
>

Reply via email to