Skip to content

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

Description

@ZhangHuiGui

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(autores, 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) {
returnStatus::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++

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions