-
Notifications
You must be signed in to change notification settings - Fork 4.1k
[C++] DCHECK_EQ failed in ResolveDecimalAdditionOrSubtractionOutput function when execute two different decimal scale arrays' add #40126
Description
Describe the bug, including details regarding any error messages, version, and platform.
How to reproduce
Add two different precision and scale decimal arrays through expression.
auto expr = arrow::compute::call("add", {arrow::compute::field_ref("l1"),
arrow::compute::field_ref("r1")});
auto s1 = arrow::schema({arrow::field("l1", arrow::decimal128(30, 3)),
arrow::field("r1", arrow::decimal128(20, 9))});
ASSERT_OK_AND_ASSIGN(expr, expr.Bind(*s1)); // crash
auto b = arrow::RecordBatchFromJSON(s1, R"([
["1.000", "-1.000000000"],
["-123456789012345678901234567.890", "12345678901.234567890"]
])");
ASSERT_OK_AND_ASSIGN(auto input, arrow::compute::MakeExecBatch(*s1, b));
ASSERT_OK_AND_ASSIGN(auto res_dat,
arrow::compute::ExecuteScalarExpression(expr, input));The test will crash at below function's DCHECK_EQ because the input types' scale is different during arrow::compute::Expression::Bind.
Result<TypeHolder> ResolveDecimalAdditionOrSubtractionOutput(
KernelContext*, const std::vector<TypeHolder>& types) {
return ResolveDecimalBinaryOperationOutput(
types, [](int32_t p1, int32_t s1, int32_t p2, int32_t s2) {
DCHECK_EQ(s1, s2);
const int32_t scale = s1;
const int32_t precision = std::max(p1 - s1, p2 - s2) + scale + 1;
return std::make_pair(precision, scale);
});
}The CallFunction is correct.
Reason
The expression's bind function will skip DispatchBest in BindNonRecursive. Because DispatchExact will return ok and skip the DispatchBest which overrided by ArithmeticFunction.
// First try and bind exactly
Result<const Kernel*> maybe_exact_match = call.function->DispatchExact(types);
if (maybe_exact_match.ok()) {
call.kernel = *maybe_exact_match;
}So the implicit cast in ArithmeticFunction::DispatchBest for decimal will be skipped.
But the logic is correct in CallFunction which will call DispatchBest first.
If we want to let user do cast by theirselves in expression call, we should return an error if we checked current situation? Or return not-ok in DispatchExact for decimal special case?
Component(s)
C++