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 Mon, 20 Mar 2017 08:18:29 GMT

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



Hi Eric,

Thanks for the corrections.

I ran 'ant clean test' with your patch and TestHiveImport failed for me so please review your
changes there.

Besides that, I had some minor findings related to naming conventions.

Thanks,
Bogi


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

    Thanks for splitting up the test case, however, could you please use a self-explanatory
naming? testGetCreateTableStmtDecimal1 and testGetCreateTableStmtDecimal2 don't bring a clear
message when some runs the test and these fail.



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

    Is it still needed to use decimal_column1 name instead of decimal_column? It could be
misleading in my opinion. This applies to the other test case too.


- Boglarka Egyed


On March 20, 2017, 8:13 a.m., Eric Lin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57551/
> -----------------------------------------------------------
> 
> (Updated March 20, 2017, 8:13 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/TestHiveImport.java 1d67a2d 
>   src/test/com/cloudera/sqoop/hive/TestTableDefWriter.java 4db629f 
>   testdata/hive/scripts/decimalImport.q PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/57551/diff/3/
> 
> 
> Testing
> -------
> 
> Test case + maunaul test
> 
> 
> Thanks,
> 
> Eric Lin
> 
>


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