You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Yash Sharma <ya...@gmail.com> on 2014/05/28 21:05:35 UTC
Review Request 21985: Drill-708: trunc(num) bugfix
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21985/
-----------------------------------------------------------
Review request for drill, Aditya Kishore, Mehant Baid, and Timothy Chen.
Repository: drill-git
Description
-------
Drill-708: trunc(num) bugfix
Diffs
-----
exec/java-exec/src/main/codegen/data/MathFunc.tdd 228d207
Diff: https://reviews.apache.org/r/21985/diff/
Testing
-------
Yes. On Sqlline.
Thanks,
Yash Sharma
Re: Review Request 21985: Drill-708: trunc(num) bugfix
Posted by Yash Sharma <ya...@gmail.com>.
> On May 30, 2014, 7 a.m., Mehant Baid wrote:
> > exec/java-exec/src/main/codegen/data/MathFunc.tdd, line 87
> > <https://reviews.apache.org/r/21985/diff/1/?file=597748#file597748line87>
> >
> > Wouldn't this cause a loss of data? Double can represent approximately 1.8 * (10 ^ 308), simply casting it to long (which can represent only 19 digits) will cause loss of integer digits. Your change specifically isn't introducing this problem, seems like this issue always existed.
> >
> > Might be better to use a java function to truncate the fractional digits but still return double / float depending on the input.
> >
> >
Are you suggesting to use truncation code like below in our function implementation:
BigDecimal formatted = new BigDecimal(String.valueOf(num)).setScale(0, BigDecimal.ROUND_FLOOR);
Or,
DecimalFormat format = new DecimalFormat("#");
format.setRoundingMode(RoundingMode.FLOOR);
String formatted = format.format(num);
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21985/#review44338
-----------------------------------------------------------
On May 28, 2014, 7:05 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21985/
> -----------------------------------------------------------
>
> (Updated May 28, 2014, 7:05 p.m.)
>
>
> Review request for drill, Aditya Kishore, Mehant Baid, and Timothy Chen.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Drill-708: trunc(num) bugfix
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/data/MathFunc.tdd 228d207
>
> Diff: https://reviews.apache.org/r/21985/diff/
>
>
> Testing
> -------
>
> Yes. On Sqlline.
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 21985: Drill-708: trunc(num) bugfix
Posted by Yash Sharma <ya...@gmail.com>.
> On May 30, 2014, 7 a.m., Mehant Baid wrote:
> > exec/java-exec/src/main/codegen/data/MathFunc.tdd, line 87
> > <https://reviews.apache.org/r/21985/diff/1/?file=597748#file597748line87>
> >
> > Wouldn't this cause a loss of data? Double can represent approximately 1.8 * (10 ^ 308), simply casting it to long (which can represent only 19 digits) will cause loss of integer digits. Your change specifically isn't introducing this problem, seems like this issue always existed.
> >
> > Might be better to use a java function to truncate the fractional digits but still return double / float depending on the input.
> >
> >
>
> Yash Sharma wrote:
> Are you suggesting to use truncation code like below in our function implementation:
>
> BigDecimal formatted = new BigDecimal(String.valueOf(num)).setScale(0, BigDecimal.ROUND_FLOOR);
>
> Or,
> DecimalFormat format = new DecimalFormat("#");
> format.setRoundingMode(RoundingMode.FLOOR);
> String formatted = format.format(num);
Added DRILL-988 for bug fix for long.
https://reviews.apache.org/r/22607/
Return type same as passed types. Marking issue as wont fix.
- Yash
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21985/#review44338
-----------------------------------------------------------
On May 28, 2014, 7:05 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21985/
> -----------------------------------------------------------
>
> (Updated May 28, 2014, 7:05 p.m.)
>
>
> Review request for drill, Aditya Kishore, Mehant Baid, and Timothy Chen.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Drill-708: trunc(num) bugfix
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/data/MathFunc.tdd 228d207
>
> Diff: https://reviews.apache.org/r/21985/diff/
>
>
> Testing
> -------
>
> Yes. On Sqlline.
>
>
> Thanks,
>
> Yash Sharma
>
>
Re: Review Request 21985: Drill-708: trunc(num) bugfix
Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21985/#review44338
-----------------------------------------------------------
exec/java-exec/src/main/codegen/data/MathFunc.tdd
<https://reviews.apache.org/r/21985/#comment78708>
Wouldn't this cause a loss of data? Double can represent approximately 1.8 * (10 ^ 308), simply casting it to long (which can represent only 19 digits) will cause loss of integer digits. Your change specifically isn't introducing this problem, seems like this issue always existed.
Might be better to use a java function to truncate the fractional digits but still return double / float depending on the input.
- Mehant Baid
On May 28, 2014, 7:05 p.m., Yash Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21985/
> -----------------------------------------------------------
>
> (Updated May 28, 2014, 7:05 p.m.)
>
>
> Review request for drill, Aditya Kishore, Mehant Baid, and Timothy Chen.
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> Drill-708: trunc(num) bugfix
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/codegen/data/MathFunc.tdd 228d207
>
> Diff: https://reviews.apache.org/r/21985/diff/
>
>
> Testing
> -------
>
> Yes. On Sqlline.
>
>
> Thanks,
>
> Yash Sharma
>
>