rluvaton commented on code in PR #18921:
URL: https://github.com/apache/datafusion/pull/18921#discussion_r2971375268
##########
datafusion/functions-nested/src/array_transform.rs:
##########
Review Comment:
Please add the following tests because you have bug in both of them from
what I can see (and add similar tests for fixed size list)
```rust
fn create_i32_list(
values: impl Into<Int32Array>,
offsets: OffsetBuffer<i32>,
nulls: Option<NullBuffer>,
) -> GenericListArray<i32> {
let values = values.into();
let list_field = Arc::new(Field::new(
"item",
values.data_type().clone(),
values.is_nullable(),
));
GenericListArray::<i32>::new(Arc::clone(&list_field), offsets,
Arc::new(values), nulls)
}
#[test]
fn transform_on_sliced_list_should_not_evaluate_on_unreachable_values() {
let list = create_i32_list(
vec![
// Have 0 here so if the expression is called on data that it
will fail
0, 4, 100, 25, 20, 5, 2, 1, 10,
],
OffsetBuffer::<i32>::from_lengths(vec![1, 3, 4, 1]).slice(1, 3),
None,
);
let res = evaluate_array_transform(list, item -> 100 / item);
let ColumnarValue::Array(res) = res else {
panic!("expect array")
};
let actual_list = res.as_list::<i32>();
let expected_list = create_i32_list(
vec![25, 1, 4, 5, 20, 50, 100, 10],
OffsetBuffer::<i32>::from_lengths(vec![3, 4, 1]),
None,
);
assert_eq!(actual_list, &expected_list);
}
#[test]
fn transform_function_should_not_be_evaluated_on_values_underlying_null() {
let list = create_i32_list(
// 0 here for one of the values behind null, so if it will be
evaluated
// it will fail due to divide by 0
vec![100, 20, 10, 0, 1, 2, 0, 1, 50],
OffsetBuffer::<i32>::from_lengths(vec![3, 4, 2]),
Some(NullBuffer::from(vec![true, false, true])),
);
let schema = Arc::new(Schema::new(vec![Field::new(
"col_0",
list.data_type().clone(),
true,
)]));
let batch = RecordBatch::try_new(Arc::clone(&schema),
vec![Arc::new(list)]).unwrap();
let res = evaluate_array_transform(list, item -> 100 / item);
let res = transform_expr.evaluate(&batch).unwrap();
let ColumnarValue::Array(res) = res else {
panic!("expect array")
};
let actual_list = res.as_list::<i32>();
let expected_list = create_i32_list(
vec![1, 5, 10, 100, 2],
OffsetBuffer::<i32>::from_lengths(vec![3, 0, 2]),
Some(NullBuffer::from(vec![true, false, true])),
);
assert_eq!(actual_list, &expected_list);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
