> 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."
> 
> kalyan kumar kalvagadda wrote:
>     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.

Lina, what is the latest on this? you you planning to fix the code in 
FullUpdateModifier.java?


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

Reply via email to