Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-25 Thread Peter Vary via Review Board 

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


Ship it!




Ship It!

- Peter Vary


On Jan. 25, 2018, 1:47 p.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65284/
> ---
> 
> (Updated Jan. 25, 2018, 1:47 p.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18484
> https://issues.apache.org/jira/browse/HIVE-18484
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create tests to cover listPartition(s) methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65284/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-25 Thread Marta Kuczora via Review Board 

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


Ship it!




Ship It!

- Marta Kuczora


On Jan. 25, 2018, 1:47 p.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65284/
> ---
> 
> (Updated Jan. 25, 2018, 1:47 p.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18484
> https://issues.apache.org/jira/browse/HIVE-18484
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create tests to cover listPartition(s) methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65284/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-25 Thread Adam Szita via Review Board 


> On Jan. 25, 2018, 11:37 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
> > Lines 762-765 (patched)
> > 
> >
> > Is this the same use case as in the 
> > testListPartitionsByFilterEmptyFilter test? If it is, I think it is enough 
> > to have only one test for this use case.

Removed redundant case


> On Jan. 25, 2018, 11:37 a.m., Marta Kuczora wrote:
> > standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
> > Lines 1028-1031 (patched)
> > 
> >
> > What is the expected behaviour in this case? Should the 
> > getNumPartitionsByFilter return 0 or an other value?

Added missing assertion


- Adam


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


On Jan. 25, 2018, 1:47 p.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65284/
> ---
> 
> (Updated Jan. 25, 2018, 1:47 p.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18484
> https://issues.apache.org/jira/browse/HIVE-18484
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create tests to cover listPartition(s) methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65284/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-25 Thread Adam Szita via Review Board 

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

(Updated Jan. 25, 2018, 1:47 p.m.)


Review request for hive, Marta Kuczora and Peter Vary.


Changes
---

Addressing Marta's comments


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


Repository: hive-git


Description
---

Create tests to cover listPartition(s) methods


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Adam Szita

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-25 Thread Marta Kuczora via Review Board 

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



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


standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
Lines 762-765 (patched)


Is this the same use case as in the testListPartitionsByFilterEmptyFilter 
test? If it is, I think it is enough to have only one test for this use case.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
Lines 871-873 (patched)


The same as above. If it is the same use case as in 
testListPartitionSpecsByFilterEmptyFilter, I think it would enought to have 
only the one of them.



standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
Lines 1028-1031 (patched)


What is the expected behaviour in this case? Should the 
getNumPartitionsByFilter return 0 or an other value?


- Marta Kuczora


On Jan. 24, 2018, 11:22 a.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65284/
> ---
> 
> (Updated Jan. 24, 2018, 11:22 a.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18484
> https://issues.apache.org/jira/browse/HIVE-18484
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create tests to cover listPartition(s) methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65284/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-24 Thread Adam Szita via Review Board 

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

(Updated Jan. 24, 2018, 11:22 a.m.)


Review request for hive, Marta Kuczora and Peter Vary.


Changes
---

Addressed review comments


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


Repository: hive-git


Description
---

Create tests to cover listPartition(s) methods


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Adam Szita

Re: Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-23 Thread Peter Vary via Review Board 

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



That is a hugh number of tests.
Might add some more. Please check if the proposed ones are valid


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


This is strange :)
What is the puprose of this?



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


nit: _



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


nit: Java 8?



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


Is this needed?



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


Is this needed?



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


Maybe a case, when there is no result?



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


nit: newline?



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


nit: newline?



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


nit: checkstyle?



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


Shouldn't we check the results?



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


NoDb test case?



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


Shouldn't we check the results?



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


Shouldn't we check the results?



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


What happens with empty ("") filter?



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


What are the allowed keywords?
- OR
- AND
- ?



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


What happens with filter like this:
"YYyy=\"2017\""?



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


Do we want to use the result? :)


- Peter Vary


On Jan. 23, 2018, 9:34 a.m., Adam Szita wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65284/
> ---
> 
> (Updated Jan. 23, 2018, 9:34 a.m.)
> 
> 
> Review request for hive, Marta Kuczora and Peter Vary.
> 
> 
> Bugs: HIVE-18484
> https://issues.apache.org/jira/browse/HIVE-18484
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Create tests to cover listPartition(s) methods
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/client/TestListPartitions.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65284/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Adam Szita
> 
>

Review Request 65284: Create tests to cover listPartition(s) methods

2018-01-23 Thread Adam Szita via Review Board 

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

Review request for hive, Marta Kuczora and Peter Vary.


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


Repository: hive-git


Description
---

Create tests to cover listPartition(s) methods


Diffs
-

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


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


Testing
---


Thanks,

Adam Szita