> On 2012-05-04 22:39:58, Travis Crawford wrote:
> > src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java, line 116
> > <https://reviews.apache.org/r/4971/diff/1/?file=105999#file105999line116>
> >
> >     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.

No idea, looks like dead code to me. I don't mind having it removed as part of 
this patch.


- Francis


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


On 2012-05-05 01:23:50, Travis Crawford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4971/
> -----------------------------------------------------------
> 
> (Updated 2012-05-05 01:23:50)
> 
> 
> 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/mapreduce/HCatBaseInputFormat.java 268167e 
>   src/java/org/apache/hcatalog/mapreduce/HCatRecordReader.java 65f96f4 
>   src/java/org/apache/hcatalog/mapreduce/InternalUtil.java 1837081 
>   src/java/org/apache/hcatalog/mapreduce/ProgressReporter.java fb379cd 
>   src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java f3d07a0 
> 
> 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