----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42776/#review117465 -----------------------------------------------------------
common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java (line 25) <https://reviews.apache.org/r/42776/#comment178661> Nit: We're usually trying to avoid asterisk imports, so let's not make this change. common-test/src/main/java/org/apache/sqoop/common/test/asserts/ProviderAsserts.java (lines 57 - 58) <https://reviews.apache.org/r/42776/#comment178662> Since BLOB is for "binary" data, would it make sense to use the getBytes() method and compare using bytes rather then casting to string? common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java (line 26) <https://reviews.apache.org/r/42776/#comment178663> Nit: Asterisk import. common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java (lines 321 - 325) <https://reviews.apache.org/r/42776/#comment178664> Aren't we missing case for BLOB here? common-test/src/main/java/org/apache/sqoop/common/test/db/DatabaseProvider.java (lines 335 - 336) <https://reviews.apache.org/r/42776/#comment178665> Why are we skipping Blob? common-test/src/main/java/org/apache/sqoop/common/test/db/types/DerbyTypeList.java (lines 112 - 118) <https://reviews.apache.org/r/42776/#comment178666> Let's make the whole method throw "Exception" to avoid a need to use try {} catch blocks. common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java (lines 28 - 29) <https://reviews.apache.org/r/42776/#comment178667> Nit: Asterisk import. common/src/main/java/org/apache/sqoop/schema/type/Blob.java (line 20) <https://reviews.apache.org/r/42776/#comment178668> What is the difference between Blob and Binary? Aren't they the same? connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java (line 20) <https://reviews.apache.org/r/42776/#comment178669> Nit: Asterisk imports. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java (line 20) <https://reviews.apache.org/r/42776/#comment178670> Nit: Asterisk imports. connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java (lines 81 - 87) <https://reviews.apache.org/r/42776/#comment178671> Why do we need to encode the BLOB differently? test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java (lines 123 - 128) <https://reviews.apache.org/r/42776/#comment178672> The changes in this file seems independent on the other changes. Can we perhaps submit them in separate JIRA? - Jarek Cecho On Jan. 26, 2016, 4:55 a.m., Colin Ma wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42776/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2016, 4:55 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 > 4e1ef6a > > 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/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 > 9b0885a > test/src/main/java/org/apache/sqoop/test/infrastructure/SqoopTestCase.java > 74fe29b > > Diff: https://reviews.apache.org/r/42776/diff/ > > > Testing > ------- > > > Thanks, > > Colin Ma > >
