----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28636/#review63680 -----------------------------------------------------------
Overall the approach looks good to me, I have just two comments. I've run all unit+integration tests and they are passing for me. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java <https://reviews.apache.org/r/28636/#comment105951> Super nit: Would be great if the line would end up with comman so that adding new error code won't require changing this line a gain. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java <https://reviews.apache.org/r/28636/#comment105952> It seems that the schemaColumns variable will very likely be a LinkedList [1]. As schemaColumns.get() will be called for every row and every column, let's perhaps convert the List to Array, so that we get constant time lookup? 1: https://github.com/apache/sqoop/blob/sqoop2/common/src/main/java/org/apache/sqoop/schema/Schema.java#L62 - Jarek Cecho On Dec. 3, 2014, 3:54 p.m., Veena Basavaraj wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28636/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2014, 3:54 p.m.) > > > Review request for Sqoop. > > > Bugs: SQOOP-1830 > https://issues.apache.org/jira/browse/SQOOP-1830 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > see jira > > > Diffs > ----- > > common/src/main/java/org/apache/sqoop/job/etl/ExtractorContext.java 4875ed0 > common/src/main/java/org/apache/sqoop/job/etl/LoaderContext.java 563b9ad > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcConnectorError.java > c291cb2 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java > af9320b > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java > d1e6805 > > connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestPartitioner.java > dee0242 > > connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java > 0a6369f > > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopInputFormat.java > d20c903 > execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopMapper.java > 664692a > > Diff: https://reviews.apache.org/r/28636/diff/ > > > Testing > ------- > > mvn test passes > > > Thanks, > > Veena Basavaraj > >
