[C++][Compute] Inaccurate calculation results will be produced when the 'cases' arrays of 'case_when' have different Decimal types

[ZhangHuiGui]

Describe the bug, including details regarding any error messages, version, and platform.

It's an interesting problem we find recently in case_when kernel.

For the call function:
CaseWhen({MakeStruct({cond1, cond2, cond3}), {arr1, arr2, arr3, arr4}})
will produce the case_when logic like below:

cond1 --> arr1 decimal(9,7),
cond2 --> arr2 decimal(6,2),
cond3 --> arr3 decimal(3, 2),
arr4 decimal(2,1)

The case_when will use the LastType which is decimal(2,1) as it's output type.

arrow/cpp/src/arrow/compute/kernels/scalar_if_else.cc

Lines 2683 to 2685 in 79799e5

	ScalarKernel kernel(
	KernelSignature::Make({InputType(Type::STRUCT), InputType(get_id.id)}, LastType,
	/*is_varargs=*/true),

.

That means for the arr1/arr2/arr3's output results, the precision will be set to decimal(2,1), obviously it's inaccurate in fact especially when the case_when as an expression combining with aggregate's sum/avg....

laotan332 qinpengxiang@outlook.com

Component(s)

C++

[zanmato1984]

Interesting finding.

I think using LastType for case_when output type resolution isn't the real issue. Because when doing exact match, it is reasonable to assume that all branches have the same type, thus assuming any of them to be the output should be fine. However an exception is for parametric types, they can be of the same type yet not exactly the same - differing on their type parameters (e.g., decimal128(9, 7) VS. decimal128(3, 2)).

We used to be shorthand handling such cases, until #47297 introduced a way to suppress exact matching by using customized match constraints. So what we can do for case_when is: apply a constraint that requires all the branch decimals have the same p and s, so that cases in the issue description won't be an exact match. Then the matching naturally falls into DispatchBest and implicit casts will be applied.

Let me try to do that in a PR.

[zanmato1984]

Hi @ZhangHuiGui , I see you have a PR for this issue. However I believe we have a better way to fix it using #47297. Would you consider doing that in your PR? Or do you mind if I work on a new fix? Thanks.

[ZhangHuiGui]

Hi @ZhangHuiGui , I see you have a PR for this issue. However I believe we have a better way to fix it using #47297. Would you consider doing that in your PR? Or do you mind if I work on a new fix? Thanks.

Your method is more elegant and you can submit a separate PR. Thank you very much.

[zanmato1984]

Issue resolved by pull request 47479
#47479