----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34390/#review84347 -----------------------------------------------------------
Have a few nits. Also, could you add a test or two to TestMerge.java or an equivalent? src/java/org/apache/sqoop/mapreduce/MergeAvroReducer.java <https://reviews.apache.org/r/34390/#comment135608> org.apache.sqoop.lib.SqoopRecord src/java/org/apache/sqoop/mapreduce/MergeAvrodMapper.java <https://reviews.apache.org/r/34390/#comment135609> use org.apache.* equivalents. src/java/org/apache/sqoop/mapreduce/MergeAvrodMapper.java <https://reviews.apache.org/r/34390/#comment135562> MergeAvrodMapper => MergeAvroMapper src/java/org/apache/sqoop/mapreduce/MergeJob.java <https://reviews.apache.org/r/34390/#comment135607> NIT: New line not needed. src/java/org/apache/sqoop/mapreduce/MergeJob.java <https://reviews.apache.org/r/34390/#comment135567> Might want DEBUG logging to log schemas. This can get really tricky in practice. src/java/org/apache/sqoop/mapreduce/MergeJob.java <https://reviews.apache.org/r/34390/#comment135569> Might want to check if these are avro files. Avro has a header that should be grokkable: https://github.com/apache/avro/blob/trunk/lang/py/src/avro/datafile.py#L145. Also, I think there are sometimes 2 avro files: "*.avro" and "*.asc". src/java/org/apache/sqoop/mapreduce/MergeReducer.java <https://reviews.apache.org/r/34390/#comment135606> NIT: New line not needed. src/java/org/apache/sqoop/mapreduce/MergeReducer.java <https://reviews.apache.org/r/34390/#comment135605> NIT: New line not needed. src/java/org/apache/sqoop/mapreduce/MergeReducerBase.java <https://reviews.apache.org/r/34390/#comment135604> Don't forget license. - Abraham Elmahrek On May 19, 2015, 1:50 a.m., Yibing Shi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34390/ > ----------------------------------------------------------- > > (Updated May 19, 2015, 1:50 a.m.) > > > Review request for Sqoop and Jarek Cecho. > > > Repository: sqoop-trunk > > > Description > ------- > > SQOOP-1094 Add avro support to merge tool > > > Diffs > ----- > > src/java/org/apache/sqoop/mapreduce/AvroJob.java > bb4755c880d4b70f812caf5e812135400602ee36 > src/java/org/apache/sqoop/mapreduce/MergeAvroReducer.java PRE-CREATION > src/java/org/apache/sqoop/mapreduce/MergeAvrodMapper.java PRE-CREATION > src/java/org/apache/sqoop/mapreduce/MergeJob.java > 4e2a916911e7f47838366edf46b5ba5073502453 > src/java/org/apache/sqoop/mapreduce/MergeReducer.java > cafff8ab0609cd0580ff85e49082a59ce68e7141 > src/java/org/apache/sqoop/mapreduce/MergeReducerBase.java PRE-CREATION > > Diff: https://reviews.apache.org/r/34390/diff/ > > > Testing > ------- > > > Thanks, > > Yibing Shi > >
