zanmato1984 commented on code in PR #47459:
URL: https://github.com/apache/arrow/pull/47459#discussion_r2309097408
##########
cpp/src/arrow/compute/kernels/scalar_compare.cc:
##########
@@ -436,8 +436,8 @@ std::shared_ptr<ScalarFunction>
MakeCompareFunction(std::string name, FunctionDo
for (const auto id : {Type::DECIMAL128, Type::DECIMAL256}) {
auto exec = GenerateDecimal<applicator::ScalarBinaryEqualTypes,
BooleanType, Op>(id);
- DCHECK_OK(
- func->AddKernel({InputType(id), InputType(id)}, boolean(),
std::move(exec)));
+ DCHECK_OK(func->AddKernel({InputType(id), InputType(id)}, boolean(),
std::move(exec),
Review Comment:
Yep, the fix is as simple as this.
##########
cpp/src/arrow/compute/expression_test.cc:
##########
@@ -708,6 +712,42 @@ TEST(Expression, BindWithImplicitCasts) {
ExpectBindsTo(cmp(field_ref("i32"),
literal(std::make_shared<DoubleScalar>(10.0))),
cmp(cast(field_ref("i32"), float32()),
literal(std::make_shared<FloatScalar>(10.0f))));
+
+ // decimal int
+ ExpectBindsTo(cmp(field_ref("dec128_3_2"), field_ref("i64")),
+ cmp(field_ref("dec128_3_2"), cast(field_ref("i64"),
decimal128(21, 2))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("i64"), field_ref("dec128_3_2")),
+ cmp(cast(field_ref("i64"), decimal128(21, 2)),
field_ref("dec128_3_2")),
+ /*bound_out=*/nullptr, *exciting_schema);
+
+ // decimal128 decimal256 with different scales
+ ExpectBindsTo(
+ cmp(field_ref("dec128_3_2"), field_ref("dec256_5_3")),
+ cmp(cast(field_ref("dec128_3_2"), decimal256(4, 3)),
field_ref("dec256_5_3")),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(
+ cmp(field_ref("dec256_5_3"), field_ref("dec128_3_2")),
+ cmp(field_ref("dec256_5_3"), cast(field_ref("dec128_3_2"),
decimal256(4, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("dec128_5_3"), field_ref("dec256_3_2")),
+ cmp(cast(field_ref("dec128_5_3"), decimal256(5, 3)),
+ cast(field_ref("dec256_3_2"), decimal256(4, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("dec256_3_2"), field_ref("dec128_5_3")),
+ cmp(cast(field_ref("dec256_3_2"), decimal256(4, 3)),
+ cast(field_ref("dec128_5_3"), decimal256(5, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
Review Comment:
These cases will actually pass w/o this fix. Just added them for intact
coverage of all decimal casting paths.
##########
cpp/src/arrow/compute/expression_test.cc:
##########
@@ -708,6 +712,42 @@ TEST(Expression, BindWithImplicitCasts) {
ExpectBindsTo(cmp(field_ref("i32"),
literal(std::make_shared<DoubleScalar>(10.0))),
cmp(cast(field_ref("i32"), float32()),
literal(std::make_shared<FloatScalar>(10.0f))));
+
+ // decimal int
+ ExpectBindsTo(cmp(field_ref("dec128_3_2"), field_ref("i64")),
+ cmp(field_ref("dec128_3_2"), cast(field_ref("i64"),
decimal128(21, 2))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("i64"), field_ref("dec128_3_2")),
+ cmp(cast(field_ref("i64"), decimal128(21, 2)),
field_ref("dec128_3_2")),
+ /*bound_out=*/nullptr, *exciting_schema);
+
+ // decimal128 decimal256 with different scales
+ ExpectBindsTo(
+ cmp(field_ref("dec128_3_2"), field_ref("dec256_5_3")),
+ cmp(cast(field_ref("dec128_3_2"), decimal256(4, 3)),
field_ref("dec256_5_3")),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(
+ cmp(field_ref("dec256_5_3"), field_ref("dec128_3_2")),
+ cmp(field_ref("dec256_5_3"), cast(field_ref("dec128_3_2"),
decimal256(4, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("dec128_5_3"), field_ref("dec256_3_2")),
+ cmp(cast(field_ref("dec128_5_3"), decimal256(5, 3)),
+ cast(field_ref("dec256_3_2"), decimal256(4, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(cmp(field_ref("dec256_3_2"), field_ref("dec128_5_3")),
+ cmp(cast(field_ref("dec256_3_2"), decimal256(4, 3)),
+ cast(field_ref("dec128_5_3"), decimal256(5, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
+
+ // decimal decimal with different scales
+ ExpectBindsTo(
+ cmp(field_ref("dec128_3_2"), field_ref("dec128_5_3")),
+ cmp(cast(field_ref("dec128_3_2"), decimal128(4, 3)),
field_ref("dec128_5_3")),
+ /*bound_out=*/nullptr, *exciting_schema);
+ ExpectBindsTo(
+ cmp(field_ref("dec128_5_3"), field_ref("dec128_3_2")),
+ cmp(field_ref("dec128_5_3"), cast(field_ref("dec128_3_2"),
decimal128(4, 3))),
+ /*bound_out=*/nullptr, *exciting_schema);
Review Comment:
These two will fail w/o the fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
