You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Na Li via Review Board <no...@reviews.apache.org> on 2018/02/23 06:51:52 UTC

Review Request 65768: SENTRY-2144: Table Rename should allow database name change

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/
-----------------------------------------------------------

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-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java 5fe6625 


Diff: https://reviews.apache.org/r/65768/diff/1/


Testing
-------

unit test succeeded


Thanks,

Na Li


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.

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


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.

> 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.
> 
> kalyan kumar kalvagadda wrote:
>     Lina, what is the latest on this? you you planning to fix the code in FullUpdateModifier.java?

I have removed the code to change all tables in db in alter table processing.


- Na


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/#review199197
-----------------------------------------------------------


On April 10, 2018, 7:22 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65768/
> -----------------------------------------------------------
> 
> (Updated April 10, 2018, 7:22 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 83c0fc4 
>   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/4/
> 
> 
> Testing
> -------
> 
> unit test succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.

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


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.

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


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/#review199197
-----------------------------------------------------------



Lina, Don't we need changes to notifcation processor to handle this?


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/#comment279491>

    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


- kalyan kumar kalvagadda


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


Re: Review Request 65768: SENTRY-2144: Table Rename Cross Database should update permission correctly

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/#review203456
-----------------------------------------------------------


Ship it!




Looks Good.

- kalyan kumar kalvagadda


On May 18, 2018, 9:23 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65768/
> -----------------------------------------------------------
> 
> (Updated May 18, 2018, 9:23 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
> 
> This fix only handles notification event processing when there is no full snapshot. The changes related to update to full snapshot is tracked in "SENTRY-2239 FullUpdateModifier does not handle alter table rename correctly"
> 
> 
> Diffs
> -----
> 
>   sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cafe2b5 
>   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/5/
> 
> 
> Testing
> -------
> 
> unit test succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 65768: SENTRY-2144: Table Rename Cross Database should update permission correctly

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/
-----------------------------------------------------------

(Updated May 18, 2018, 9:23 p.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Sergio Pena.


Summary (updated)
-----------------

SENTRY-2144: Table Rename Cross Database should update permission correctly


Repository: sentry


Description
-------

fix the bug that moves privilege to wrong table when DB name changes in alter table rename

This fix only handles notification event processing when there is no full snapshot. The changes related to update to full snapshot is tracked in "SENTRY-2239 FullUpdateModifier does not handle alter table rename correctly"


Diffs
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cafe2b5 
  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/5/


Testing
-------

unit test succeeded


Thanks,

Na Li


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/
-----------------------------------------------------------

(Updated May 18, 2018, 8:03 p.m.)


Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and Sergio Pena.


Repository: sentry


Description (updated)
-------

fix the bug that moves privilege to wrong table when DB name changes in alter table rename

This fix only handles notification event processing when there is no full snapshot. The changes related to update to full snapshot is tracked in "SENTRY-2239 FullUpdateModifier does not handle alter table rename correctly"


Diffs (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java cafe2b5 
  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/5/

Changes: https://reviews.apache.org/r/65768/diff/4-5/


Testing
-------

unit test succeeded


Thanks,

Na Li


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by kalyan kumar kalvagadda via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/#review200840
-----------------------------------------------------------




sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/FullUpdateModifier.java
Lines 364-379 (original), 364-373 (patched)
<https://reviews.apache.org/r/65768/#comment281745>

    I don't quite understand this logic.
    
    alter table could do couple of things
    1. alter the table name (alter table db1.tb1 rename to db1.tb2)
    2. move the table from one database to another one (alter table db1.tb2 rename to db2.tb2;)
    3. both (alter table db1.tb2 rename to db2.tb3)
    
    From what I understand, difference in prevDbName and newDbName means that table is moved from one database to another.


- kalyan kumar kalvagadda


On April 10, 2018, 7:22 p.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65768/
> -----------------------------------------------------------
> 
> (Updated April 10, 2018, 7:22 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 83c0fc4 
>   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/4/
> 
> 
> Testing
> -------
> 
> unit test succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/
-----------------------------------------------------------

(Updated April 10, 2018, 7:22 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 (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 83c0fc4 
  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/4/

Changes: https://reviews.apache.org/r/65768/diff/3-4/


Testing
-------

unit test succeeded


Thanks,

Na Li


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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 (updated)
-----

  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/

Changes: https://reviews.apache.org/r/65768/diff/2-3/


Testing
-------

unit test succeeded


Thanks,

Na Li


Re: Review Request 65768: SENTRY-2144: Table Rename should allow database name change

Posted by Na Li via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65768/
-----------------------------------------------------------

(Updated March 7, 2018, 7:16 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 (updated)
-----

  sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java 7a31a01 
  sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java b410027 
  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/2/

Changes: https://reviews.apache.org/r/65768/diff/1-2/


Testing
-------

unit test succeeded


Thanks,

Na Li