GH-40911: [C++][Compute] Fix the decimal division kernel dispatching

[zanmato1984]

Rationale for this change

The issues in #40911 and #39875 are the same: we have a fundamental defect when dispatching kernels for decimal division. The following statements assume both the dividend and the divisor are of the same decimal type (Decimal32/64/128/256), with possibly different (p, s).

• When doing DispatchBest, which is directly invoked through CallFunction("divide", ...), w/o trying DispatchExact ahead, the dividend is ALWAYS promoted and the result will have the same (p, s) as the dividend, according to the rule listed in our documentation [1] (this is actually adopting the Redshift one [2]).
• When doing DispatchExact, which is first tried by expression evaluation, there will be a match w/o any promotions so the subsequent try of DispatchMatch won't happen.

The issue is obvious - DispatchExact and DispatchBest are conflicting - one saying "OK, for any decimal128(p1, s1) / decimal128(p2, s2), it is a match" and the other saying "No, we must promote the dividend according to (p1, s1) and (p2, s2)".

Then we actually have two choices to fix it:

1. Consider DispatchBest is doing the right thing (justified by [1]), and NEVER "exact match" any kernel for decimal division. This is what this PR does. The only problem is that we are basically ALWAYS rejecting a kernel to be "exactly matched" - weird, though functionally correct.
2. Consider DispatchExact is doing the right thing, and NOT promoting dividend in DispatchBest. The kernel is matched only based on their decimal type (not considering their (p, s)). And only the result is promoted (this also complies [1]). This is what the other attempting PR GH-40911: [C++] Remove decimal division's precision and scale calculate logic from implicit casts #40969 does. But that PR only claims a promoted result type w/o actually promoting the computation (i.e., the memory representation of a decimal needs to be promoted when doing the division) so the result is wrong. Though this is amendable by supporting basic decimal methods like PromoteAndDivide that does the promotion of the dividend and the division all together in one run, the modification can be cumbersome - the "scale up" needs to be propagated from the kernel definition all down to the basic decimal primitives. Besides, I assume this may not be as performant as doing batch promotion + batch division.

[1] https://docs.aws.amazon.com/redshift/latest/dg/r_numeric_computations201.html#r_numeric_computations201-precision-and-scale-of-computed-decimal-results
[2] https://arrow.apache.org/docs/cpp/compute.html#arithmetic-functions

What changes are included in this PR?

Suppress the DispatchExact for decimal division.

Also, the match constraint BinaryDecimalScale1GeScale2 introduced in #47297 becomes useless thus gets removed.

Are these changes tested?

Yes.

Are there any user-facing changes?

None.

• GitHub Issue: [C++] Decimal divide promotion rule output wrong precision and scale in expression Bind when scale1 >= scale2 #40911

[zanmato1984]

We don't really need this constraint to suppress the exact matching as this is now done via overridden DispatchExact.

[pitrou]

Why not a DecimalsHaveSameScaleAndPrecision? (or full type equality, which is exactly equivalent here)

[zanmato1984]

That's the tricky part about decimal division - there is no "exact match" at all.

By definition we ALWAYS promote the dividend no matter their (p, s) are. For example, decimal(5, 1) / decimal(5, 1) = decimal(11, 6).

As long as we allow any exact match, the promotion won't happen.

[pitrou]

Well, {decimal(5, 1), decimal(5, 1)} looks like an exact match in this example. The result type is unrelated to this.

[pitrou]

Or you mean the dividend gets promoted to decimal(11, 6)?

[zanmato1984]

Or you mean the dividend gets promoted to decimal(11, 6)?

Exactly. Except that it is actually promoted to decimal(11, 7) but you get the idea.

[pitrou]

Hmm, thanks. Perhaps the PR description can be clearer about this?

[zanmato1984]

And this is how we obey the resulting type rule we claim - promoting the dividend.

That said, there is an alternative though - as you implied in your previous comment

looks like an exact match in this example. The result type is unrelated to this.

This is also explained in my PR description approach 2. I didn't take that approach because that would require the promotion to happen during the underlying division for each individual value in the array. Can be cumbersome in terms of both coding and performance.

[pitrou]

Ok, thanks for the explanation!

[zanmato1984]

Some decimal division kernel dispatching tests need to update. Will do.

cc @pitrou @bkietz @ZhangHuiGui

[zanmato1984]

The fix is now complete. @pitrou @bkietz @westonpace @ZhangHuiGui do you want to take a look? Thanks.

[pitrou]

Perhaps the test would be more interesting if those types had different precisions?

[zanmato1984]

This is not directly related to this PR though, updated by more interesting combinations of (p, s).

[pitrou]

Thank you.

[pitrou]

Thanks a lot for the elaborate explanations and answers @zanmato1984 . This is looking good and CI failures look unrelated.

[conbench-apache-arrow]

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 2e2aa0b.

There weren't enough matching historic benchmark results to make a call on whether there were regressions.

The full Conbench report has more details.