Review Request 65276: HIVE-18516

2018-01-22 Thread Deepak Jaiswal

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

Review request for hive, Eugene Koifman and Jason Dere.


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


Repository: hive-git


Description
---

load data should rename files consistent with insert statements for ACID Tables.
Includes test change for a missed test.


Diffs
-

  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 23983d85b3 
  ql/src/java/org/apache/hadoop/hive/ql/parse/LoadSemanticAnalyzer.java 
5868d4dd56 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHiveCopyFiles.java 
c6a4a8926b 
  ql/src/test/queries/clientpositive/load_data_acid_rename.q PRE-CREATION 
  ql/src/test/queries/clientpositive/smb_mapjoin_7.q 4a6afb0496 
  ql/src/test/results/clientpositive/beeline/smb_mapjoin_7.q.out 7a6f8c53a5 
  ql/src/test/results/clientpositive/llap/load_data_acid_rename.q.out 
PRE-CREATION 
  ql/src/test/results/clientpositive/smb_mapjoin_7.q.out b71c5b87c1 
  ql/src/test/results/clientpositive/spark/smb_mapjoin_7.q.out ac49c02913 


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


Testing
---


Thanks,

Deepak Jaiswal



[jira] [Created] (HIVE-18516) load data should rename files consistent with insert statements for ACID Tables

2018-01-22 Thread Deepak Jaiswal (JIRA)
Deepak Jaiswal created HIVE-18516:
-

 Summary: load data should rename files consistent with insert 
statements for ACID Tables
 Key: HIVE-18516
 URL: https://issues.apache.org/jira/browse/HIVE-18516
 Project: Hive
  Issue Type: Bug
Reporter: Deepak Jaiswal
Assignee: Deepak Jaiswal


h1. load data should rename files consistent with insert statements for ACID 
Tables.



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


[jira] [Created] (HIVE-18515) Custom Hive on Spark Tab in Spark Web UI

2018-01-22 Thread Sahil Takiar (JIRA)
Sahil Takiar created HIVE-18515:
---

 Summary: Custom Hive on Spark Tab in Spark Web UI
 Key: HIVE-18515
 URL: https://issues.apache.org/jira/browse/HIVE-18515
 Project: Hive
  Issue Type: Sub-task
  Components: Spark
Reporter: Sahil Takiar


This is more of an investigation JIRA. It would be nice if Hive-on-Spark had 
its own dedicated tab in the Spark Web UI. Something similar to what Spark SQL 
has. It may be do-able if we follow the same model that Spark SQL does - 
creating a custom class that extends {{SparkUITab}}



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


[jira] [Created] (HIVE-18514) add service output for ranger to WM DDL operations

2018-01-22 Thread Sergey Shelukhin (JIRA)
Sergey Shelukhin created HIVE-18514:
---

 Summary: add service output for ranger to WM DDL operations
 Key: HIVE-18514
 URL: https://issues.apache.org/jira/browse/HIVE-18514
 Project: Hive
  Issue Type: Bug
Reporter: Sergey Shelukhin
 Attachments: HIVE-18514.patch





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


[jira] [Created] (HIVE-18513) Query results caching

2018-01-22 Thread Jason Dere (JIRA)
Jason Dere created HIVE-18513:
-

 Summary: Query results caching
 Key: HIVE-18513
 URL: https://issues.apache.org/jira/browse/HIVE-18513
 Project: Hive
  Issue Type: Bug
  Components: Query Planning
Reporter: Jason Dere
Assignee: Jason Dere


Add a query results cache that can save the results of an executed Hive query 
for reuse on subsequent queries. This may be useful in cases where the same 
query is issued many times, since Hive can return back the results of a cached 
query rather than having to execute the full query on the cluster.



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


Re: Review Request 65250: HIVE-18387

2018-01-22 Thread Jesús Camacho Rodríguez

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

(Updated Jan. 22, 2018, 10:33 p.m.)


Review request for hive and Ashutosh Chauhan.


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


Repository: hive-git


Description (updated)
---

HIVE-18321


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/Constants.java 
4e0c50a66256271507a377f8f7fa4ab7cb9dc279 
  ql/src/java/org/apache/hadoop/hive/ql/QueryLifeTimeHookRunner.java 
53d716bceb98c2ced3a3ba3f0cd607766447dfd9 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
51ef39057434c41fbe760c547e3bf231e65e4cc0 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewUpdateRegistryTask.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/exec/MaterializedViewUpdateRegistryWork.java
 PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 
85cef8664674db72cd69929d4ad96f1bd85279da 
  
ql/src/java/org/apache/hadoop/hive/ql/hooks/MaterializedViewRegistryUpdateHook.java
 e886399d5371e4b8d9692811998f6cc8ca3c2b7b 
  ql/src/java/org/apache/hadoop/hive/ql/optimizer/GenMapRedUtils.java 
923372d5b6da42446997051d0758e9aab4881e2e 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
511014c37c761bebbe90ec4906fcac97d9f83bae 
  ql/src/java/org/apache/hadoop/hive/ql/parse/TaskCompiler.java 
0c1c4e09d67f25c307e693bc220015f8b47f7ce1 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
97baf25ea8bbe6f55e46e2ea5bdf33a5a71eecdf 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ImportTableDesc.java 
3535fa4d02106f4e96af6c33ffa291c8db21e3bc 
  ql/src/java/org/apache/hadoop/hive/ql/plan/StatsWork.java 
9b45f430261331a3d3af2fd1706d5892a76d84e4 
  ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java 
b48379013d74c56df245bb9e292e45bb298367da 
  ql/src/test/queries/clientpositive/druidmini_mv.q 
e0593576020af7dd5cc26dd613395d6cde72496e 
  ql/src/test/queries/clientpositive/materialized_view_create_rewrite_4.q 
efc65c4061c608fac5ba308a9a8238aca443555f 
  ql/src/test/results/clientpositive/druid/druidmini_mv.q.out 
5a0b885f7759d013eb6b1e5d411f02e9c4d4468b 
  ql/src/test/results/clientpositive/materialized_view_create.q.out 
89523042a9acac42dbfe865cc6f091ae6aea4a6e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite.q.out 
95b1d61f94dd54a5419ca16d3ec5ec073a50428e 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_3.q.out 
0d8d238e8b43fdf453f3c11cd1cdd0f1aa8764bc 
  ql/src/test/results/clientpositive/materialized_view_create_rewrite_4.q.out 
8ab151718668daa8915575ffed5845d62e15b775 
  
ql/src/test/results/clientpositive/materialized_view_create_rewrite_multi_db.q.out
 d7ee468b49af904da93a74c86f0898c310970cab 
  ql/src/test/results/clientpositive/materialized_view_describe.q.out 
efd60d4345d68fe627f11ea8fd4b82ea5404e1eb 


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

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


Testing
---


Thanks,

Jesús Camacho Rodríguez



Re: Review Request 65047: Fix for HIVE-18393

2018-01-22 Thread Sahil Takiar

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



Overall, looks good to me. I'm really not that familiar with the serde / OI 
classes. So maybe someone else should take a look. Alternatively, a brief 
description of what these classes do and how they work would be very helpful.


ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetStringInspector.java
Lines 64 (patched)


do we need to handle all the others types like date, interval, or any of 
the comples types like array, struct, union, etc.


- Sahil Takiar


On Jan. 18, 2018, 5:34 p.m., Janaki Lahorani wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65047/
> ---
> 
> (Updated Jan. 18, 2018, 5:34 p.m.)
> 
> 
> Review request for hive, Aihua Xu, Alexander Kolbasov, Andrew Sherman, 
> Ashutosh Chauhan, cheng xu, ferdy, Alan Gates, Owen O'Malley, Sergey 
> Shelukhin, Sahil Takiar, and Vihang Karajgaonkar.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Data saved as TimeStamp, Decimal, Double, Float, BigInt, Int, SmallInt, 
> Tinyint and Boolean, after the type is changed to String, VarChar or Char 
> will return correct results.
> 
> 
> Diffs
> -
> 
>   
> ql/src/java/org/apache/hadoop/hive/ql/io/parquet/serde/primitive/ParquetStringInspector.java
>  70a92ab3bc9d3b2ecc205fad274e8c639812e59a 
>   ql/src/test/queries/clientpositive/typechangetest.q PRE-CREATION 
>   ql/src/test/results/clientpositive/typechangetest.q.out PRE-CREATION 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableHiveCharObjectInspector.java
>  3204e309686f295810c1e850c8028303b2f5f7ff 
>   
> serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/primitive/WritableHiveVarcharObjectInspector.java
>  8ac2d8477e1071f95d3f5faed8a7be92c76e6347 
> 
> 
> Diff: https://reviews.apache.org/r/65047/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Janaki Lahorani
> 
>



Review Request 65271: JDBC: Provide a way for JDBC users to pass cookie info via connection string

2018-01-22 Thread Vaibhav Gumashta

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

Review request for hive and Thejas Nair.


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


Repository: hive-git


Description
---

https://issues.apache.org/jira/browse/HIVE-18447


Diffs
-

  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/thrift/TestThriftHttpCLIServiceFeatures.java
 1911d2ce17 
  jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java b5d289e023 
  jdbc/src/java/org/apache/hive/jdbc/HttpBasicAuthInterceptor.java 8cb7a69df7 
  jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java 
3509cab775 
  jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java 68b0ff19b8 
  jdbc/src/java/org/apache/hive/jdbc/HttpTokenAuthInterceptor.java 207ed9e663 
  jdbc/src/java/org/apache/hive/jdbc/Utils.java 1c1f644c92 


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


Testing
---


Thanks,

Vaibhav Gumashta



[jira] [Created] (HIVE-18512) Get Results ReadAhead

2018-01-22 Thread BELUGA BEHR (JIRA)
BELUGA BEHR created HIVE-18512:
--

 Summary: Get Results ReadAhead
 Key: HIVE-18512
 URL: https://issues.apache.org/jira/browse/HIVE-18512
 Project: Hive
  Issue Type: Improvement
  Components: HiveServer2
Affects Versions: 3.0.0, 2.4.0
Reporter: BELUGA BEHR


I don't have any data to back this up, but I wanted to put it on the radar.

It may be possible to improve performance of HS2 with an HDFS read-ahead reader 
for result data.  This would require adding a cache (configurable size) to the 
Driver/Context object and adding a separate thread for loading results 
asynchronously while the client is processing its current batch of results.  It 
seems that currently, results are loaded on demand.

https://github.com/apache/hive/blob/6d890faf22fd1ede3658a5eed097476eab3c67e9/ql/src/java/org/apache/hadoop/hive/ql/Driver.java#L2298



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


[jira] [Created] (HIVE-18511) Fix generated checkstyle errors

2018-01-22 Thread Peter Vary (JIRA)
Peter Vary created HIVE-18511:
-

 Summary: Fix generated checkstyle errors
 Key: HIVE-18511
 URL: https://issues.apache.org/jira/browse/HIVE-18511
 Project: Hive
  Issue Type: Sub-task
Reporter: Peter Vary
Assignee: Peter Vary


HIVE-18510 identified, that checkstyle was not running for test sources.

After running checkstyle several errors are identified



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


Re: Review Request 65264: HIVE-18481: Create tests for table related methods (get, list, exists)

2018-01-22 Thread Peter Vary via Review Board

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

(Updated Jan. 22, 2018, 3:38 p.m.)


Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.


Changes
---

Addressed checkstyle errors


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


Repository: hive-git


Description
---

Create IMetaStoreClient tests to cover the table query methods


Diffs (updated)
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
 PRE-CREATION 


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

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


Testing
---

Run the created tests


Thanks,

Peter Vary



Re: Review Request 65265: HIVE-18509 Create tests for table manipulation related methods (create, alter, drop)

2018-01-22 Thread Peter Vary via Review Board

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

(Updated Jan. 22, 2018, 3:36 p.m.)


Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.


Changes
---

Addressed checkstyle errors


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


Repository: hive-git


Description
---

Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is 
possible to provide configuration value overrides for non-metastore config 
values (like fs trash settings)


Diffs (updated)
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
 PRE-CREATION 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java
 ed071f8 


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

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


Testing
---

Run the tests


Thanks,

Peter Vary



Re: Review Request 65217: Create tests to cover alterPartition and renamePartition methods

2018-01-22 Thread Marta Kuczora via Review Board

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



Thanks a lot Adam for the patch.
It looks good to me, I just have few questions.


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
Lines 223 (patched)


When altering a partition, is it possible to modify its StorageDescriptor 
and location? 
If so, what happens if we try to modify the sd or location to null or 
empty? 
Is it possible to change the columns in the partition's storage descriptor?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
Lines 497 (patched)


When tyring to alter two partitions, but an exception occurs during 
altering one of them, what happens with the other? All modification will be 
rolled back correctly?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
Lines 769 (patched)


What happens if the newPart partition has different db/table than the ones 
set in the method parameter?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
Lines 770 (patched)


What happens if the db/table is null in the newPart partition?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
Lines 771 (patched)


Is it possible to change any other attribute of a partition besides the 
value with the rename method?


- Marta Kuczora


On Jan. 19, 2018, 2:29 p.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65217/
> ---
> 
> (Updated Jan. 19, 2018, 2:29 p.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18468
> https://issues.apache.org/jira/browse/HIVE-18468
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Adding IMetastoreClient API tests that cover alterPartition ande 
> renamePartition methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAlterPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65217/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>



[jira] [Created] (HIVE-18510) Enable running checkstyle on test sources as well

2018-01-22 Thread Adam Szita (JIRA)
Adam Szita created HIVE-18510:
-

 Summary: Enable running checkstyle on test sources as well
 Key: HIVE-18510
 URL: https://issues.apache.org/jira/browse/HIVE-18510
 Project: Hive
  Issue Type: Improvement
Reporter: Adam Szita
Assignee: Adam Szita


Currently only source files are in the scope of checkstyle testing. We should 
expand the scope to include our testing code as well.



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


Re: Review Request 65210: HIVE-18480 Create tests for function related methods

2018-01-22 Thread Peter Vary via Review Board


> On Jan. 22, 2018, 1:41 p.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
> > Lines 409 (patched)
> > 
> >
> > After dropping the functions, it could be verified if they are really 
> > deleted. Like doing a get or list for the deleted function and check if it 
> > doesn't exist any more.

Done, thanks, this caught an error in the test :)


> On Jan. 22, 2018, 1:41 p.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
> > Lines 455 (patched)
> > 
> >
> > What happens if the filter is empty? Is the result going to be empty or 
> > contain all functions?

Added a test for this as well. Thanks!


> On Jan. 22, 2018, 1:41 p.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
> > Lines 544 (patched)
> > 
> >
> > What do you mean by changing only the class won't cause exception? In 
> > which cases will you get exception during altering a function?

Rewritten the comment :)
I hope this is better now :)


> On Jan. 22, 2018, 1:41 p.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
> > Lines 605 (patched)
> > 
> >
> > Calling alterFunction with null newFunction migh be also a test case.

Done. Thanks!


- Peter


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


On Jan. 22, 2018, 2:33 p.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65210/
> ---
> 
> (Updated Jan. 22, 2018, 2:33 p.m.)
> 
> 
> Review request for hive, Alan Gates, Marta Kuczora, Adam Szita, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18480
> https://issues.apache.org/jira/browse/HIVE-18480
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The test tries to go throught all af the function related tests and test with 
> various imputs, so not only the happy path, but the edge cases are covered too
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65210/diff/2/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Peter Vary
> 
>



Re: Review Request 65210: HIVE-18480 Create tests for function related methods

2018-01-22 Thread Peter Vary via Review Board

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

(Updated Jan. 22, 2018, 2:33 p.m.)


Review request for hive, Alan Gates, Marta Kuczora, Adam Szita, and Vihang 
Karajgaonkar.


Changes
---

Addressed Marta's comments, and fixed checkstyle errors


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


Repository: hive-git


Description
---

The test tries to go throught all af the function related tests and test with 
various imputs, so not only the happy path, but the edge cases are covered too


Diffs (updated)
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
 PRE-CREATION 


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

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


Testing
---

Run the tests


Thanks,

Peter Vary



Re: Review Request 65241: HIVE-18496: Create tests to cover add/alter/drop index methods

2018-01-22 Thread Peter Vary via Review Board

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



Thanks for the patch Marta! Only one question below


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAddAlterDropIndexes.java
Lines 218 (patched)


Is this ok to have this characters in the index name?


- Peter Vary


On Jan. 19, 2018, 3:17 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65241/
> ---
> 
> (Updated Jan. 19, 2018, 3:17 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18496
> https://issues.apache.org/jira/browse/HIVE-18496
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - void createIndex(Index, Table)
> - boolean dropIndex(String, String, String, boolean)
> - void alter_index(String, String, String, Index)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAddAlterDropIndexes.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65241/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



Re: Review Request 65241: HIVE-18496: Create tests to cover add/alter/drop index methods

2018-01-22 Thread Adam Szita via Review Board

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



Thanks for this patch Marta, it looks good! I've made a few recommendations only


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAddAlterDropIndexes.java
Lines 678 (patched)


For handling future bug fixes more easily we could use consistent notation 
for these TODOs, see line 343.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAddAlterDropIndexes.java
Lines 876-880 (patched)


If you'd like to shorten this one, I can recommend something like this:

cols.stream().collect(
  Collectors.toMap(col->col.getName(), col->col)
).get("id")


- Adam Szita


On Jan. 19, 2018, 3:17 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65241/
> ---
> 
> (Updated Jan. 19, 2018, 3:17 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18496
> https://issues.apache.org/jira/browse/HIVE-18496
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - void createIndex(Index, Table)
> - boolean dropIndex(String, String, String, boolean)
> - void alter_index(String, String, String, Index)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestAddAlterDropIndexes.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65241/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



Re: Review Request 65240: HIVE-18498: Create tests to cover get and list index methods

2018-01-22 Thread Peter Vary via Review Board

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



Nice job, looks good to me, one minor question


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetListIndexes.java
Lines 172 (patched)


Is it possible to have a set of indexes in most of the cases, and use that?


- Peter Vary


On Jan. 19, 2018, 3:05 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65240/
> ---
> 
> (Updated Jan. 19, 2018, 3:05 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18498
> https://issues.apache.org/jira/browse/HIVE-18498
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - Index getIndex(String, String, String)
> - List listIndexes(String, String, short)
> - List listIndexNames(String, String, short)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetListIndexes.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65240/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



Re: Review Request 65210: HIVE-18480 Create tests for function related methods

2018-01-22 Thread Marta Kuczora via Review Board

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



Thanks a lot Peter for the patch!
It looks good to me, I only have few questions and nits.


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
Lines 409 (patched)


After dropping the functions, it could be verified if they are really 
deleted. Like doing a get or list for the deleted function and check if it 
doesn't exist any more.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
Lines 455 (patched)


What happens if the filter is empty? Is the result going to be empty or 
contain all functions?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
Lines 544 (patched)


What do you mean by changing only the class won't cause exception? In which 
cases will you get exception during altering a function?



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
Lines 605 (patched)


Calling alterFunction with null newFunction migh be also a test case.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
Lines 628 (patched)


I guess the NullPointerException here is a typo. In the catch there is 
InvalidObjectException.


- Marta Kuczora


On Jan. 18, 2018, 11:37 a.m., Peter Vary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65210/
> ---
> 
> (Updated Jan. 18, 2018, 11:37 a.m.)
> 
> 
> Review request for hive, Alan Gates, Marta Kuczora, Adam Szita, and Vihang 
> Karajgaonkar.
> 
> 
> Bugs: HIVE-18480
> https://issues.apache.org/jira/browse/HIVE-18480
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The test tries to go throught all af the function related tests and test with 
> various imputs, so not only the happy path, but the edge cases are covered too
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestFunctions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65210/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Peter Vary
> 
>



Review Request 65265: HIVE-18509 Create tests for table manipulation related methods (create, alter, drop)

2018-01-22 Thread Peter Vary via Review Board

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

Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Added the new tests.
Modified AbstractMetaStoreService, so when starting mini-metastore it is 
possible to provide configuration value overrides for non-metastore config 
values (like fs trash settings)


Diffs
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesCreateDropAlterTruncate.java
 PRE-CREATION 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/minihms/AbstractMetaStoreService.java
 ed071f8 


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


Testing
---

Run the tests


Thanks,

Peter Vary



[jira] [Created] (HIVE-18509) Create tests for table manipulation related methods (create, alter, drop)

2018-01-22 Thread Peter Vary (JIRA)
Peter Vary created HIVE-18509:
-

 Summary: Create tests for table manipulation related methods 
(create, alter, drop)
 Key: HIVE-18509
 URL: https://issues.apache.org/jira/browse/HIVE-18509
 Project: Hive
  Issue Type: Sub-task
Reporter: Peter Vary
Assignee: Peter Vary


Create API tests for table metadata manipulations



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


Review Request 65264: HIVE-18481: Create tests for table related methods (get, list, exists)

2018-01-22 Thread Peter Vary via Review Board

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

Review request for hive, Marta Kuczora, Adam Szita, and Vihang Karajgaonkar.


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


Repository: hive-git


Description
---

Create IMetaStoreClient tests to cover the table query methods


Diffs
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestTablesGetListExists.java
 PRE-CREATION 


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


Testing
---

Run the created tests


Thanks,

Peter Vary



Re: Review Request 65213: HIVE-18479: Create tests to cover methods for dropping Partitions

2018-01-22 Thread Marta Kuczora via Review Board

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

(Updated Jan. 22, 2018, 12:38 p.m.)


Review request for hive, Peter Vary and Adam Szita.


Changes
---

Fixed the review findings.


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


Repository: hive-git


Description
---

The following methods of IMetaStoreClient are covered by this test.
- boolean dropPartition(String, String, List, boolean)
- boolean dropPartition(String, String, List, PartitionDropOptions)
- boolean dropPartition(String, String, String, boolean)

The test covers not just the happy pathes, but the edge cases as well.


Diffs (updated)
-

  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
 PRE-CREATION 


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

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


Testing
---

Run the tests


Thanks,

Marta Kuczora



Re: Review Request 65213: HIVE-18479: Create tests to cover methods for dropping Partitions

2018-01-22 Thread Marta Kuczora via Review Board


> On Jan. 19, 2018, 11:50 a.m., Peter Vary wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
> > Lines 341 (patched)
> > 
> >
> > Question: Does this behave the same way for external tables too?

Thanks a lot Peter for the review.

That's a good point. :) No, with external tables, the data won't be deleted.
Added an additional test case where deleteData = purge = true with an external 
table and the test checks if the partitions is dropped but the location is not 
deleted.


- Marta


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


On Jan. 22, 2018, 12:38 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65213/
> ---
> 
> (Updated Jan. 22, 2018, 12:38 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18479
> https://issues.apache.org/jira/browse/HIVE-18479
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - boolean dropPartition(String, String, List, boolean)
> - boolean dropPartition(String, String, List, PartitionDropOptions)
> - boolean dropPartition(String, String, String, boolean)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65213/diff/2/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



Re: Review Request 65213: HIVE-18479: Create tests to cover methods for dropping Partitions

2018-01-22 Thread Marta Kuczora via Review Board


> On Jan. 19, 2018, 1:02 p.m., Adam Szita wrote:
> > Overall looks good, I just have small observations.
> > 
> > One more nitpick: some helper methods could be static e.g. 
> > getYearAndMonthCols

Thanks a lot Adam for the review.

Good point, changed the getYearAndMonthPartCols and getYearPartCol helper 
methods to be static.


> On Jan. 19, 2018, 1:02 p.m., Adam Szita wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
> > Lines 64 (patched)
> > 
> >
> > Is there any reason to use 10? Supplying -1 would return all partitions 
> > which I think was the original intent in the test cases below (in 
> > listPartitions calls).

No specific reason, I just missed this. This is a good point, thanks for it.


> On Jan. 19, 2018, 1:02 p.m., Adam Szita wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
> > Lines 165 (patched)
> > 
> >
> > createTable is called here twice because it's also in @Before. Does 
> > this mean it overrides the existing table with the supplied tableParams?

In this test case I wanted to create a different table with the 
"auto.purge=true" table parameter. This table has the name "purge_test". In the 
@Before method, I create the default table with the name 
"test_drop_part_table". So there will be two tables in this test case.


> On Jan. 19, 2018, 1:02 p.m., Adam Szita wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
> > Lines 539-553 (patched)
> > 
> >
> > Guava lib has a handy method for these use cases:
> > 
> >  ArrayList com.google.common.collect.Lists.newArrayList(E... 
> > elements)
> > 
> > ...which is doing exactly the same + optimising on initial list size.

That's an excellent point, thanks a lot for the hint. I didn't know this lib 
before.


- Marta


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


On Jan. 18, 2018, 1:11 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65213/
> ---
> 
> (Updated Jan. 18, 2018, 1:11 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18479
> https://issues.apache.org/jira/browse/HIVE-18479
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - boolean dropPartition(String, String, List, boolean)
> - boolean dropPartition(String, String, List, PartitionDropOptions)
> - boolean dropPartition(String, String, String, boolean)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestDropPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65213/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



Re: Review Request 65240: HIVE-18498: Create tests to cover get and list index methods

2018-01-22 Thread Adam Szita via Review Board

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



This change looks good! I only spotted some small code-duplications that we may 
want to clean up.


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetListIndexes.java
Lines 312-329 (patched)


We may want to separate the common code parts into helper/checker methods 
here and then call with that with different num values.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetListIndexes.java
Lines 463-481 (patched)


Same remark here relating code duplication


- Adam Szita


On Jan. 19, 2018, 3:05 p.m., Marta Kuczora wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65240/
> ---
> 
> (Updated Jan. 19, 2018, 3:05 p.m.)
> 
> 
> Review request for hive, Peter Vary and Adam Szita.
> 
> 
> Bugs: HIVE-18498
> https://issues.apache.org/jira/browse/HIVE-18498
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following methods of IMetaStoreClient are covered by this test.
> - Index getIndex(String, String, String)
> - List listIndexes(String, String, short)
> - List listIndexNames(String, String, short)
> 
> The test covers not just the happy pathes, but the edge cases as well.
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetListIndexes.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65240/diff/1/
> 
> 
> Testing
> ---
> 
> Run the tests
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>



[GitHub] hive pull request #295: HIVE-15631

2018-01-22 Thread Tartarus0zm
GitHub user Tartarus0zm opened a pull request:

https://github.com/apache/hive/pull/295

HIVE-15631

When the Hive client is started, the sessionid is printed from the console.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Tartarus0zm/hive console_sessionid

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/hive/pull/295.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #295


commit 1d0c2d606013498b246f5a30b1972ad8b10cb9a2
Author: Tartarus 
Date:   2018-01-22T08:47:56Z

HIVE-15631
When the Hive client is started, the sessionid is printed from the console.




---


[GitHub] hive pull request #132: #HIVE-15631

2018-01-22 Thread Tartarus0zm
Github user Tartarus0zm closed the pull request at:

https://github.com/apache/hive/pull/132


---