[C++] DCHECK_EQ failed in ResolveDecimalAdditionOrSubtractionOutput function when execute two different decimal scale arrays' add

[ZhangHuiGui]

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++

[ZhangHuiGui]

@westonpace I have seen your response in #35843, maybe we could solve the problem internal rather than restricting user behavior(eg. explicit call cast for decimal compute in expressions)?

[westonpace]

@westonpace I have seen your response in #35843, maybe we could solve the problem internal rather than restricting user behavior(eg. explicit call cast for decimal compute in expressions)?

I'm not sure I understand. Are you suggesting we visit the expression tree and insert casts when receive expressions involving decimals?

[bkietz]

Issue resolved by pull request 40223
#40223