Describe the bug, including details regarding any error messages, version, and platform.
This is a recently discovered bug that was previously ignored. When we perform Bind on the division of two decimal types decimal(p1,s1), decimal(p2,s2), except for the case of s1<s2, the other output decimal types's precision and scale are both wrong.(Different with our expect rules : https://arrow.apache.org/docs/cpp/compute.html#arithmetic-functions)
Simple reproduce:
auto arr1 = arrow::ArrayFromJSON(decimal128(6, 1), R"([
"0.7", "2.9", "3.1"
])");
auto arr2 = arrow::ArrayFromJSON(decimal128(7, 2), R"([
"-2.17", "9.11", "12.31"
])");
auto expr = call("divide", {field_ref("a"), field_ref("b")});
auto schema = arrow::schema({field("a", arrow::decimal128(7, 2)), field("b", decimal128(6, 1))});
ASSERT_OK_AND_ASSIGN(expr, expr.Bind(*schema));
ASSERT_OK_AND_ASSIGN(auto res, ExecuteScalarExpression(expr, ExecBatch({arr2, arr1}, arr1->length())));
std::cout << res.ToString() << std::endl;
ASSERT_OK_AND_ASSIGN(res, CallFunction("divide", ExecBatch({arr2, arr1}, arr1->length())));
std::cout << res.ToString() << std::endl;Output results with expression call and CallFunction are different.
Because the rule constraints in the output type resolver are not strict. Many situations, such as (s1=s2 || s2 > s2), will directly return success and skip the DispatchBest in BindNonRecursive, thus getting a wrong precision and scale.
|
int32_t s2) -> Result<std::pair<int32_t, int32_t>> { |
|
if (s1 < s2) { |
|
return Status::Invalid("Division of two decimal types scale1 < scale2. ", "(", |
|
s1, s2, ")."); |
|
} |
This means that we should make all decimal divisions go into DispatchBest, but we haven't thought of an elegant solution yet.
For example: The first time you enter ResolveDecimalDivisionOutput, it returns invalid, let the upper Bind go to dispatchBest, and the second time you enter it, it will execute the current logic.
Component(s)
C++
Describe the bug, including details regarding any error messages, version, and platform.
This is a recently discovered bug that was previously ignored. When we perform Bind on the division of two decimal types decimal(p1,s1), decimal(p2,s2), except for the case of s1<s2, the other output decimal types's precision and scale are both wrong.(Different with our expect rules : https://arrow.apache.org/docs/cpp/compute.html#arithmetic-functions)
Simple reproduce:
Output results with expression call and CallFunction are different.
Because the rule constraints in the output type resolver are not strict. Many situations, such as (s1=s2 || s2 > s2), will directly return success and skip the DispatchBest in BindNonRecursive, thus getting a wrong precision and scale.
arrow/cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
Lines 543 to 547 in 9f0101e
This means that we should make all decimal divisions go into DispatchBest, but we haven't thought of an elegant solution yet.
For example: The first time you enter ResolveDecimalDivisionOutput, it returns invalid, let the upper Bind go to dispatchBest, and the second time you enter it, it will execute the current logic.
Component(s)
C++