sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <egyedb...@gmail.com>
Subject Re: Review Request 57551: SQOOP-2272 - Import decimal columns from mysql to hive 0.14
Date Sat, 18 Mar 2017 12:42:38 GMT

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



Hi Eric,

Thanks for your contribution!

I had some minor findings related to your change, please find them below.

On the top of them, could you please add a Hive import test case too for your use case for
example in TestHiveImport class?

Many thanks,
Bogi


src/java/org/apache/sqoop/hive/TableDefWriter.java
Line 23 (original), 23 (patched)
<https://reviews.apache.org/r/57551/#comment241723>

    java.sql.Types shown as an unused import for me when I'm applying your patch.



src/java/org/apache/sqoop/hive/TableDefWriter.java
Line 24 (original), 24 (patched)
<https://reviews.apache.org/r/57551/#comment241722>

    Please avoid using wildcard imports based on our common guidelines and add java.util.ArrayList,
java.util.Date and java.util.Properties imports back.



src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java
Lines 38 (patched)
<https://reviews.apache.org/r/57551/#comment241724>

    Please avoid using wildcard imports based on our common guidelines.



src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java
Lines 256 (patched)
<https://reviews.apache.org/r/57551/#comment241727>

    This test case looks great, however, I would split it into two smaller test case to make
it easier to maintain in the future: one for info1 and one for info2 as these test two different
behavior.


- Boglarka Egyed


On March 13, 2017, 10:02 a.m., Eric Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57551/
> -----------------------------------------------------------
> 
> (Updated March 13, 2017, 10:02 a.m.)
> 
> 
> Review request for Sqoop, Attila Szabo and Szabolcs Vasas.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Currently Sqoop converts DECIMAL from RDMS into DOUBLE in Hive, which is not correct
as user will lose precisions. Since Hive supports DECIMAL long ago, we should support DECIMAL
to DECIMAL conversion from Sqoop to Hive.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/hive/HiveTypes.java ad00535 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 32fcca3 
>   src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java 4db629f 
> 
> 
> Diff: https://reviews.apache.org/r/57551/diff/1/
> 
> 
> Testing
> -------
> 
> Test case + maunaul test
> 
> 
> Thanks,
> 
> Eric Lin
> 
>


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