Re: Review Request 69914: HIVE-21227: HIVE-20776 causes view access regression

2019-02-07 Thread Na Li via Review Board

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

(Updated Feb. 8, 2019, 6:48 a.m.)


Review request for hive and Vihang Karajgaonkar.


Bugs: hive-21227
https://issues.apache.org/jira/browse/hive-21227


Repository: hive-git


Description
---

HIVE-20776 introduces a change that causes regression for view access.

Before the change, a user with select access of a view can get all columns of a 
view with select access of a view that is derived from a partitioned table.

With the change, that user cannot access that view.

The reason is that

When user accesses columns of a view, Hive needs to get the partitions of the 
table that the view is derived from. The user name is the user who issues the 
query to access the view.
The change in HIVE-20776 checks if user has access to a table before getting 
its partitions. When user only has access of a view, not the access of a table 
itself, this change denies the user access of the view.
The solution is when getting table partitions, do not filter on table at HMS 
client


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 a1826fa259d424c9f3d5a2f58a18f617355d586f 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 6a0d0aa8840d9fc6799c16463a70ed6f0cc2c354 


Diff: https://reviews.apache.org/r/69914/diff/2/

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


Testing
---

TestGetPartitions and TestListPartitions pass


Thanks,

Na Li



Re: Review Request 69914: HIVE-21227: HIVE-20776 causes view access regression

2019-02-07 Thread Na Li via Review Board


> On Feb. 7, 2019, 5:48 p.m., Karthik Manamcheri wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Line 2509 (original), 2512 (patched)
> > 
> >
> > We should add unit tests to make sure that this doesn't happen again. 
> > Can you add a test which would fail if this fix wasn't there?

I changed the test on partitions to refect the fact that HMS client does not 
filter partitions' table due to "HIVE-21227: HIVE-20776 causes view access 
regression"


- Na


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


On Feb. 7, 2019, 3:31 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69914/
> ---
> 
> (Updated Feb. 7, 2019, 3:31 a.m.)
> 
> 
> Review request for hive and Vihang Karajgaonkar.
> 
> 
> Bugs: hive-21227
> https://issues.apache.org/jira/browse/hive-21227
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-20776 introduces a change that causes regression for view access.
> 
> Before the change, a user with select access of a view can get all columns of 
> a view with select access of a view that is derived from a partitioned table.
> 
> With the change, that user cannot access that view.
> 
> The reason is that
> 
> When user accesses columns of a view, Hive needs to get the partitions of the 
> table that the view is derived from. The user name is the user who issues the 
> query to access the view.
> The change in HIVE-20776 checks if user has access to a table before getting 
> its partitions. When user only has access of a view, not the access of a 
> table itself, this change denies the user access of the view.
> The solution is when getting table partitions, do not filter on table at HMS 
> client
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  a1826fa259d424c9f3d5a2f58a18f617355d586f 
> 
> 
> Diff: https://reviews.apache.org/r/69914/diff/1/
> 
> 
> Testing
> ---
> 
> TestGetPartitions and TestListPartitions pass
> 
> 
> Thanks,
> 
> Na Li
> 
>



Review Request 69914: HIVE-21227: HIVE-20776 causes view access regression

2019-02-06 Thread Na Li via Review Board

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

Review request for hive and Vihang Karajgaonkar.


Bugs: hive-21227
https://issues.apache.org/jira/browse/hive-21227


Repository: hive-git


Description
---

HIVE-20776 introduces a change that causes regression for view access.

Before the change, a user with select access of a view can get all columns of a 
view with select access of a view that is derived from a partitioned table.

With the change, that user cannot access that view.

The reason is that

When user accesses columns of a view, Hive needs to get the partitions of the 
table that the view is derived from. The user name is the user who issues the 
query to access the view.
The change in HIVE-20776 checks if user has access to a table before getting 
its partitions. When user only has access of a view, not the access of a table 
itself, this change denies the user access of the view.
The solution is when getting table partitions, do not filter on table at HMS 
client


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 a1826fa259d424c9f3d5a2f58a18f617355d586f 


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


Testing
---

TestGetPartitions and TestListPartitions pass


Thanks,

Na Li



Re: Review Request 69831: Port from master to branch-3 HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-24 Thread Na Li via Review Board

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

(Updated Jan. 24, 2019, 8:30 p.m.)


Review request for hive and Vihang Karajgaonkar.


Bugs: hive-20776
https://issues.apache.org/jira/browse/hive-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 40affeff1714eab68800fb2acd191ae5c90097de 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 5739e89bb3bfbb477b0ba7a48094f5ac78ad 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 46a6d532b63a6f1b3a3bc1ac4d272eee775f0ff3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
 a8b6e316da33e6fd4b484534eb8e915d5128a89e 


Diff: https://reviews.apache.org/r/69831/diff/3/

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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69831: Port from master to branch-3 HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-24 Thread Na Li via Review Board

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

(Updated Jan. 24, 2019, 5:19 p.m.)


Review request for hive and Vihang Karajgaonkar.


Bugs: hive-20776
https://issues.apache.org/jira/browse/hive-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 40affeff1714eab68800fb2acd191ae5c90097de 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 5739e89bb3bfbb477b0ba7a48094f5ac78ad 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 46a6d532b63a6f1b3a3bc1ac4d272eee775f0ff3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
 a8b6e316da33e6fd4b484534eb8e915d5128a89e 


Diff: https://reviews.apache.org/r/69831/diff/2/

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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Review Request 69831: Port from master to branch-3 HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-24 Thread Na Li via Review Board

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

Review request for hive and Vihang Karajgaonkar.


Bugs: hive-20776
https://issues.apache.org/jira/browse/hive-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs
-

  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 40affeff1714eab68800fb2acd191ae5c90097de 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 5739e89bb3bfbb477b0ba7a48094f5ac78ad 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 46a6d532b63a6f1b3a3bc1ac4d272eee775f0ff3 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
 a8b6e316da33e6fd4b484534eb8e915d5128a89e 


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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-18 Thread Na Li via Review Board

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




standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
Line 1290 (original), 1290 (patched)


I change the expected exception type to be consistent with 
testListPartitionsAllNoDbName().

Same for test cases for NoTblName


- Na Li


On Jan. 18, 2019, 9:30 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 18, 2019, 9:30 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  19bd9bac84c20f94ac819a80e3cc89e0dc66396d 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  a338bd4032eb746cd541a562ead0fa2caf9e3ed4 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/11/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> add code to enabled/disable filtering at HMS server based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-18 Thread Na Li via Review Board

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

(Updated Jan. 18, 2019, 9:30 p.m.)


Review request for hive, Adam Holley, Karthick Sankarachary, Morio Ramdenbourg, 
Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 19bd9bac84c20f94ac819a80e3cc89e0dc66396d 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
 a338bd4032eb746cd541a562ead0fa2caf9e3ed4 


Diff: https://reviews.apache.org/r/69585/diff/11/

Changes: https://reviews.apache.org/r/69585/diff/10-11/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-18 Thread Na Li via Review Board

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

(Updated Jan. 18, 2019, 3:45 p.m.)


Review request for hive, Adam Holley, Karthick Sankarachary, Morio Ramdenbourg, 
Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 19bd9bac84c20f94ac819a80e3cc89e0dc66396d 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/10/

Changes: https://reviews.apache.org/r/69585/diff/9-10/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-17 Thread Na Li via Review Board


> On Jan. 10, 2019, 11:20 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
> > Lines 254 (patched)
> > 
> >
> > Its not clear why this should throw NoSuchObjectException? Can you 
> > please add a comment. Are we changing the behavior of this API?

We are not changing the behavior of this API. The original behavior could throw 
NoSuchObjectException as well. In this specific test, NoSuchObjectException was 
not thrown when user cannot access a table without my patch because we did not 
configure pre event listener (which checks if the user can access the 
partition's table, and throw NoSuchObjectException when user cannot access 
partition's table). NoSuchObjectException is thrown now because we add checking 
the filter hook, and NoSuchObjectException is thrown when user cannot access a 
table


> On Jan. 10, 2019, 11:20 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
> > Lines 49 (patched)
> > 
> >
> > This test looks very familiar to TestFilterHooks. What is the 
> > difference? If it is different, can you please add some javadoc on the top 
> > to explain what the test is doing. If there is not much difference can we 
> > refactor (or add these tests to TestFilterHooks) to re-use the code instead 
> > of duplicating it?

I remove TestFilterHooks since its test cases are subset of what are in 
TestHiveMetastoreFilterHook


- Na


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


On Jan. 10, 2019, 9:10 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 10, 2019, 9:10 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  748b56b0a268c1ec7dea022722478ec50889c016 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/8/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> add code to enabled/disable filtering at HMS server based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-17 Thread Na Li via Review Board

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

(Updated Jan. 17, 2019, 10:33 p.m.)


Review request for hive, Adam Holley, Karthick Sankarachary, Morio Ramdenbourg, 
Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 19bd9bac84c20f94ac819a80e3cc89e0dc66396d 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/9/

Changes: https://reviews.apache.org/r/69585/diff/8-9/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-10 Thread Na Li via Review Board

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

(Updated Jan. 10, 2019, 9:10 p.m.)


Review request for hive, Adam Holley, Karthick Sankarachary, Morio Ramdenbourg, 
Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 748b56b0a268c1ec7dea022722478ec50889c016 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/8/

Changes: https://reviews.apache.org/r/69585/diff/7-8/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-10 Thread Na Li via Review Board


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
> > Lines 318 (patched)
> > 
> >
> > Don't we need a boolean argument here too to confirm that only server 
> > side filter logic is tested?

No need. Some functions do not call filtering hook, so we cannot call it to 
test filter hook at HMS server. That is why we need input for server or client 
to decide if we should skip some cases.
For partition, all functions call filtering hook, so we don't need to skip some 
cases for testing filter hook at server.


- Na


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


On Jan. 8, 2019, 8:03 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 8, 2019, 8:03 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  748b56b0a268c1ec7dea022722478ec50889c016 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/7/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> add code to enabled/disable filtering at HMS server based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-10 Thread Na Li via Review Board


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4711 (patched)
> > 
> >
> > move this method call below checkLimitNumberOfPartitionsByFilter.

good point!


- Na


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


On Jan. 8, 2019, 8:03 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 8, 2019, 8:03 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  748b56b0a268c1ec7dea022722478ec50889c016 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/7/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> add code to enabled/disable filtering at HMS server based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-10 Thread Na Li via Review Board


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4655 (patched)
> > 
> >
> > same comment as above. not sure if this method is helping much with the 
> > performance here.

same as above explaination


- Na


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


On Jan. 8, 2019, 8:03 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 8, 2019, 8:03 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  748b56b0a268c1ec7dea022722478ec50889c016 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79404a15894aa42f451df5d18ed3e4c 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/7/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> add code to enabled/disable filtering at HMS server based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-10 Thread Na Li via Review Board


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > Thanks for the patch. Do we really need to introduce 
> > authorizeTableForPartitionMetadata in these API calls. For the common case, 
> > it can potentially degrade API performance. For instance, for fetching a 
> > single partition, we are now doing a get_table and then get_partition for 
> > the common case. I think if it is not related to the functionality of this 
> > patch, we should do it in a separate patch with more investigation on its 
> > impact.

I have changed it to avoid using get_table. Can you take a look?


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 217 (patched)
> > 
> >
> > If you want to initialize this member using init() it shouldn't be 
> > static since it relies on the conf object which is not static. Technically, 
> > there is a race-condition in this variable since it is being overwritten 
> > every time init() method is called with the instance specific conf object.

good point. I was thinking all configuration is the same. but it is better to 
let each MetaStore instance has its own value.


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 682 (patched)
> > 
> >
> > Is there a better way to do this? This method is introducing a 
> > additional db call for all the methods for the common case of users having 
> > the required permissions.

This is avoided in new update


> On Jan. 8, 2019, 9:44 p.m., Vihang Karajgaonkar wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4610 (patched)
> > 
> >
> > The original API is fetching only one partition, this method is not 
> > improving performance but rather degrading it since this would do a fetch 
> > table and fetch partition for the most common case. I think we should do 
> > this check only in case of fetching lots of partitions where the cost of 
> > doing one get_table call is relatively low compared to fetching lots of 
> > partitions.

We need this call to authorize the partition access only when user can access 
its table. Sentry does not do any filtering at partition level.

1) With this approach, 
1.1) sentry get privilege info from DB and authorize its table.
1.1.1) If allowed, HMS gets partition from DB, and sentry just returns input
1.1.2) If not allowed, throw exception.

2) Don't filter on table in HMS, but sentry filter on partition in filter hook 
(Your sugestion)
2.1) HMS gets partition object from DB, then call sentry filter on partition
2.2) Sentry gets privilege info from DB and authorize its table
2.2.1) If allowed, Sentry just returns input (sentry does not filter on 
partition itself)
2.2.2) If not allowed, throw exception.

As you can see, approach 1) has less overhead when user cannot access that 
table. I have changed the implementation of authorizeTableForPartitionMetadata, 
so it does not get table object from HMS db.


- Na


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


On Jan. 8, 2019, 8:03 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Jan. 8, 2019, 8:03 p.m.)
> 
> 
> Review request for hive, Adam Holley, Karthick Sankarachary, Morio 
> Ramdenbourg, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  748b56b0a268c1ec7dea022722478ec50889c016 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  a9398ae1e79

Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-08 Thread Na Li via Review Board

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

(Updated Jan. 8, 2019, 8:03 p.m.)


Review request for hive, Adam Holley, Karthick Sankarachary, Morio Ramdenbourg, 
Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 748b56b0a268c1ec7dea022722478ec50889c016 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/7/


Testing (updated)
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration
add code to enabled/disable filtering at HMS server based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2019-01-04 Thread Na Li via Review Board

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

(Updated Jan. 4, 2019, 4:07 p.m.)


Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 748b56b0a268c1ec7dea022722478ec50889c016 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 be1f8c78497fe3d0816ad3935ba07cd5ad379b08 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/7/

Changes: https://reviews.apache.org/r/69585/diff/6-7/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-30 Thread Na Li via Review Board

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

(Updated Dec. 31, 2018, 6:54 a.m.)


Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 748b56b0a268c1ec7dea022722478ec50889c016 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 eb95e129a1a69fdbf9304676f8e8d97c06d48dea 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 a9398ae1e79404a15894aa42f451df5d18ed3e4c 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/6/

Changes: https://reviews.apache.org/r/69585/diff/5-6/


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
> > Lines 662 (patched)
> > 
> >
> > The second parameter in ConfVars corresponds to the Hive property name. 
> > 
> > E.g. metastore.client.filter.result -> 
> > hive.metastore.client.filter.result
> > 
> > So this should be: 
> > METASTORE_CLIENT_FILTER_RESULT("metastore.client.filter.result", 
> > "hive.metastore.client.filter.result" ...)
> 
> Na Li wrote:
> this is new field, and does not exist in hive configuration. Since we are 
> splitting HMS from Hive, does it make sense to add this new one in hive 
> configuration?
> 
> Morio Ramdenbourg wrote:
> Hmm, I'm not exactly sure - HMS is still part of the Hive code base.
> 
> Others who have been adding new properties to only HMS have been putting 
> Hive name, so I think it's safe. Someone should confirm this though.
> 
> Na Li wrote:
> I see both examples in MetastoreConf.java. I will talk with others to 
> find out the rule.
> 
> 1) hiveName is "hive." + varname
> ADDED_JARS("metastore.added.jars.path", "hive.added.jars.path", "",
> "This an internal parameter."),
> 
> 
> 2) hiveName is the same as varname
> CATALOG_DEFAULT("metastore.catalog.default", 
> "metastore.catalog.default", "hive",
> "The default catalog to use when a catalog is not specified.  
> Default is 'hive' (the " +
> "default catalog)."),

I changed the hiveName to start with "hive."


- Na


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


On Dec. 22, 2018, 12:27 a.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 22, 2018, 12:27 a.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
>  7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/5/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board

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

(Updated Dec. 22, 2018, 12:27 a.m.)


Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestFilterHooks.java
 7dc69bc4e92875c8962dcd313b16f0f90ea8b057 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/5/

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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4621 (patched)
> > 
> >
> > This is the same method as the one on the HiveMetaStoreClient. Would it 
> > be possible to move it to a common method under 
> > hive-standalone-metastore-common?
> 
> Na Li wrote:
> This function authorize access to the table of the partitions. Sergion is 
> going to implement this when filtering partitions in SENTRY-2481. So I will 
> remove this function in HMS client and server

I moved it to common untility class


- Na


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


On Dec. 21, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 21, 2018, 9:36 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/4/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 1405 (patched)
> > 
> >
> > I thought we were going to use the PreReadEvent instead. Also, I'm 
> > concerned the implementation of filterDatabase does not throw 
> > NoSuchObjectException, and instead returns null. We should check both cases 
> > to guarantee get_database and others methods will deny the access to this 
> > database.
> > 
> > I think we should use the PreReadEvent which is meant for authorization 
> > hooks.
> 
> Na Li wrote:
> The PreReadEvent calls sentry MetastoreAuthzBindingBase for 
> authorization. As you can see in
> 
> https://github.com/apache/sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java#L169,
>  it does not do anything on reading metadata. Therefore, we have to use 
> filter to prevent user from reading metadata it does not have privileges. 
> 
> I can only remove the filter for get_database() or get_table() if we 
> change sentry's implementation for that file.

for get_table(), get_database(), get_parittion*(), I remove the filter check, 
and use firePreEvent() to verify that user can access the metadata.

When getting list of names, we only call filter hook to remove names that user 
should not see.


Accessing metadata of an object requires more privileges than just getting the 
names. For example, "CREATE" privilege on server allows listing DB names, so 
user does not create a DB using a name of an existing DB.  "CREATE" privilege 
on DB allows listing Table names. But "CREATE" privilege does not allow 
describe a particular table (show meta data of a table user has no privilege) 
regardless it is for authorization or filtering. So we should only check 
authorization (firePreEvent()) for single item like get_database() or 
get_table(). For getting a list of item names, we only call filter hook.


- Na


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


On Dec. 21, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 21, 2018, 9:36 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/4/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
> > Lines 662 (patched)
> > 
> >
> > The second parameter in ConfVars corresponds to the Hive property name. 
> > 
> > E.g. metastore.client.filter.result -> 
> > hive.metastore.client.filter.result
> > 
> > So this should be: 
> > METASTORE_CLIENT_FILTER_RESULT("metastore.client.filter.result", 
> > "hive.metastore.client.filter.result" ...)
> 
> Na Li wrote:
> this is new field, and does not exist in hive configuration. Since we are 
> splitting HMS from Hive, does it make sense to add this new one in hive 
> configuration?
> 
> Morio Ramdenbourg wrote:
> Hmm, I'm not exactly sure - HMS is still part of the Hive code base.
> 
> Others who have been adding new properties to only HMS have been putting 
> Hive name, so I think it's safe. Someone should confirm this though.

I see both examples in MetastoreConf.java. I will talk with others to find out 
the rule.

1) hiveName is "hive." + varname
ADDED_JARS("metastore.added.jars.path", "hive.added.jars.path", "",
"This an internal parameter."),


2) hiveName is the same as varname
CATALOG_DEFAULT("metastore.catalog.default", "metastore.catalog.default", 
"hive",
"The default catalog to use when a catalog is not specified.  Default 
is 'hive' (the " +
"default catalog)."),


- Na


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


On Dec. 21, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 21, 2018, 9:36 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/4/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 816 (patched)
> > 
> >
> > Nit: This might benefit from some extra spacing or spreading them out 
> > into if statements. I had a difficult time reading the nested ternary 
> > operators (or maybe that's just me?)

I wrap it in a function. So it will be easier to read.


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 2458 (patched)
> > 
> >
> > This is the same method as the one on the HiveMetaStore. Would it be 
> > possible to move it to a common method under 
> > hive-standalone-metastore-common?
> 
> Na Li wrote:
> some variables used are different. such as isClientFilterEnabled vs 
> isServerFilterEnabled and filterHook. If we make is generate function, those 
> need to be input. No sure how valuable to make that change.
> 
> Morio Ramdenbourg wrote:
> Ah, didn't notice those. If that's the case, I'm not sure either how 
> valuable the generic method would be. Could probably drop this recommendation

I decide to have a common method under hive-standalone-metastore-common


- Na


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


On Dec. 21, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 21, 2018, 9:36 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/4/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 4621 (patched)
> > 
> >
> > This is the same method as the one on the HiveMetaStoreClient. Would it 
> > be possible to move it to a common method under 
> > hive-standalone-metastore-common?

This function authorize access to the table of the partitions. Sergion is going 
to implement this when filtering partitions in SENTRY-2481. So I will remove 
this function in HMS client and server


- Na


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


On Dec. 21, 2018, 9:36 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 21, 2018, 9:36 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/4/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board

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

(Updated Dec. 21, 2018, 9:36 p.m.)


Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FilterUtils.java
 PRE-CREATION 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/4/

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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 2934 (patched)
> > 
> >
> > What is this extra space?

just make it easier to read. I will remove it.


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 2910-2911 (original), 2943-2946 (patched)
> > 
> >
> > Why is this change needed? I don't see anything new except splitting it 
> > in two lines. This will stay in the git history.

I will revert the change


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Line 2931 (original), 2966-2970 (patched)
> > 
> >
> > Notice the firePreEvent before the filter. If we use that for 
> > authorization checks, then the filter is not required here.

I will remove the call for filterhook. But Sentry needs to implement 
authorization in PreEvent listener for read operation for HMS


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 3050-3053 (patched)
> > 
> >
> > Same question, why splitting the lines where if this patch doesn't need 
> > it?

will remove it


- Na


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


On Dec. 19, 2018, 10:50 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 10:50 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/3/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-21 Thread Na Li via Review Board


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
> > Lines 220-221 (patched)
> > 
> >
> > These two variables are not necessary. To disable the filter, the admin 
> > can just remove the filter hooks from the configuration and voilĂ , filter 
> > disabled. This applies to both sides, server-side and client-side.

removing the filter hooks is more like a workaround. It is more straightforward 
to keep the hook configuration, and have specific configuration to 
enable/disable filtering at HMS client and server.


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/pom.xml
> > Lines 242-246 (patched)
> > 
> >
> > Why is this dep necessary? I wonder if this conflicts with the 
> > standalone solution where all the Hive dependences were removed.

It is not needed. I have removed it.


> On Dec. 20, 2018, 2:46 p.m., Sergio Pena wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 1405 (patched)
> > 
> >
> > I thought we were going to use the PreReadEvent instead. Also, I'm 
> > concerned the implementation of filterDatabase does not throw 
> > NoSuchObjectException, and instead returns null. We should check both cases 
> > to guarantee get_database and others methods will deny the access to this 
> > database.
> > 
> > I think we should use the PreReadEvent which is meant for authorization 
> > hooks.

The PreReadEvent calls sentry MetastoreAuthzBindingBase for authorization. As 
you can see in
https://github.com/apache/sentry/blob/master/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/MetastoreAuthzBindingBase.java#L169,
 it does not do anything on reading metadata. Therefore, we have to use filter 
to prevent user from reading metadata it does not have privileges. 

I can only remove the filter for get_database() or get_table() if we change 
sentry's implementation for that file.


- Na


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


On Dec. 19, 2018, 10:50 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 10:50 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/3/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-20 Thread Na Li via Review Board


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
> > Lines 2458 (patched)
> > 
> >
> > This is the same method as the one on the HiveMetaStore. Would it be 
> > possible to move it to a common method under 
> > hive-standalone-metastore-common?

some variables used are different. such as isClientFilterEnabled vs 
isServerFilterEnabled and filterHook. If we make is generate function, those 
need to be input. No sure how valuable to make that change.


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
> > Lines 662 (patched)
> > 
> >
> > The second parameter in ConfVars corresponds to the Hive property name. 
> > 
> > E.g. metastore.client.filter.result -> 
> > hive.metastore.client.filter.result
> > 
> > So this should be: 
> > METASTORE_CLIENT_FILTER_RESULT("metastore.client.filter.result", 
> > "hive.metastore.client.filter.result" ...)

this is new field, and does not exist in hive configuration. Since we are 
splitting HMS from Hive, does it make sense to add this new one in hive 
configuration?


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-server/pom.xml
> > Lines 242 (patched)
> > 
> >
> > Nit: Won't this add a Hive dependency to the standalone metastore 
> > server? 
> > 
> > May need to modify this comment here: 
> > https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/pom.xml#L158

good catch. I have removed this dependency. It is not needed.


> On Dec. 19, 2018, 11:33 p.m., Morio Ramdenbourg wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 78 (patched)
> > 
> >
> > I don't think this is being used anywhere, could probably remove

Yes. I removed it.


- Na


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


On Dec. 19, 2018, 10:50 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 10:50 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/3/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-19 Thread Na Li via Review Board

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

(Updated Dec. 19, 2018, 10:50 p.m.)


Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
  standalone-metastore/metastore-server/pom.xml 
895abfc423f00b121ee63e40904f5b3e57aea8ed 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/3/

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


Testing
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-19 Thread Na Li via Review Board


> On Dec. 19, 2018, 4:43 p.m., Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 5235 (patched)
> > 
> >
> > Can we make a helper fuction called "filterTablesIfEnabled"?

Done.


- Na


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


On Dec. 19, 2018, 5:07 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 5:07 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/2/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-19 Thread Na Li via Review Board


> On Dec. 19, 2018, 4:43 p.m., Karthik Manamcheri wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
> > Lines 2956 (patched)
> > 
> >
> > All of this logic of endFunction seems to be inside getTableInternal. 
> > Why duplicate it here?

I wanted to show the filtering overhead. That is why. Now I move the filter 
code in getTableInternal(), and remove the added code in get_table()


- Na


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


On Dec. 19, 2018, 5:07 p.m., Na Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69585/
> ---
> 
> (Updated Dec. 19, 2018, 5:07 p.m.)
> 
> 
> Review request for hive, Adam Holley, Morio Ramdenbourg, Peter Vary, Sergio 
> Pena, and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20776
> https://issues.apache.org/jira/browse/HIVE-20776
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> add filtering to read result at HMS server, so user cannot see metadata 
> he/she has no privileges. Filtering is enabled/disabled based on 
> configuration.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
>   
> standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
>   standalone-metastore/metastore-server/pom.xml 
> 895abfc423f00b121ee63e40904f5b3e57aea8ed 
>   
> standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
>   
> standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/69585/diff/2/
> 
> 
> Testing
> ---
> 
> Existing unit tests passed. 
> add new unit tests for filtering at HMS server and HMS client
> add code to enabled/disable filtering at HMS client based on configuration
> 
> 
> Thanks,
> 
> Na Li
> 
>



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-18 Thread Na Li via Review Board

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

(Updated Dec. 19, 2018, 6:57 a.m.)


Review request for hive, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
  standalone-metastore/metastore-server/pom.xml 
895abfc423f00b121ee63e40904f5b3e57aea8ed 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/2/


Testing (updated)
---

Existing unit tests passed. 
add new unit tests for filtering at HMS server and HMS client
add code to enabled/disable filtering at HMS client based on configuration


Thanks,

Na Li



Re: Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-18 Thread Na Li via Review Board

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

(Updated Dec. 19, 2018, 6:56 a.m.)


Review request for hive, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs (updated)
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 9eb1193a27120b5167f92daf67bf6a1c4e1d9927 
  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe6fb9fd4b4c92a6a39f06f39a4641aaabd 
  standalone-metastore/metastore-server/pom.xml 
895abfc423f00b121ee63e40904f5b3e57aea8ed 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96dcf62d3536cab2ce074d27a6225b2d3443 
  
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreFilterHook.java
 PRE-CREATION 


Diff: https://reviews.apache.org/r/69585/diff/2/

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


Testing
---

Existing unit tests passed. 
Will add new unit tests for filtering at HMS server in next update of this jira
Will add code to enabled/disable filtering at HMS client based on configuration 
in next update of this jira


Thanks,

Na Li



Review Request 69585: HIVE-20776: Run HMS filterHooks on server-side in addition to client-side

2018-12-18 Thread Na Li via Review Board

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

Review request for hive, Peter Vary, Sergio Pena, and Vihang Karajgaonkar.


Bugs: HIVE-20776
https://issues.apache.org/jira/browse/HIVE-20776


Repository: hive-git


Description
---

add filtering to read result at HMS server, so user cannot see metadata he/she 
has no privileges. Filtering is enabled/disabled based on configuration.


Diffs
-

  
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
 fb0b2fe 
  standalone-metastore/metastore-server/pom.xml 895abfc 
  
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 0a1b96d 


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


Testing
---

Existing unit tests passed. 
Will add new unit tests for filtering at HMS server in next update of this jira
Will add code to enabled/disable filtering at HMS client based on configuration 
in next update of this jira


Thanks,

Na Li



Re: Review Request 65985: HIVE-18783: ALTER TABLE post-commit listener does not include the transactional listener responses

2018-03-12 Thread Na Li via Review Board

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
Lines 372 (patched)


why when db name is changed at alter table,  the 
transactionalListenerResponses is not past into 
MetaStoreListenerNotifier.notifyEvent()? And the command won't be able to block 
until sentry gets the notification of alter table.


- Na Li


On March 8, 2018, 4:11 p.m., Sergio Pena wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65985/
> ---
> 
> (Updated March 8, 2018, 4:11 p.m.)
> 
> 
> Review request for hive, Alexander Kolbasov, Sahil Takiar, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18783
> https://issues.apache.org/jira/browse/HIVE-18783
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16164 introduced a mechanism to pass HMS notification events ID to the 
> post-commit listeners for all DDL operations, but it didn't add it to the 
> ALTER TABLE event. This patch in review adds the same behavior for ALTER 
> TABLE events.
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  e0e29652da94bbdaca515a17955d1409824c1742 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
>  89354a2d34249903a9ff13c4ed913a68de93057e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  662de9a66767f27f31998f14c68f854e59993ab6 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IHMSHandler.java
>  e6de0013bc1be12b2772e2e97102ed476cf5 
> 
> 
> Diff: https://reviews.apache.org/r/65985/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Sergio Pena
> 
>