sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <egyedb...@gmail.com>
Subject Re: Review Request 59346: SQOOP-3178 : Incremental Merging for Parquet File Format
Date Wed, 19 Jul 2017 15:30:19 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message