Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-12 Thread Boglarka Egyed 

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


Ship it!




Ran unit and 3rd party tests successfully for the updated patch too.

- Boglarka Egyed


On Sept. 12, 2017, 12:19 p.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62057/
> ---
> 
> (Updated Sept. 12, 2017, 12:19 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3014
> https://issues.apache.org/jira/browse/SQOOP-3014
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
> doesn't change BigDecimals
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 
> aba2458e 
>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java d784a205 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62057/diff/3/
> 
> 
> Testing
> ---
> 
> I ran unit tests and integration tests as well. New test cases were added to 
> test the change
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-12 Thread Szabolcs Vasas 

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


Ship it!




Ship It!

- Szabolcs Vasas


On Sept. 12, 2017, 12:19 p.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62057/
> ---
> 
> (Updated Sept. 12, 2017, 12:19 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3014
> https://issues.apache.org/jira/browse/SQOOP-3014
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
> doesn't change BigDecimals
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 
> aba2458e 
>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java d784a205 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62057/diff/3/
> 
> 
> Testing
> ---
> 
> I ran unit tests and integration tests as well. New test cases were added to 
> test the change
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-12 Thread Zoltán Tóth 

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

(Updated Sept. 12, 2017, 12:19 p.m.)


Review request for Sqoop, Boglarka Egyed and Anna Szonyi.


Changes
---

Change updated based on review


Bugs: SQOOP-3014
https://issues.apache.org/jira/browse/SQOOP-3014


Repository: sqoop-trunk


Description
---

HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
doesn't change BigDecimals


Diffs (updated)
-

  src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java aba2458e 
  src/test/org/apache/sqoop/hcat/HCatalogImportTest.java d784a205 
  src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/62057/diff/3/

Changes: https://reviews.apache.org/r/62057/diff/2-3/


Testing
---

I ran unit tests and integration tests as well. New test cases were added to 
test the change


Thanks,

Zoltán Tóth

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-11 Thread Szabolcs Vasas 

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




src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java
Lines 449 (patched)


I think this method could be non-static.



src/test/org/apache/sqoop/hcat/HCatalogImportTest.java
Lines 404 (patched)


Can you please extract the magic constants to make this test more readable?



src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java
Lines 28 (patched)


Nit: please remove this extra new line here.



src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java
Lines 31 (patched)


Nit: please remove these extra 2 lines here.


- Szabolcs Vasas


On Sept. 5, 2017, 3:32 p.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62057/
> ---
> 
> (Updated Sept. 5, 2017, 3:32 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3014
> https://issues.apache.org/jira/browse/SQOOP-3014
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
> doesn't change BigDecimals
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 
> aba2458e 
>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java d784a205 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62057/diff/2/
> 
> 
> Testing
> ---
> 
> I ran unit tests and integration tests as well. New test cases were added to 
> test the change
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-07 Thread Boglarka Egyed 

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


Ship it!




Ship It!

- Boglarka Egyed


On Sept. 5, 2017, 3:32 p.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62057/
> ---
> 
> (Updated Sept. 5, 2017, 3:32 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3014
> https://issues.apache.org/jira/browse/SQOOP-3014
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
> doesn't change BigDecimals
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 
> aba2458e 
>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java d784a205 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62057/diff/2/
> 
> 
> Testing
> ---
> 
> I ran unit tests and integration tests as well. New test cases were added to 
> test the change
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-05 Thread Boglarka Egyed 

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



Hi Zoltan,

Thank you for this fix!

Could you please add an integration test too where you perform an actual sqoop 
import testing the specific cases? Check out HCatalogImportTest for example.

Thanks,
Bogi

- Boglarka Egyed


On Sept. 5, 2017, 3:32 p.m., Zoltán Tóth wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62057/
> ---
> 
> (Updated Sept. 5, 2017, 3:32 p.m.)
> 
> 
> Review request for Sqoop, Boglarka Egyed and Anna Szonyi.
> 
> 
> Bugs: SQOOP-3014
> https://issues.apache.org/jira/browse/SQOOP-3014
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
> doesn't change BigDecimals
> 
> 
> Diffs
> -
> 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java 
> aba2458e 
>   src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
> PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62057/diff/1/
> 
> 
> Testing
> ---
> 
> I ran unit tests and integration tests as well. New test cases were added to 
> test the change
> 
> 
> Thanks,
> 
> Zoltán Tóth
> 
>

Re: Review Request 62057: SQOOP-3014 Sqoop with HCatalog import loose precision for large numbers that does not fit into double

2017-09-04 Thread Zoltán Tóth 

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

(Updated Sept. 4, 2017, 3:26 p.m.)


Review request for Sqoop and Boglarka Egyed.


Bugs: SQOOP-3014
https://issues.apache.org/jira/browse/SQOOP-3014


Repository: sqoop-trunk


Description
---

HCatalog rounded BigDecimals but that should not happen. Now Sqoop HCatalog 
doesn't change BigDecimals


Diffs
-

  src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java aba2458e 
  src/test/org/apache/sqoop/mapreduce/hcat/TestSqoopHCatImportHelper.java 
PRE-CREATION 


Diff: https://reviews.apache.org/r/62057/diff/1/


Testing
---

I ran unit tests and integration tests as well. New test cases were added to 
test the change


Thanks,

Zoltán Tóth