ARROW-4204: [Gandiva] add support for decimal subtract#3636
ARROW-4204: [Gandiva] add support for decimal subtract#3636pravindra wants to merge 1 commit intoapache:masterfrom
Conversation
| int i = 0; | ||
| for (auto& in_arg : function->args()) { | ||
| if (i == 3) { | ||
| auto y_neg_value = ir_builder()->CreateNeg(&in_arg); |
There was a problem hiding this comment.
I was wondering if this could overflow, but the decimal type is limited to 38 decimal digits, which is too small for 2**127.
As a side note, it seems we don't validate inputs to the Decimal128 type and happily accept larger numbers:
>>> ty = pa.decimal128(39, 0)
>>> arr = pa.array([(2**127) - 1], type=ty)
>>> arr
<pyarrow.lib.Decimal128Array object at 0x7f9b89444e08>
[
170141183460469231731687303715884105727
]
>>> arr = pa.array([(2**127)], type=ty)
>>> arr
<pyarrow.lib.Decimal128Array object at 0x7f9b89444138>
[
-170141183460469231731687303715884105728
]
>>> arr = pa.array([-(2**127)], type=ty)
>>> arr
<pyarrow.lib.Decimal128Array object at 0x7f9b89432688>
[
-170141183460469231731687303715884105728
]There was a problem hiding this comment.
I'll create a jira to ensure gandiva fails when the type has a precision/scale is > 38.
For the actual values itself, I can add a IsValid() api to BasicDecimal (that checks against a min/max value). but, checking this each time will be a perf overhead. maybe, we should have a conf variable to do overflow checks.
There was a problem hiding this comment.
Intuitively, I'd say people who use decimals value correctness. Though it's clear our C++ API currently doesn't check...
There was a problem hiding this comment.
I've created ARROW-4569 and ARROW-4570 to follow up on these.
2 participants
No description provided.