----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59346/#review180878 -----------------------------------------------------------
Hi Sandish, Thank you for making these changes so quickly! I have a few small comments, please address those, then I can +1 your patch. Also for future reference our naming convention for patches is SQOOP-(issue#)-(revision#).patch, so if you have jira issue 123 and it's your 4th revision in the review board, it should be SQOOP-123-4.patch :) Also please attach the patch to the jira as well. Thanks, Anna src/java/org/apache/sqoop/mapreduce/MergeJob.java Lines 47 (patched) <https://reviews.apache.org/r/59346/#comment256195> apologies for nit picking, but unfortunately moving imports can cause a lot of merge conflicts later on, could you please reorder the lines as they were? src/java/org/apache/sqoop/mapreduce/MergeJob.java Line 128 (original) <https://reviews.apache.org/r/59346/#comment256197> another tiny nit-pick: it makes merging/backporting easier if we do not make unnecesary whitespace/new line changes. - Anna Szonyi On July 18, 2017, 8:53 p.m., Sandish Kumar HN wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59346/ > ----------------------------------------------------------- > > (Updated July 18, 2017, 8:53 p.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/7/ > > > 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 > >
