Github user daveoshinsky commented on the issue:
https://github.com/apache/drill/pull/517
The overflow problem will require separate changes to fix, from any changes
we make in the short-term to fix this issue (see DRILL-4834). Â However, if I
understand some of Paul's findings earlier, the two issues are related as
follows. Â If we choose a large precision based only on the kind of integer
that is input to the cast (either an int, or a long, but not minimized to
represent the integer value to be casted), then we are more likely to encounter
the overflow issue, because this precision is more likely to exceed the
capacity of the destination decimal of the cast. Â Is that right, Paul? Â Given
the risk of overflow, it is safer to choose the absolute minimum precision
possible, which is what the changes I made to CastIntDecimal.java are doing. Â
We can't avoid overflow in every situation, but we can try to minimize the
chances of it. Â My understanding is that the actual integer value to be casted
isn't available in ExpressionTreeMaterializer.java; at least, I did not see
how to get it. Â And that integer value is required to compute the minimum
precision to represent that value. Â I don't see how to fix this problem
reliably in ExpressionTreeMaterializer.java, as I said earlier. Â If you
(Jinfeng) know how to do that, why don't you just take the changes in PR-517,
and then check changes in on top of those to fix the problem your way? Â At
least, you get my unit test, which was for a condition (DRILL-4704) that was
never tested by any earlier unit test.
On Monday, August 8, 2016 3:25 PM, Jinfeng Ni
<[email protected]> wrote:
I'm not fully convinced that we should check the precision for each input
value for this castIntDecimal function. The argument of proposed patch is
parameter "precision=0" is not valid, and has to be calculated on-the-fly or
"dynamically" for each input value. To me, 1) if "precision=0" is a wrong input
to this function, then we should fix the place where precision=0 is passed in.
2) why would you treat "precision=0" only? what if I pass in "precision = 1",
and a integer value of 123456? in such case, do we think "precison=1" is valid
or not? Regarding the overflow problem, that seems to be a separate issue, and
probably is true for most of the existing decimal functions. â
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
