Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.2k
ARROW-13067: [C++][Compute] Implement integer to decimal cast#11045
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -391,6 +391,53 @@ struct CastFunctor<O, I, | ||
| } | ||
| }; | ||
| // ---------------------------------------------------------------------- | ||
| // Integer to decimal | ||
| struct IntegerToDecimal { | ||
| template <typename OutValue, typename IntegerType> | ||
| OutValue Call(KernelContext*, IntegerType val, Status* st) const { | ||
| auto maybe_decimal = OutValue(val).Rescale(0, out_scale_); | ||
| if (ARROW_PREDICT_TRUE(maybe_decimal.ok())) { | ||
| return maybe_decimal.MoveValueUnsafe(); | ||
| } | ||
| *st = maybe_decimal.status(); | ||
| return OutValue{}; | ||
cyb70289 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| int32_t out_scale_; | ||
| }; | ||
| template <typename O, typename I> | ||
| struct CastFunctor<O, I, | ||
| enable_if_t<is_decimal_type<O>::value && is_integer_type<I>::value>> { | ||
| static Status Exec(KernelContext* ctx, const ExecBatch& batch, Datum* out) { | ||
| const auto& out_type = checked_cast<const O&>(*out->type()); | ||
| const auto out_scale = out_type.scale(); | ||
| const auto out_precision = out_type.precision(); | ||
| // verify precision and scale | ||
| if (out_scale < 0) { | ||
| return Status::Invalid("Scale must be non-negative"); | ||
| } | ||
| // maximal number of decimal digits for int8/16/32/64 | ||
| constexpr std::array<int, 4> decimal_digits{3, 5, 10, 19}; | ||
cyb70289 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| using ctype = typename I::c_type; | ||
| static_assert(sizeof(ctype) <= 8, ""); | ||
| const int precision = decimal_digits[BitUtil::Log2(sizeof(ctype))] + out_scale; | ||
| if (out_precision < precision) { | ||
| return Status::Invalid( | ||
| "Precision is not great enough for the result. " | ||
| "It should be at least ", | ||
| precision); | ||
| } | ||
| applicator::ScalarUnaryNotNullStateful<O, I, IntegerToDecimal> kernel( | ||
| IntegerToDecimal{out_scale}); | ||
| return kernel.Exec(ctx, batch, out); | ||
| } | ||
| }; | ||
| // ---------------------------------------------------------------------- | ||
| // Decimal to decimal | ||
| @@ -641,6 +688,12 @@ std::shared_ptr<CastFunction> GetCastToDecimal128() { | ||
| DCHECK_OK(func->AddKernel(Type::DOUBLE, {float64()}, sig_out_ty, | ||
| CastFunctor<Decimal128Type, DoubleType>::Exec)); | ||
| // Cast from integer | ||
| for (const std::shared_ptr<DataType>& in_ty : IntTypes()) { | ||
| auto exec = GenerateInteger<CastFunctor, Decimal128Type>(in_ty->id()); | ||
| DCHECK_OK(func->AddKernel(in_ty->id(), {in_ty}, sig_out_ty, std::move(exec))); | ||
| } | ||
cyb70289 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Cast from other decimal | ||
| auto exec = CastFunctor<Decimal128Type, Decimal128Type>::Exec; | ||
| // We resolve the output type of this kernel from the CastOptions | ||
| @@ -664,6 +717,12 @@ std::shared_ptr<CastFunction> GetCastToDecimal256() { | ||
| DCHECK_OK(func->AddKernel(Type::DOUBLE, {float64()}, sig_out_ty, | ||
| CastFunctor<Decimal256Type, DoubleType>::Exec)); | ||
| // Cast from integer | ||
| for (const std::shared_ptr<DataType>& in_ty : IntTypes()) { | ||
| auto exec = GenerateInteger<CastFunctor, Decimal256Type>(in_ty->id()); | ||
| DCHECK_OK(func->AddKernel(in_ty->id(), {in_ty}, sig_out_ty, std::move(exec))); | ||
| } | ||
cyb70289 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // Cast from other decimal | ||
| auto exec = CastFunctor<Decimal256Type, Decimal128Type>::Exec; | ||
| DCHECK_OK( | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -71,9 +71,11 @@ static std::vector<std::shared_ptr<DataType>> kNumericTypes = { | ||
| uint8(), int8(), uint16(), int16(), uint32(), | ||
| int32(), uint64(), int64(), float32(), float64()}; | ||
| static std::vector<std::shared_ptr<DataType>> kDictionaryIndexTypes = { | ||
| static std::vector<std::shared_ptr<DataType>> kIntegerTypes = { | ||
| int8(), uint8(), int16(), uint16(), int32(), uint32(), int64(), uint64()}; | ||
| static std::vector<std::shared_ptr<DataType>> kDictionaryIndexTypes = kIntegerTypes; | ||
| static std::vector<std::shared_ptr<DataType>> kBaseBinaryTypes = { | ||
| binary(), utf8(), large_binary(), large_utf8()}; | ||
| @@ -587,6 +589,36 @@ TEST(Cast, Decimal256ToInt) { | ||
| CheckCast(negative_scale, ArrayFromJSON(int64(), "[1234567890000, -120000]"), options); | ||
| } | ||
| TEST(Cast, IntegerToDecimal) { | ||
| for (auto decimal_type : {decimal128(21, 2), decimal256(21, 2)}) { | ||
| for (auto integer_type : kIntegerTypes) { | ||
| CheckCast( | ||
| ArrayFromJSON(integer_type, "[0, 7, null, 100, 99]"), | ||
| ArrayFromJSON(decimal_type, R"(["0.00", "7.00", null, "100.00", "99.00"])")); | ||
| } | ||
| } | ||
| // extreme value | ||
| for (auto decimal_type : {decimal128(19, 0), decimal256(19, 0)}) { | ||
| CheckCast(ArrayFromJSON(int64(), "[-9223372036854775808, 9223372036854775807]"), | ||
| ArrayFromJSON(decimal_type, | ||
| R"(["-9223372036854775808", "9223372036854775807"])")); | ||
| CheckCast(ArrayFromJSON(uint64(), "[0, 18446744073709551615]"), | ||
| ArrayFromJSON(decimal_type, R"(["0", "18446744073709551615"])")); | ||
| } | ||
| // insufficient output precision | ||
| { | ||
| CastOptions options; | ||
| options.to_type = decimal128(5, 3); | ||
| CheckCastFails(ArrayFromJSON(int8(), "[0]"), options); | ||
| options.to_type = decimal256(76, 67); | ||
| CheckCastFails(ArrayFromJSON(int32(), "[0]"), options); | ||
| } | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we file a follow up issue to allow truncation via an unsafe cast in these cases? ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, will do. | ||
| } | ||
| TEST(Cast, Decimal128ToDecimal128) { | ||
| CastOptions options; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, so an
int64is cast to a 1-digit decimal? Unrelated to this PR surely, but I'm a bit surprised :-)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is testing that given an integer and a decimal, the integer is cast to a decimal of the same width/precision/scale.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But surely the actual cast from int64 to decimal128(1,0) will fail in the precision check?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. Maybe we want the cast logic to be smarter about that, then. As you said, though, that can be tackled separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jira issue created
https://issues.apache.org/jira/browse/ARROW-13856