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



exec/java-exec/src/main/codegen/templates/MathFunctions.java
<https://reviews.apache.org/r/22607/#comment83144>

    Instead of converting the input to String and passing it to BigDecimal do 
you think it will be better if you use BigDecimal.valueOf(double value) ?



exec/java-exec/src/main/codegen/templates/MathFunctions.java
<https://reviews.apache.org/r/22607/#comment83143>

    You are addressing two issues here. 
    
    1. If the second input to the mod function is 0 then your output is the 
first input as is, this looks good.
    
    2. You are trying to set the scale of the output by using BigDecimal to be 
the same as the first input. The problem is when you get the float/ double 
value from BigDecimal there is no guarantee that the same scale will be 
maintained because float/ double is imprecise and it may end up using a much 
larger scale. 
    
    I think it'll be best to modify mod function to add your change to address 
the first issue you solved and remove the changes for the second issue.


- Mehant Baid


On July 3, 2014, 6:59 p.m., Yash Sharma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22607/
> -----------------------------------------------------------
> 
> (Updated July 3, 2014, 6:59 p.m.)
> 
> 
> Review request for drill and Mehant Baid.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> Drill-988 - Trunc func injects a long cast which would cause loss of data
> Added BigDecimal for truncation and return type is same as passed params.
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/codegen/data/MathFunc.tdd b5b5543 
>   exec/java-exec/src/main/codegen/templates/MathFunctions.java c4298fb 
>   
> exec/java-exec/src/test/java/org/apache/drill/exec/fn/impl/TestNewMathFunctions.java
>  3289601 
>   exec/java-exec/src/test/resources/functions/testDivModTruncFunctions.json 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22607/diff/
> 
> 
> Testing
> -------
> 
> Via Sqlline
> 
> 
> Thanks,
> 
> Yash Sharma
> 
>

Reply via email to