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


I'll take a look at adding a test for this, address the issues below and update 
the patch. Thanks for the feedback!


src/java/org/apache/hcatalog/common/HCatUtil.java
<https://reviews.apache.org/r/4971/#comment16773>

    Good suggestion, I'll move to internal util since its not intended for 
public use.



src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
<https://reviews.apache.org/r/4971/#comment16771>

    Basically to minimize the diff - will update.



src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java
<https://reviews.apache.org/r/4971/#comment16772>

    I don't actually understand this whole properties loop, since its not used. 
Any clue why this is here? If not, mind if I just remove it entirely? 
Originally I left this alone since it was not strictly necessary for the patch.


- Travis


On 2012-05-02 17:55:29, Travis Crawford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4971/
> -----------------------------------------------------------
> 
> (Updated 2012-05-02 17:55:29)
> 
> 
> Review request for hcatalog and Francis Liu.
> 
> 
> Summary
> -------
> 
> Update ProgressReporter to work with both old and new mapreduce API. Delay 
> creating the base record reader so we have a StatusReporter and can use 
> counters.
> 
> 
> This addresses bug HCATALOG-373.
>     https://issues.apache.org/jira/browse/HCATALOG-373
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/hcatalog/common/HCatUtil.java cb6404a 
>   src/java/org/apache/hcatalog/mapreduce/HCatBaseInputFormat.java 268167e 
>   src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 65f96f4 
>   src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java fb379cd 
> 
> Diff: https://reviews.apache.org/r/4971/diff
> 
> 
> Testing
> -------
> 
> "ant clean test" passes
> 
> I can run pig+hcatalog queries using Elephant-Bird deprecated API wrappers, 
> which is why this issue originally came up.
> 
> 
> Thanks,
> 
> Travis
> 
>

Reply via email to