sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jarek Cecho <jar...@apache.org>
Subject Re: Review Request 42776: SQOOP-2797: Sqoop2: Datatypes: Add Blob data type support for Derby
Date Thu, 04 Feb 2016 02:42:21 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message