[
https://issues.apache.org/jira/browse/ARROW-11160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17261533#comment-17261533
]
Jorge Leitão commented on ARROW-11160:
--------------------------------------
Hi.
I am sorry, I can't understand what the issue is:
These functions are private implementations, which is why
1. they are marked as pub(super)
2. they are not exposed in `mod.rs`
3. there is a single public function on the whole equal package, `equal`.
I can't see how it is a problem to have private functions that require specific
conditions to return a useful result.
Secondly, the test above passes on my computer, so I can't reproduce it :/
Thirdly, we can find the following comment in `mod.rs`:
{code}
// these methods assume the same type, len and null count.
// For this reason, they are not exposed and are instead used
// to build the generic functions below (`equal_range` and `equal`).
use boolean::boolean_equal;
...
{code}
I.e. IMO we are doing everything right:
1. keep them as implementation details
2. expose only what needs to be exposed (and test it accordingly)
3. document the conditions that need to be fulfilled for the functions to be
useful
> [Rust] pitfall in xxx_equal functions
> -------------------------------------
>
> Key: ARROW-11160
> URL: https://issues.apache.org/jira/browse/ARROW-11160
> Project: Apache Arrow
> Issue Type: Improvement
> Reporter: Qingyou Meng
> Priority: Major
>
> Functions `pub(super) fn xxxx_equal` in rust/src/array/equal/ MUST fail if
> called directly instead of from `equal`. boolean.rs as example:
> {code:java}
> lhs_is_null || (lhs_is_null == rhs_is_null)
> && equal_bits(...){code}
>
> The bug is not seen in tests at present because `equal` shortcuts the
> condition: zero null s v.s. nulls
>
> To verify this problem , adds a new test in `mod.rs`
>
> {code:java}
> #[test]
> fn test_direct_call_boolean_equal() {
> let a = BooleanArray::from(vec![Some(true)]).data();
> let b = BooleanArray::from(vec![Some(false)]).data();
> let lhs = a.as_ref();
> let rhs = b.as_ref();
> let lhs_nulls = lhs.null_buffer();
> let rhs_nulls = rhs.null_buffer();
> assert_eq!(boolean::boolean_equal(lhs, rhs, lhs_nulls, rhs_nulls, 0, 0,
> lhs.len()), false);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
