-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29980/#review69034
-----------------------------------------------------------


Looks cool. Couple of comments.


connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/AvroDataTypeUtil.java
<https://reviews.apache.org/r/29980/#comment113600>

    Avro DOUBLE is a floating point. It should be signed: 
http://avro.apache.org/docs/1.7.5/spec.html#schema_primitive



connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
<https://reviews.apache.org/r/29980/#comment113603>

    Seems this can be cleaned up a bit:
    
        if (byteSize == null) return false;
        if (signed != null && !signed) byteSize *= 2;
        return byteSize <= (Integer.SIZE / Byte.SIZE);



connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
<https://reviews.apache.org/r/29980/#comment113602>

    `byteSize = byteSize << 1` :).


- Abraham Elmahrek


On Jan. 21, 2015, 1:26 a.m., Veena Basavaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29980/
> -----------------------------------------------------------
> 
> (Updated Jan. 21, 2015, 1:26 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-2022
>     https://issues.apache.org/jira/browse/SQOOP-2022
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> see jira
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/sqoop/json/util/SchemaSerialization.java 
> bbd58fc 
>   common/src/main/java/org/apache/sqoop/schema/type/FixedPoint.java 836e4ad 
>   common/src/main/java/org/apache/sqoop/schema/type/FloatingPoint.java 
> 7dcc753 
>   
> common/src/test/java/org/apache/sqoop/json/util/TestSchemaSerialization.java 
> 0cb7175 
>   
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/util/SqlTypesUtils.java
>  8228397 
>   
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java
>  83599d3 
>   
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java
>  249cd34 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/AvroDataTypeUtil.java
>  a71385b 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopAvroUtils.java
>  c19c7f4 
>   
> connector/connector-sdk/src/main/java/org/apache/sqoop/connector/common/SqoopIDFUtils.java
>  800630f 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/common/TestSqoopIDFUtils.java
>  1cef714 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestAVROIntermediateDataFormat.java
>  dd7a3e1 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestCSVIntermediateDataFormat.java
>  2556711 
>   
> connector/connector-sdk/src/test/java/org/apache/sqoop/connector/idf/TestJSONIntermediateDataFormat.java
>  12c66c2 
>   execution/mapreduce/src/test/java/org/apache/sqoop/job/TestMatching.java 
> 4945584 
>   
> execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java
>  ce39a78 
>   test/src/main/java/org/apache/sqoop/test/data/Cities.java 1e90c59 
>   test/src/main/java/org/apache/sqoop/test/data/UbuntuReleases.java 7979686 
> 
> Diff: https://reviews.apache.org/r/29980/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Veena Basavaraj
> 
>

Reply via email to