----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2724/#review9540 -----------------------------------------------------------
Hi Masatake, I've finished reviewing this version. I was able to move some bits, so thank you very much for this connector! I've gathered couple others small notes: /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java <https://reviews.apache.org/r/2724/#comment20375> Could add option to create those temporary tables in different database? /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java <https://reviews.apache.org/r/2724/#comment20376> Could you add between debug output that will print out all used arguments? Something like: LOG.debug("Starting pg_bulkload with arguments:"); for (String arg : args) { LOG.debug(" " + arg); } /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java <https://reviews.apache.org/r/2724/#comment20377> I've noticed that very similar method is already present in DirectPostgresqlManager, do you think you could reuse this one? Please incorporate my comments from both this and previous review - I believe we're on the right track to get this committed! Jarcec - Jarek Cecho On July 26, 2012, 10:41 a.m., Masatake Iwasaki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2724/ > ----------------------------------------------------------- > > (Updated July 26, 2012, 10:41 a.m.) > > > Review request for Sqoop. > > > Description > ------- > > Patch for SQOOP-390 > https://issues.apache.org/jira/browse/SQOOP-390 > > > This addresses bug SQOOP-390. > https://issues.apache.org/jira/browse/SQOOP-390 > > > Diffs > ----- > > /src/java/org/apache/sqoop/manager/PGBulkloadManager.java PRE-CREATION > /src/java/org/apache/sqoop/mapreduce/AutoProgressReducer.java PRE-CREATION > /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportJob.java PRE-CREATION > /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportMapper.java > PRE-CREATION > /src/java/org/apache/sqoop/mapreduce/PGBulkloadExportReducer.java > PRE-CREATION > /src/test/com/cloudera/sqoop/manager/PGBulkloadManagerManualTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/2724/diff/ > > > Testing > ------- > > This patch include the test class PGBulkloadManagerTest. > I've tested "ant test" and passed. > > > Thanks, > > Masatake Iwasaki > >
