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



Hi Sandish,

On the top of Anna's findings I have one more, general comment on your patch. 
Otherwise I ran the unit tests with your change successfully. Could you please 
address my question below?

Thanks,
Bogi


src/java/org/apache/sqoop/mapreduce/MergeGenericRecordExportMapper.java
Lines 38 (patched)
<https://reviews.apache.org/r/59346/#comment256268>

    Maybe it's just me but instead of using comments in the code I would prefer 
to see verbose enough field names here. This comment also applies to the rest 
of your code in general, e.g. "InputSplit is" could be "InputSplit inputSplit" 
instead, etc. Could you please use a more self-explanatory/clean naming 
convention?


- Boglarka Egyed


On July 19, 2017, 7:37 a.m., Sandish Kumar HN wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59346/
> -----------------------------------------------------------
> 
> (Updated July 19, 2017, 7:37 a.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed, Attila Szabo, and Szabolcs Vasas.
> 
> 
> Bugs: PARQUET-1010 and SQOOP-3178
>     https://issues.apache.org/jira/browse/PARQUET-1010
>     https://issues.apache.org/jira/browse/SQOOP-3178
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> New feature for sqoop-1: Sqoop Merge and Incremental Merge for Parquet File 
> Format
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/mapreduce/MergeGenericRecordExportMapper.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/MergeJob.java 8b1cba33 
>   src/java/org/apache/sqoop/mapreduce/MergeParquetMapper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/MergeParquetReducer.java PRE-CREATION 
>   src/java/org/apache/sqoop/tool/ImportTool.java 78c7758e 
>   src/test/com/cloudera/sqoop/TestMerge.java 114e934a 
> 
> 
> Diff: https://reviews.apache.org/r/59346/diff/9/
> 
> 
> Testing
> -------
> 
> Hi,
> 
>  I have written a Sqoop Merge and Incremental Merge MR for Parquet File 
> Format and I have tested with million records of data with N number of 
> iterations. Please review My patch.
> 
> THIS ISSUE HAS DEPENDANCY ON PARQUET BUG. SO I HAVE RESOLVED THE PARQUET 
> ISSUE AND CREATED A PATCH FOR IT HERE 
> https://issues.apache.org/jira/browse/PARQUET-1010
> 
> Please let me know if there are any mistakes in My patch
> 
> 
> Thanks,
> 
> Sandish Kumar HN
> 
>

Reply via email to