Mailing-List: contact dev-help@hive.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@hive.apache.org
Content-Type: multipart/alternative;
 boundary="===============3128423707156044069=="
MIME-Version: 1.0
Subject: Re: Review Request 55498: HIVE-15538: Test HIVE-13884 with more
 complex query predicates
From: Barna Zsombor Klara <zsombor.klara@cloudera.com>
To: Chaoyu Tang <ctang.ma@gmail.com>
Cc: Barna Zsombor Klara <zsombor.klara@cloudera.com>,
 hive <dev@hive.apache.org>, Marta Kuczora <kuczoram@cloudera.com>
Date: Fri, 13 Jan 2017 14:22:14 -0000
Message-ID: <20170113142214.1677.48481@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: Barna Zsombor Klara <noreply@reviews.apache.org>
References: <20170113115849.25657.50589@reviews.apache.org>
In-Reply-To: <20170113115849.25657.50589@reviews.apache.org>
Reply-To: Barna Zsombor Klara <zsombor.klara@cloudera.com>
archived-at: Fri, 13 Jan 2017 14:22:16 -0000

--===============3128423707156044069==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit


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


Thanks for the patch Marta, +1. And thank you for taking the time to write a unit test for this instead of a qtest.
If you can, please take a look at my comments below, but in the current state it is already fine.


itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java (line 267)
<https://reviews.apache.org/r/55498/#comment232822>

    Not sure if it can be done easily but could we somehow check the returned values as well? Right now we only know that the query returned the correct nr of records, whether those are the expected values is unclear.


itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java (line 288)
<https://reviews.apache.org/r/55498/#comment232817>

    Can we make the original exception message in HiveMetaStore into a public string pattern and use it here to construct the error message? This way if someone edits the message the test should keep working.


- Barna Zsombor Klara


On Jan. 13, 2017, 11:58 a.m., Marta Kuczora wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55498/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2017, 11:58 a.m.)
> 
> 
> Review request for hive and Chaoyu Tang.
> 
> 
> Bugs: HIVE-15538
>     https://issues.apache.org/jira/browse/HIVE-15538
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Added unit test for testing HIVE-13884 with more complex queries and hive.metastore.limit.partition.request enabled.
> It covers cases when the query predicates can be pushed down and the number of partitions can be retrieved via directSQL.
> It also covers cases when the number of partitions cannot be retrieved via directSQL, so it falls back to ORM.
> 
> 
> Diffs
> -----
> 
>   data/files/max_partition_test_input.txt PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestMetaStoreLimitPartitionRequest.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/55498/diff/
> 
> 
> Testing
> -------
> 
> The patch contains only a new unit test. Ran the test multiple times successfully.
> 
> 
> Thanks,
> 
> Marta Kuczora
> 
>


--===============3128423707156044069==--