You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Abraham Elmahrek <ab...@cloudera.com> on 2015/07/03 05:16:58 UTC

Re: Review Request 35859: SQOOP-1493: Add ability to import/export true decimal in Avro instead of serializing it to String


> On June 29, 2015, 11:04 p.m., Ryan Blue wrote:
> > src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java, line 59
> > <https://reviews.apache.org/r/35859/diff/2/?file=992257#file992257line59>
> >
> >     This looks unrelated to the decimal change to me. If it is related, could you add a comment to explain?

I thought this was necessary because DEFLATE_LEVEL = -1. It turns out that this is acceptable. The import, however, has been moved!


> On June 29, 2015, 11:04 p.m., Ryan Blue wrote:
> > src/java/org/apache/sqoop/orm/AvroSchemaGenerator.java, line 49
> > <https://reviews.apache.org/r/35859/diff/2/?file=992259#file992259line49>
> >
> >     This looks like what was in Hive. While we don't need to update the copyright because it is all ASF, do we need to update the license file to state that some content came from Hive? I think we do because contributors maintain copyright. Therefore we need to be able to track this addition back to the project it came from.

Good point. I'll make a note in the license file and add a comment.


> On June 29, 2015, 11:04 p.m., Ryan Blue wrote:
> > src/test/com/cloudera/sqoop/TestAvroImport.java, line 126
> > <https://reviews.apache.org/r/35859/diff/2/?file=992262#file992262line126>
> >
> >     Is this necessary? If the code runs correctly I thought this would be added by the mapper.

We need this in the test case itself though. We're reading Avro files for validation.


- Abraham


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


On June 25, 2015, 6:13 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35859/
> -----------------------------------------------------------
> 
> (Updated June 25, 2015, 6:13 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1493
>     https://issues.apache.org/jira/browse/SQOOP-1493
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> commit 8583077a33956cd2a3abc05f73172210c31994a9
> Author: Abraham Elmahrek <ab...@apache.org>
> Date:   Fri Jun 19 05:45:25 2015 +0300
> 
>     SQOOP-1493: Add ability to import/export true decimal in Avro instead of serializing it to String
> 
> :100644 100644 2e3d884... 628491c... M  ivy/libraries.properties
> :100644 100644 ee3cf62... 5970981... M  src/java/org/apache/sqoop/avro/AvroUtil.java
> :100644 100644 d9569c5... 5650079... M  src/java/org/apache/sqoop/manager/ConnManager.java
> :100644 100644 20f056a... 76c3458... M  src/java/org/apache/sqoop/mapreduce/AvroExportMapper.java
> :100644 100644 aed1e72... a4ac46e... M  src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java
> :100644 100644 ab263c1... b60ee42... M  src/java/org/apache/sqoop/mapreduce/GenericRecordExportMapper
> :100644 100644 2576673... 8490582... M  src/java/org/apache/sqoop/orm/AvroSchemaGenerator.java
> :100644 100644 1c6f7f4... aff8d08... M  src/java/org/apache/sqoop/orm/ClassWriter.java
> :100644 100644 663828c... 42e36ab... M  src/test/com/cloudera/sqoop/TestAvroExport.java
> :100644 100644 260e80a... 2f680c8... M  src/test/com/cloudera/sqoop/TestAvroImport.java
> 
> 
> Diffs
> -----
> 
>   ivy/libraries.properties 2e3d884 
>   src/java/org/apache/sqoop/avro/AvroUtil.java ee3cf62 
>   src/java/org/apache/sqoop/manager/ConnManager.java d9569c5 
>   src/java/org/apache/sqoop/mapreduce/AvroExportMapper.java 20f056a 
>   src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java aed1e72 
>   src/java/org/apache/sqoop/mapreduce/GenericRecordExportMapper.java ab263c1 
>   src/java/org/apache/sqoop/orm/AvroSchemaGenerator.java 2576673 
>   src/java/org/apache/sqoop/orm/ClassWriter.java 1c6f7f4 
>   src/test/com/cloudera/sqoop/TestAvroExport.java 663828c 
>   src/test/com/cloudera/sqoop/TestAvroImport.java 260e80a 
> 
> Diff: https://reviews.apache.org/r/35859/diff/
> 
> 
> Testing
> -------
> 
> ant test && manual
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>