Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparison of different ScalarValue types produces the silent errors #3511

Open
metesynnada opened this issue Sep 16, 2022 · 1 comment
Open
Labels
bug Something isn't working

Comments

@metesynnada
Copy link
Contributor

Describe the bug

PartialOrd implementation of ScalarValue types gives None for different types and it is automatically cast to false. I think this is the wrong behavior and introduces silent errors.

To Reproduce

Below test code shows the error

fn ord_diff_type() {
    assert_eq!(
        true,
        (ScalarValue::Float32(Some(2.0)) < ScalarValue::Int32(Some(3)))
    );
}

Expected behavior

I expected comparison to be true, however comparison (ScalarValue::Float32(Some(2.0)) < ScalarValue::Int32(Some(3))) returns false. Hence assertion fails. I think returning false here is misleading. This comparison either should produce an error or return true. If we do the comparison below it works as expected., since they have the same type

fn ord_same_type() {
    assert_eq!(
        true,
            (ScalarValue::Int32(Some(2))<
            ScalarValue::Int32(Some(3)))
    );
}

Additional context

I think casting the left or right side of the comparison to the larger type would solve the problem. The relevant section of the code is below
https://github.com/apache/arrow-datafusion/blob/66dd253d2e0cdd00c8d3611f2ca470b9cde48abc/datafusion/common/src/scalar.rs#L196

@HaoYang670
Copy link
Contributor

This comparison either should produce an error or return true.

I prefer returning an error because the casting should have been done in the type_coercion stage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants