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




sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
Lines 135-136 (patched)
<https://reviews.apache.org/r/65704/#comment278239>

    We should also check that tableEvent and the getOldTable and getNewTable 
values are not null. These are values gotten from Hive but if Hive has a bug, 
then we will have NPE errors here.
    
    Also, have you considered the use of Preconditions.checkNotNull() instead 
of an if() statement? We are not expecting null values, so these would work to 
check for not nulls.



sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
Lines 2412-2413 (patched)
<https://reviews.apache.org/r/65704/#comment278233>

    Is this bug part of the patch? The jira is to sync alter table renames 
only, this is reading privileges which do not match with the jira description.


- Sergio Pena


On Feb. 22, 2018, 1:28 a.m., Na Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65704/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2018, 1:28 a.m.)
> 
> 
> Review request for sentry, Alexander Kolbasov, kalyan kumar kalvagadda, and 
> Sergio Pena.
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> wait for HMS sync at alter table, which including table rename and changing 
> table columns
> 
> 
> Diffs
> -----
> 
>   
> sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java
>  24d7763 
>   
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java
>  edea5b6 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java
>  5fe6625 
>   
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java
>  4cd00e6 
> 
> 
> Diff: https://reviews.apache.org/r/65704/diff/3/
> 
> 
> Testing
> -------
> 
> unit tests succeeded
> 
> 
> Thanks,
> 
> Na Li
> 
>

Reply via email to