[ 
https://issues.apache.org/jira/browse/ARROW-11160?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17261748#comment-17261748
 ] 

Qingyou Meng edited comment on ARROW-11160 at 1/9/21, 4:14 AM:
---------------------------------------------------------------

[~jorgecarleitao] thanks for taking time to reply!
 * Firstly, the test code had wrong data in array `a`, my bad! Fixed in 
[https://github.com/apache/arrow/compare/master...mqy:array-equal]  

 * Secondly, I totally agree with you about private functions and the comment 
in `mod.rs`, that's great!
 * Thirdly,  it may be less error-prune to update condition from:

 
{code:java}
lhs_is_null || (lhs_is_null == rhs_is_null) && non_null_equal(){code}
to

 
{code:java}
lhs_is_null == rhs_is_null && (lhs_is_null || non_null_equal()){code}
I re-pushed with several files changed, to demonstrating the failures at:
{code:java}
array::equal::tests::test_direct_call_boolean_equal
array::transform::tests::test_struct
{code}
Perhaps this is a hint to *possible hidden bug* somewhere, for example 
*MutableArrayData*::extend()? I'm sorry for taking your time again.

 


was (Author: mqy):
[~jorgecarleitao] thanks for taking time to reply!
 * Firstly, the test code had wrong data in array `a`, my bad! Fixed in 
[https://github.com/apache/arrow/compare/master...mqy:array-equal] 

 

 

 

* Secondly, I totally agree with you about private functions and the comment in 
`mod.rs`, that's great!
 * Thirdly,  it may be less error-prune to update condition from:

 
{code:java}
lhs_is_null || (lhs_is_null == rhs_is_null) && non_null_equal(){code}
to

 
{code:java}
lhs_is_null == rhs_is_null && (lhs_is_null || non_null_equal()){code}
I re-pushed with several files changed, to demonstrating the failures at:
{code:java}
array::equal::tests::test_direct_call_boolean_equal
array::transform::tests::test_struct
{code}
Perhaps this is a hint to *possible hidden bug* somewhere, for example 
*MutableArrayData*::extend()? I'm sorry for taking your time again.

 

> [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 nulls 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)

Reply via email to