You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Qian Xu <sx...@googlemail.com> on 2014/09/04 06:36:44 UTC
Review Request 25326: SQOOP-1491: Refactoring AvroUtil and
SqoopAvroRecord
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25326/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-1491
https://issues.apache.org/jira/browse/SQOOP-1491
Repository: sqoop-trunk
Description
-------
This is a patch to SQOOP-1395. As the entity class name is no longer identical to the record name in Avro schema, there is not need to have SqoopAvroRecord. The patch proposes to do two things:
1. Remove SqoopAvroRecord
2. Move common Avro handling code to AvtoUtil
Diffs
-----
src/java/org/apache/sqoop/avro/AvroUtil.java 4b37d58
src/java/org/apache/sqoop/lib/SqoopAvroRecord.java 80875d2
src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 6adad79
src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 905ba8c
src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java effbadd
src/java/org/apache/sqoop/mapreduce/ParquetJob.java a74432a
src/java/org/apache/sqoop/orm/ClassWriter.java 4f9dedd
Diff: https://reviews.apache.org/r/25326/diff/
Testing
-------
All unittests passed. No new unittest added.
Manually tested couple of import tests.
Thanks,
Qian Xu
Re: Review Request 25326: SQOOP-1491: Refactoring AvroUtil and
SqoopAvroRecord
Posted by Jarek Cecho <ja...@apache.org>.
> On Sept. 5, 2014, 6:13 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java, lines 51-53
> > <https://reviews.apache.org/r/25326/diff/1/?file=676401#file676401line51>
> >
> > Both Parquet and Avro do have "Decimal" as a first class citizen now (for example check out AVRO-1402). I'm wondering if we could use that instead of serializing big decimal as string. But that is definitely work for separate JIRA.
Filled SQOOP-1493.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25326/#review52289
-----------------------------------------------------------
On Sept. 4, 2014, 4:38 a.m., Qian Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25326/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 4:38 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1491
> https://issues.apache.org/jira/browse/SQOOP-1491
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> This is a patch to SQOOP-1395. As the entity class name is no longer identical to the record name in Avro schema, there is not need to have SqoopAvroRecord. The patch proposes to do two things:
> 1. Remove SqoopAvroRecord (reverted ClassWriter to previous revision)
> 2. Move common Avro handling code to AvtoUtil
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroUtil.java 4b37d58
> src/java/org/apache/sqoop/lib/SqoopAvroRecord.java 80875d2
> src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 6adad79
> src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 905ba8c
> src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java effbadd
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java a74432a
> src/java/org/apache/sqoop/orm/ClassWriter.java 4f9dedd
>
> Diff: https://reviews.apache.org/r/25326/diff/
>
>
> Testing
> -------
>
> All unittests passed. No new unittest added.
> Manually tested couple of import tests.
>
>
> Thanks,
>
> Qian Xu
>
>
Re: Review Request 25326: SQOOP-1491: Refactoring AvroUtil and
SqoopAvroRecord
Posted by Jarek Cecho <ja...@apache.org>.
> On Sept. 5, 2014, 6:13 a.m., Jarek Cecho wrote:
> > src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java, line 91
> > <https://reviews.apache.org/r/25326/diff/1/?file=676400#file676400line91>
> >
> > Nit: Typo in the name "Arvo" should be "Avro" :) Fine to fix in follow up JIRA.
Filled SQOOP-1494.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25326/#review52289
-----------------------------------------------------------
On Sept. 4, 2014, 4:38 a.m., Qian Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25326/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 4:38 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1491
> https://issues.apache.org/jira/browse/SQOOP-1491
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> This is a patch to SQOOP-1395. As the entity class name is no longer identical to the record name in Avro schema, there is not need to have SqoopAvroRecord. The patch proposes to do two things:
> 1. Remove SqoopAvroRecord (reverted ClassWriter to previous revision)
> 2. Move common Avro handling code to AvtoUtil
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroUtil.java 4b37d58
> src/java/org/apache/sqoop/lib/SqoopAvroRecord.java 80875d2
> src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 6adad79
> src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 905ba8c
> src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java effbadd
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java a74432a
> src/java/org/apache/sqoop/orm/ClassWriter.java 4f9dedd
>
> Diff: https://reviews.apache.org/r/25326/diff/
>
>
> Testing
> -------
>
> All unittests passed. No new unittest added.
> Manually tested couple of import tests.
>
>
> Thanks,
>
> Qian Xu
>
>
Re: Review Request 25326: SQOOP-1491: Refactoring AvroUtil and
SqoopAvroRecord
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25326/#review52289
-----------------------------------------------------------
Ship it!
src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java
<https://reviews.apache.org/r/25326/#comment91054>
Nit: Typo in the name "Arvo" should be "Avro" :) Fine to fix in follow up JIRA.
src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java
<https://reviews.apache.org/r/25326/#comment91055>
Both Parquet and Avro do have "Decimal" as a first class citizen now (for example check out AVRO-1402). I'm wondering if we could use that instead of serializing big decimal as string. But that is definitely work for separate JIRA.
- Jarek Cecho
On Sept. 4, 2014, 4:38 a.m., Qian Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25326/
> -----------------------------------------------------------
>
> (Updated Sept. 4, 2014, 4:38 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-1491
> https://issues.apache.org/jira/browse/SQOOP-1491
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> This is a patch to SQOOP-1395. As the entity class name is no longer identical to the record name in Avro schema, there is not need to have SqoopAvroRecord. The patch proposes to do two things:
> 1. Remove SqoopAvroRecord (reverted ClassWriter to previous revision)
> 2. Move common Avro handling code to AvtoUtil
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/avro/AvroUtil.java 4b37d58
> src/java/org/apache/sqoop/lib/SqoopAvroRecord.java 80875d2
> src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 6adad79
> src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 905ba8c
> src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java effbadd
> src/java/org/apache/sqoop/mapreduce/ParquetJob.java a74432a
> src/java/org/apache/sqoop/orm/ClassWriter.java 4f9dedd
>
> Diff: https://reviews.apache.org/r/25326/diff/
>
>
> Testing
> -------
>
> All unittests passed. No new unittest added.
> Manually tested couple of import tests.
>
>
> Thanks,
>
> Qian Xu
>
>
Re: Review Request 25326: SQOOP-1491: Refactoring AvroUtil and
SqoopAvroRecord
Posted by Qian Xu <sx...@googlemail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25326/
-----------------------------------------------------------
(Updated Sept. 4, 2014, 12:38 p.m.)
Review request for Sqoop.
Bugs: SQOOP-1491
https://issues.apache.org/jira/browse/SQOOP-1491
Repository: sqoop-trunk
Description (updated)
-------
This is a patch to SQOOP-1395. As the entity class name is no longer identical to the record name in Avro schema, there is not need to have SqoopAvroRecord. The patch proposes to do two things:
1. Remove SqoopAvroRecord (reverted ClassWriter to previous revision)
2. Move common Avro handling code to AvtoUtil
Diffs
-----
src/java/org/apache/sqoop/avro/AvroUtil.java 4b37d58
src/java/org/apache/sqoop/lib/SqoopAvroRecord.java 80875d2
src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 6adad79
src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 905ba8c
src/java/org/apache/sqoop/mapreduce/ParquetImportMapper.java effbadd
src/java/org/apache/sqoop/mapreduce/ParquetJob.java a74432a
src/java/org/apache/sqoop/orm/ClassWriter.java 4f9dedd
Diff: https://reviews.apache.org/r/25326/diff/
Testing
-------
All unittests passed. No new unittest added.
Manually tested couple of import tests.
Thanks,
Qian Xu