Re: Review Request 69462: HIVE-20936

2018-12-21 Thread Eugene Koifman 

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


Ship it!




Ship It!

- Eugene Koifman


On Dec. 21, 2018, 4:30 p.m., Jaume Marhuenda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69462/
> ---
> 
> (Updated Dec. 21, 2018, 4:30 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Allow the Worker thread in the metastore to run outside of it
> 
> 
> Diffs
> -
> 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
>  b290a40734 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  d3800cdf2a 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 852942e6a2 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 18253c9bab 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 42ce1746fd 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java 
> f5b901d6e8 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> cdcc0e9548 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MetaStoreCompactorThread.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/RemoteCompactorThread.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 49662cd68b 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java a3034fb195 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 287aeaecb0 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 0c55654475 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java
>  d85dda5acd 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java
>  3eb55b1b59 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java
>  17f8b7730a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java
>  f2f8fb475e 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java
>  f7e188dfda 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java
>  bd38bbe45d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java
>  fb591dcec5 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java
>  e8dfba523d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java
>  3d32f372d6 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java
>  2b176efee4 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java
>  c0fe726f8a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java
>  db91e0bf89 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java
>  d26cde23fc 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java
>  3db9095b5c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java
>  c3f71fe13e 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java
>  5716922bd3 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java
>  3ef24310b2 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java
>  77e57718ef 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java
>  f559a03b92 
>   
>

Re: Review Request 69462: HIVE-20936

2018-12-21 Thread Jaume Marhuenda 


> On Dec. 21, 2018, 8:57 p.m., Eugene Koifman wrote:
> > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
> > Line 34 (original), 42 (patched)
> > 
> >
> > Why does this have more state fields than CompactorInfoStruct?  Perhaps 
> > it can done in HIVE-21056

Not all fields are serialized because not all are needed but I agree it's a 
good idea to so.


- Jaume


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


On Dec. 22, 2018, 12:30 a.m., Jaume Marhuenda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69462/
> ---
> 
> (Updated Dec. 22, 2018, 12:30 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Allow the Worker thread in the metastore to run outside of it
> 
> 
> Diffs
> -
> 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
>  b290a40734 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  d3800cdf2a 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 852942e6a2 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 18253c9bab 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 42ce1746fd 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java 
> f5b901d6e8 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> cdcc0e9548 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MetaStoreCompactorThread.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/RemoteCompactorThread.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 49662cd68b 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java a3034fb195 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 287aeaecb0 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 0c55654475 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java
>  d85dda5acd 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java
>  3eb55b1b59 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java
>  17f8b7730a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java
>  f2f8fb475e 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java
>  f7e188dfda 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java
>  bd38bbe45d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java
>  fb591dcec5 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java
>  e8dfba523d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java
>  3d32f372d6 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java
>  2b176efee4 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java
>  c0fe726f8a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java
>  db91e0bf89 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java
>  d26cde23fc 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java
>  3db9095b5c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java
>  c3f71fe13e 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java
>  5716922bd3 
>

Re: Review Request 69462: HIVE-20936

2018-12-21 Thread Jaume Marhuenda 

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

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


Review request for hive.


Repository: hive-git


Description
---

Allow the Worker thread in the metastore to run outside of it


Diffs (updated)
-

  
hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
 b290a40734 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
 d3800cdf2a 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 852942e6a2 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 18253c9bab 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
42ce1746fd 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java 
f5b901d6e8 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java cdcc0e9548 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MetaStoreCompactorThread.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/RemoteCompactorThread.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 49662cd68b 
  ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java a3034fb195 
  ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 287aeaecb0 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 0c55654475 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java
 d85dda5acd 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java
 3eb55b1b59 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java
 17f8b7730a 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
 PRE-CREATION 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java
 f2f8fb475e 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java
 f7e188dfda 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java
 bd38bbe45d 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java
 fb591dcec5 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java
 e8dfba523d 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java
 3d32f372d6 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java
 2b176efee4 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java
 c0fe726f8a 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java
 db91e0bf89 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java
 d26cde23fc 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java
 3db9095b5c 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesRequest.java
 c3f71fe13e 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetTablesResult.java
 5716922bd3 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/InsertEventRequestData.java
 3ef24310b2 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventRequest.java
 77e57718ef 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEventResponse.java
 f559a03b92 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/OptionalCompactionInfoStruct.java
 PRE-CREATION 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/PutFileMetadataRequest.java
 0bbb9be468 
  
standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java
 0a0393ff14

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 Karthik Manamcheri 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.
> 
> Na Li wrote:
> 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.

If we remote the filter hooks we'd be removing it from both server and client. 
What we want is to be able to control one or the other. What if the server 
filtering causes a huge performance degradation for a particular use case? In 
that case I'd want to be able to turn off server side filtering only. That's 
why we need these two configs.


- Karthik


---
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 69462: HIVE-20936

2018-12-21 Thread Eugene Koifman 

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



it looks like it has merge conflits


standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java
Line 34 (original), 42 (patched)


Why does this have more state fields than CompactorInfoStruct?  Perhaps it 
can done in HIVE-21056



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java
Lines 586 (patched)


"rj.getID().toString()" shouldn't be inside the quotes



ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java
Lines 216 (patched)


this seems unused anywhere


- Eugene Koifman


On Dec. 20, 2018, 5:02 p.m., Jaume Marhuenda wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69462/
> ---
> 
> (Updated Dec. 20, 2018, 5:02 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Allow the Worker thread in the metastore to run outside of it
> 
> 
> Diffs
> -
> 
>   
> hcatalog/streaming/src/test/org/apache/hive/hcatalog/streaming/TestStreaming.java
>  b290a40734 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCompactor.java
>  5af047f465 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Cleaner.java 18253c9bab 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java 
> 42ce1746fd 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorThread.java 
> f5b901d6e8 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java 
> cdcc0e9548 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/MetaStoreCompactorThread.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/RemoteCompactorThread.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Worker.java 49662cd68b 
>   ql/src/test/org/apache/hadoop/hive/ql/TestTxnCommands2.java a3034fb195 
>   ql/src/test/org/apache/hadoop/hive/ql/TxnCommandsBaseForTests.java 
> 287aeaecb0 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 0c55654475 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java
>  d85dda5acd 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClearFileMetadataRequest.java
>  3eb55b1b59 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ClientCapabilities.java
>  17f8b7730a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java
>  f2f8fb475e 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FireEventRequest.java
>  f7e188dfda 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetAllFunctionsResponse.java
>  bd38bbe45d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprRequest.java
>  fb591dcec5 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataByExprResult.java
>  e8dfba523d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataRequest.java
>  3d32f372d6 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetFileMetadataResult.java
>  2b176efee4 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java
>  c0fe726f8a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java
>  db91e0bf89 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java
>  d26cde23fc 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java
>  3db9095b5c 
>   
>

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

2018-12-21 Thread Morio Ramdenbourg 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?
> 
> 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.

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


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

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.


- Morio


---
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 69562: HIVE-16957

2018-12-21 Thread Jesús Camacho Rodríguez 

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

(Updated Dec. 21, 2018, 8:30 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description
---

HIVE-16957


Diffs (updated)
-

  itests/hive-unit/src/test/java/org/apache/hadoop/hive/hooks/TestHs2Hooks.java 
d26af3b08130ce26006cc57c53e68efca1d01166 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/LimitPushdownOptimizer.java 
859c18f3c2059a8e0d4e3fd7f62a521a72e691fd 
  ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
3a51d9795b0384356daa0a8ab576374fb05c3378 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
11ccff44588e20d6acc47af31bfa05e3beba7e2e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
9aff0069fd0170cfec877caf481e8b6653435b81 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
5126a7915ddac7b2073356ac2f993470db2f0a94 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
f0f7b18d192f85b489ccde4e8a80e92dc11a0494 
  ql/src/test/queries/clientpositive/cbo_rp_cross_product_check_2.q 
00c19c74ad45fc13e0a2cf74af3f0fb33b73a1a3 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite.q 
9735e61598520469f176719bc51b4437204fd522 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_2.q 
3f695d1ee212902a0415ac2912a4f15d521cd380 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_3.q 
eb668a90acb546504cffb994ce25a1ab03c5b0c0 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q 
f21db8a8d87fe47eb22a3c43d0856dd5463a1671 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q 
3026d9093eddbf53611a79df5fbe1ee55884273a 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_dummy.q 
8c9da8ae69967d1e11a333a46fddc51957cd5f31 
  
ql/src/test/queries/clientpositive/materialized_view_create_rewrite_multi_db.q 
85d926f9eb8c40d01bee6dab87baf5bf29790278 
  
ql/src/test/queries/clientpositive/materialized_view_create_rewrite_rebuild_dummy.q
 72e3d65117c0712929bb217e3a5e769101b27ebd 
  
ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window.q
 4cdb715d2873b726561979a5b6d93c086467cd3d 
  
ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window_2.q
 6873673a55580b3d94f2af4b7c7c0b20f191d879 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
18b9f7d418eff200d551ce4f95a399741dfca3f8 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_10.q 
95427923164a28b1b0ef73f03fa69544daa5ff1c 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q 
3a447fc1873bf98d748fb9fd09278d60b7c9ac55 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
0823f59394dd00d9d02b0ae517454c035b21baed 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
6724cec7710f981d094576f6befccd2491ebe936 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
d87928c07363f6bd0ba4ae8f1986f5f53f513731 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
23fc3c14ce5a0c43483824de9ac0835453f74c44 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
3d1cedc4f56a1bceecba390292fcd26ad6ce1863 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q 
cfcfddce506d80ad55ac4d61dc0c8069b354c5cd 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q 
18d5cec8f98fdc7c44d3a358c765ab111019941d 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_empty.q 
9ae1d4e81b033a07cf38bb4900cb819d34bf3d3f 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt.q 
8de9c7087a80b0bd5a5a68a46f336390b02a33ff 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_no_join_opt_2.q 
a1372301feb27e0845a6dd8e260ba18bd91f03fd 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_1.q 
e6980c07f130a44ae4ade0a36e5591d75999eb6f 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_part_2.q 
b2e6ebd6956c945b454a8646e32688374522326c 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_ssb.q 
aed5bdbffdd225ee3d462b407d289205ff9775c8 
  ql/src/test/queries/clientpositive/materialized_view_rewrite_ssb_2.q 
0982b66ad7841272f234608a88cbe8e255cb3bfd 
  ql/src/test/results/clientnegative/masking_mv.q.out 
54e984321209b4893f0178d49bdc66aebc38ff44 
  ql/src/test/results/clientpositive/alter_table_update_status.q.out 
ec8a64cd65c77592fae39c1b251d4d65837043a8 
  
ql/src/test/results/clientpositive/alter_table_update_status_disable_bitvector.q.out
 1b787af0a884ff4f27415b8fc5cc1593c29e7190 
  ql/src/test/results/clientpositive/autoColumnStats_4.q.out 
83ee0f76da409f525c00aa932e40cf8d5d5630be

Re: Review Request 69562: HIVE-16957

2018-12-21 Thread Jesús Camacho Rodríguez 


> On Dec. 21, 2018, 5:12 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java
> > Lines 110 (patched)
> > 
> >
> > It will be good to add a comment here stating why we need to use table 
> > values here.

Done!


> On Dec. 21, 2018, 5:12 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
> > Lines 7719 (patched)
> > 
> >
> > Now that we support partitioned CTAS, need to pass in partSpec here and 
> > handle it.

Since CTAS does not allow static partition values, we pass null for partSpec 
(at that moment, partSpec map would only contain the static partition values). 
Then, _insertTableValuesAnalyzePipeline_ is responsible to introduce 
column_partition_name=null in the partSpec for each partition column, similar 
to the logic used for INSERT or INSERT OVERWRITE statements. Those column 
partition values will be bounded dynamically at CTAS execution time.


> On Dec. 21, 2018, 5:12 p.m., Ashutosh Chauhan wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java
> > Lines 415 (patched)
> > 
> >
> > will be good to add a comment for this.

Done!


- Jesús


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


On Dec. 13, 2018, 4:50 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69562/
> ---
> 
> (Updated Dec. 13, 2018, 4:50 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-16957
> https://issues.apache.org/jira/browse/HIVE-16957
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16957
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/hooks/TestHs2Hooks.java 
> d26af3b08130ce26006cc57c53e68efca1d01166 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/LimitPushdownOptimizer.java 
> 859c18f3c2059a8e0d4e3fd7f62a521a72e691fd 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 3a51d9795b0384356daa0a8ab576374fb05c3378 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
> 11ccff44588e20d6acc47af31bfa05e3beba7e2e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 9aff0069fd0170cfec877caf481e8b6653435b81 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> b330d710a185aa44c4a89088bb025ecb28ba8856 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
> f0f7b18d192f85b489ccde4e8a80e92dc11a0494 
>   ql/src/test/queries/clientpositive/cbo_rp_cross_product_check_2.q 
> 00c19c74ad45fc13e0a2cf74af3f0fb33b73a1a3 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite.q 
> 9735e61598520469f176719bc51b4437204fd522 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_2.q 
> 3f695d1ee212902a0415ac2912a4f15d521cd380 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_3.q 
> eb668a90acb546504cffb994ce25a1ab03c5b0c0 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q 
> f21db8a8d87fe47eb22a3c43d0856dd5463a1671 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q 
> 3026d9093eddbf53611a79df5fbe1ee55884273a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_dummy.q 
> 8c9da8ae69967d1e11a333a46fddc51957cd5f31 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_multi_db.q
>  85d926f9eb8c40d01bee6dab87baf5bf29790278 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_rebuild_dummy.q
>  72e3d65117c0712929bb217e3a5e769101b27ebd 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window.q
>  4cdb715d2873b726561979a5b6d93c086467cd3d 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window_2.q
>  6873673a55580b3d94f2af4b7c7c0b20f191d879 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
> 18b9f7d418eff200d551ce4f95a399741dfca3f8 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_10.q 
> 95427923164a28b1b0ef73f03fa69544daa5ff1c 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q 
> 3a447fc1873bf98d748fb9fd09278d60b7c9ac55 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
> 0823f59394dd00d9d02b0ae517454c035b21baed 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
>

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 69562: HIVE-16957

2018-12-21 Thread Ashutosh Chauhan 

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




ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java
Lines 110 (patched)


It will be good to add a comment here stating why we need to use table 
values here.



ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java
Lines 7719 (patched)


Now that we support partitioned CTAS, need to pass in partSpec here and 
handle it.



ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java
Lines 415 (patched)


will be good to add a comment for this.


- Ashutosh Chauhan


On Dec. 13, 2018, 4:50 p.m., Jesús Camacho Rodríguez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69562/
> ---
> 
> (Updated Dec. 13, 2018, 4:50 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-16957
> https://issues.apache.org/jira/browse/HIVE-16957
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16957
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/hooks/TestHs2Hooks.java 
> d26af3b08130ce26006cc57c53e68efca1d01166 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/LimitPushdownOptimizer.java 
> 859c18f3c2059a8e0d4e3fd7f62a521a72e691fd 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 3a51d9795b0384356daa0a8ab576374fb05c3378 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsAutoGatherContext.java 
> 11ccff44588e20d6acc47af31bfa05e3beba7e2e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/parse/ColumnStatsSemanticAnalyzer.java 
> 9aff0069fd0170cfec877caf481e8b6653435b81 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
> b330d710a185aa44c4a89088bb025ecb28ba8856 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
> f0f7b18d192f85b489ccde4e8a80e92dc11a0494 
>   ql/src/test/queries/clientpositive/cbo_rp_cross_product_check_2.q 
> 00c19c74ad45fc13e0a2cf74af3f0fb33b73a1a3 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite.q 
> 9735e61598520469f176719bc51b4437204fd522 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_2.q 
> 3f695d1ee212902a0415ac2912a4f15d521cd380 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_3.q 
> eb668a90acb546504cffb994ce25a1ab03c5b0c0 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q 
> f21db8a8d87fe47eb22a3c43d0856dd5463a1671 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_5.q 
> 3026d9093eddbf53611a79df5fbe1ee55884273a 
>   ql/src/test/queries/clientpositive/materialized_view_create_rewrite_dummy.q 
> 8c9da8ae69967d1e11a333a46fddc51957cd5f31 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_multi_db.q
>  85d926f9eb8c40d01bee6dab87baf5bf29790278 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_rebuild_dummy.q
>  72e3d65117c0712929bb217e3a5e769101b27ebd 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window.q
>  4cdb715d2873b726561979a5b6d93c086467cd3d 
>   
> ql/src/test/queries/clientpositive/materialized_view_create_rewrite_time_window_2.q
>  6873673a55580b3d94f2af4b7c7c0b20f191d879 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_1.q 
> 18b9f7d418eff200d551ce4f95a399741dfca3f8 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_10.q 
> 95427923164a28b1b0ef73f03fa69544daa5ff1c 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_2.q 
> 3a447fc1873bf98d748fb9fd09278d60b7c9ac55 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_3.q 
> 0823f59394dd00d9d02b0ae517454c035b21baed 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_4.q 
> 6724cec7710f981d094576f6befccd2491ebe936 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_5.q 
> d87928c07363f6bd0ba4ae8f1986f5f53f513731 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_6.q 
> 23fc3c14ce5a0c43483824de9ac0835453f74c44 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_7.q 
> 3d1cedc4f56a1bceecba390292fcd26ad6ce1863 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_8.q 
> cfcfddce506d80ad55ac4d61dc0c8069b354c5cd 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_9.q 
> 18d5cec8f98fdc7c44d3a358c765ab111019941d 
>   ql/src/test/queries/clientpositive/materialized_view_rewrite_empty.q 
> 9ae1d4e81b033a07cf38bb4900cb819d34bf3d3f 
>

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

[jira] [Created] (HIVE-21064) java.lang.ClassCastException: org.apache.hadoop.hive.serde2.lazy.LazyStruct cannot be cast to org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch

2018-12-21 Thread Jesus Camacho Rodriguez (JIRA) 
Jesus Camacho Rodriguez created HIVE-21064:
--

 Summary: java.lang.ClassCastException: 
org.apache.hadoop.hive.serde2.lazy.LazyStruct cannot be cast to 
org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch
 Key: HIVE-21064
 URL: https://issues.apache.org/jira/browse/HIVE-21064
 Project: Hive
  Issue Type: Bug
Reporter: Jesus Camacho Rodriguez


The error is present in one of the test files (parallel_orderby.q) and it was 
discovered while working on HIVE-16957. To reproduce:

{code}
set hive.mapred.mode=nonstrict;
set hive.stats.column.autogather=false;

create table src5_n2 (key string, value string);
load data local inpath '../../data/files/kv5.txt' into table src5_n2;
load data local inpath '../../data/files/kv5.txt' into table src5_n2;

set mapred.reduce.tasks = 4;
set hive.optimize.sampling.orderby=true;
set hive.optimize.sampling.orderby.percent=0.66f;

create table total_ordered as select * from src5_n2 order by key, value;
{code}

The create table statement throws the following exception:
{code}
java.lang.ClassCastException: org.apache.hadoop.hive.serde2.lazy.LazyStruct 
cannot be cast to org.apache.hadoop.hive.ql.exec.vector.VectorizedRowBatch
{code}

The exception is printed in the q file. This was introduced when HIVE-18910 was 
checked in, I am not sure whether it is a known issue. Cc [~djaiswal] [~jdere]




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Re: Review Request 69562: HIVE-16957

2018-12-21 Thread Jesús Camacho Rodríguez 


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/queries/clientpositive/cbo_rp_cross_product_check_2.q
> > Line 7 (original), 7 (patched)
> > 
> >
> > Is it necessary to specify schema to get auto-gather to work with CTAS?

This is a different issue: Observe that it is a return path test. CTAS is 
failing in this specific case with a column name mismatch but I did not spend 
time exploring it further. Instead, I rewrote it into a CREATE TABLE + INSERT 
to preserve the test.
I should probably create a follow-up, although I guess if we set return path to 
true we would discover many additional issues, not only this one.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/dynamic_semijoin_reduction_3.q.out
> > Lines 231-240 (original)
> > 
> >
> > Did we lose stats auto-gather for this merge statement?

The original query is:

explain
merge into acidTbl as t using nonAcidOrcTbl s ON t.a = s.a 
WHEN MATCHED AND s.a > 8 THEN DELETE
WHEN MATCHED THEN UPDATE SET b = 7
WHEN NOT MATCHED THEN INSERT VALUES(s.a, s.b);

Note that the branch that disappears is for `default.merge_tmp_table`, which is 
not part of the original query, and if you scroll down you can see that the 
autogather stats branch is still present for the INSERT to `acidTbl` that 
results from the MERGE.

The edge disappears because of the change in L7707 in SemanticAnalyzer that 
limits the scope of auto-gather column stats, excluding temporary tables. I 
think that makes sense?

The auto-gather basic stats is still preserved and part of the plan though. 
This should be fine since they are not written to metastore?


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/enforce_constraint_notnull.q.out
> > Lines 4640-4648 (original)
> > 
> >
> > No more auto-gather stats?

Merge statement, see explanation above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/insert_into_default_keyword.q.out
> > Lines 2952-2960 (original)
> > 
> >
> > Losing stats branch.

Merge statement, see explanation above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/runtime_stats_merge.q.out
> > Lines 169-176 (original)
> > 
> >
> > Losing stats branch.

Merge statement, see explanation above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/semijoin_hint.q.out
> > Lines 3361-3368 (original)
> > 
> >
> > Losing stats branch.

Merge statement, see explanation above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/sqlmerge.q.out
> > Lines 213-219 (original)
> > 
> >
> > Losing stats branch.

Merge statement, see explanation above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/tez_nway_join.q.out
> > Line 63 (original), 63 (patched)
> > 
> >
> > Did we lose stats in this scenario?

`foo` is a temporary table, root cause is the same as described above. As part 
of the changes in this patch, I excluded auto-gather column stats on temporary 
tables for all operations, not only CTAS. That is why it is reflected in this 
case too, although the collection was done after INSERT operation.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/llap/vector_udf2.q.out
> > Line 291 (original), 291 (patched)
> > 
> >
> > are we losing stats here?

`hive_14349` is a temporary table, same as above.


> On Dec. 20, 2018, 7:56 p.m., Ashutosh Chauhan wrote:
> > ql/src/test/results/clientpositive/parallel_orderby.q.out
> > Line 97 (original)
> > 
> >
> > Uhh.. we actually had a bug checked in the golden files :(

:( The issue is still there, this patch just hides it (probably because the 
task is not fully vectorized). I have created HIVE-21064 with the description 
and the test to repro the failure.


- Jesús


---
This is an automatically generated e-mail. To reply, visit: