----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42776/#review117752 -----------------------------------------------------------
Thanks for updating the JIRA description to correspond to "new" scope Colin :) I have few comments: common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java (lines 86 - 89) <https://reviews.apache.org/r/42776/#comment179029> I think that we can use assertEquals(byte [], byte[]): http://testng.org/javadoc/org/testng/Assert.html#assertEquals(byte[],%20byte[]) Also FYI: the for loop always tests first byte instead of using "i" :) common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java (line 310) <https://reviews.apache.org/r/42776/#comment179030> Can we update the javadoc that blob will be skipped? connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java (lines 20 - 36) <https://reviews.apache.org/r/42776/#comment179031> Nit: Let's not move the imports. Jarcec - Jarek Cecho On Feb. 3, 2016, 7:54 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42776/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2016, 7:54 a.m.) > > > Review request for Sqoop and Colin Ma. > > > Bugs: SQOOP-2797 > https://issues.apache.org/jira/browse/SQOOP-2797 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > Add Blob data type support for Derby > > > Diffs > ----- > > > common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java > ae1b60d > > common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java > afc5016 > > common-test/src/main/java/org/apache/sqoop/common/test/db/DerbyProvider.java > 8f3e434 > > common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java > 642651d > common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java > 3a3f9e8 > common/src/main/java/org/apache/sqoop/schema/type/Blob.java PRE-CREATION > common/src/main/java/org/apache/sqoop/schema/type/ColumnType.java 9e415bf > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java > 0235f28 > > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java > a6ffa7c > > connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java > fc25100 > > Diff: https://reviews.apache.org/r/42776/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
