You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Jefffrey (via GitHub)" <gi...@apache.org> on 2024/04/01 00:12:32 UTC
[I] UnionArray::try_new is not strict enough when validating inputs [arrow-rs]
Jefffrey opened a new issue, #5577:
URL: https://github.com/apache/arrow-rs/issues/5577
**Describe the bug**
<!--
A clear and concise description of what the bug is.
-->
We can create invalid UnionArrays when using `try_new(...)` as the validation checks done are not strict enough.
**To Reproduce**
<!--
Steps to reproduce the behavior:
-->
Run test:
```rust
#[test]
fn test_invalid_type_ids() {
let int_array = Arc::new(Int32Array::from(vec![5, 6]));
let f32_array = Arc::new(Float64Array::from(vec![10.0]));
let type_ids = [1_i8, 0, 2];
let offsets = [0_i32, 0, 0];
let type_id_buffer = Buffer::from_slice_ref(type_ids);
let value_offsets_buffer = Buffer::from_slice_ref(offsets);
let children: Vec<(Field, Arc<dyn Array>)> = vec![
(Field::new("A", DataType::Int32, false), int_array),
(Field::new("B", DataType::Float64, false), f32_array),
];
let array = UnionArray::try_new(
&[0, 1, 2],
type_id_buffer,
Some(value_offsets_buffer),
children,
)
.unwrap();
dbg!(&array);
dbg!(array.len());
dbg!(array.value(2));
}
```
Get output:
```shell
arrow-rs$ cargo test -p arrow-array --lib array::union_array::tests::test_invalid_type_ids -- --exact --nocapture
Finished test [unoptimized + debuginfo] target(s) in 0.05s
Running unittests src/lib.rs (target/debug/deps/arrow_array-d885d14cd41402de)
running 1 test
[arrow-array/src/array/union_array.rs:851:9] &array = UnionArray(Dense)
[
-- type id buffer:
ScalarBuffer([1, 0, 2])
-- offsets buffer:
ScalarBuffer([0, 0, 0])
-- child 0: "A" (Int32)
PrimitiveArray<Int32>
[
5,
6,
]
-- child 1: "B" (Float64)
PrimitiveArray<Float64>
[
10.0,
]
]
[arrow-array/src/array/union_array.rs:852:9] array.len() = 3
thread 'array::union_array::tests::test_invalid_type_ids' panicked at arrow-array/src/array/union_array.rs:232:9:
assertion failed: (type_id as usize) < self.fields.len()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test array::union_array::tests::test_invalid_type_ids ... FAILED
failures:
failures:
array::union_array::tests::test_invalid_type_ids
test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 427 filtered out; finished in 0.00s
error: test failed, to rerun pass `-p arrow-array --lib`
```
**Expected behavior**
<!--
A clear and concise description of what you expected to happen.
-->
Array should fail to create. It suggests an array with length 3 was created but panics when trying to fetch the 2nd index.
**Additional context**
<!--
Add any other context about the problem here.
-->
Note that some of the documentation is misleading too. For example, on `new_unchecked(...)`:
https://github.com/apache/arrow-rs/blob/9f36c883459405ecd9a5f4fdfa9a3317ab52302c/arrow-array/src/array/union_array.rs#L127-L129
- Though this is for `new_unchecked(...)`, it implies these inputs are validated by `try_new(...)`
However this is contradicted by the following tests:
https://github.com/apache/arrow-rs/blob/9f36c883459405ecd9a5f4fdfa9a3317ab52302c/arrow-array/src/array/union_array.rs#L740-L769
- Here it shows we can provide 0-values in the `type_ids` buffer
https://github.com/apache/arrow-rs/blob/9f36c883459405ecd9a5f4fdfa9a3317ab52302c/arrow/tests/array_transform.rs#L485-L497
- Here it shows we can provide values greater than the length of the Vec of child arrays passed in, for the `type_ids` buffer
--
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: github-unsubscribe@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] UnionArray::try_new is not strict enough when validating inputs [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on issue #5577:
URL: https://github.com/apache/arrow-rs/issues/5577#issuecomment-3600364552
Potentially closed by #5623, need to double check it covers all the points raised in this issue (or if some other PR addressed them)
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] UnionArray::try_new is not strict enough when validating inputs [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey closed issue #5577: UnionArray::try_new is not strict enough when validating inputs
URL: https://github.com/apache/arrow-rs/issues/5577
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] UnionArray::try_new is not strict enough when validating inputs [arrow-rs]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on issue #5577:
URL: https://github.com/apache/arrow-rs/issues/5577#issuecomment-3678406565
Resolved by #5623 and #8434
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org