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