sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <b...@apache.org>
Subject Re: Review Request 64371: SQOOP-3233: SqoopHCatImportHelper.convertNumberTypes check for Varchar instead of Char
Date Wed, 06 Dec 2017 16:20:23 GMT

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


Ship it!




Hi Fero,

Thanks for fixing the patch file.

Your change looks good to me, also the unit and 3rd party tests passed with it.

Thanks for fixing this bug!

Cheers,
Bogi

- Boglarka Egyed


On Dec. 6, 2017, 2:13 p.m., Fero Szabo wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64371/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2017, 2:13 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3233
>     https://issues.apache.org/jira/browse/SQOOP-3233
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> - Corrected the type from VARCHAR to CHAR in SqoopHCatImportHelper.convertNumberTypes,
that the ticket description mentions.
> - Found another bug in the convertNumberTypes function: missing parentheses in the if
clause that checks whether the type of val is BigDecimal. Corrected it.
> - Slight refactoring: extracted the BigDecimal and Number related conversion code into
two different methods, for better readability.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 1c1ed1e58c1e1c9a0364e61627ac1ef725bea8f9

>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java 4686493c79498d9ca45db227904049736b5ea493

> 
> 
> Diff: https://reviews.apache.org/r/64371/diff/2/
> 
> 
> Testing
> -------
> 
> Ran unit tests (ant clean test), and third party tests as well successfully.
> 
> 
> Thanks,
> 
> Fero Szabo
> 
>


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