Fix offsets for Binary/Lists/LargeLists #727
Conversation
gsilvestrin
commented
Mar 23, 2023
•
gsilvestrin
commented
- Added unit tests
- Testes against the data sets of issue Error when querying a Lance dataset #720
gsilvestrin
requested review from
changhiskhan and
eddyxu
and removed request for
changhiskhan
|
|
||
| // Make sure the big array was not written to the file | ||
| let file_size_bytes = store.size(&path).await.unwrap(); | ||
| assert!(file_size_bytes < 1_000); |
There was a problem hiding this comment.
seems we just need to make 2000 values instead of 1M?
There was a problem hiding this comment.
I'm just writing an array that is so big that it will be obvious if the assert doesn't match (I think the actual file size is ~ 250 bytes)
There was a problem hiding this comment.
Yea, i mean 2000 elements is good enough for test. and it is faster in CI
rust/src/io/reader.rs
Outdated
| let large_list_array = LargeListArray::from_iter_primitive::<Int32Type, _, _>(vec![ | ||
| Some(vec![Some(10), Some(11)]), | ||
| Some(vec![Some(12), Some(13)]), | ||
| Some((0..1_000_000).map(|n| Some(n)).collect::<Vec<_>>()), |
There was a problem hiding this comment.
Do we have a test to actually write multiple arrays, wanna just make sure the cummulation of offsets is correct
There was a problem hiding this comment.
Not directly, but test_read_struct_of_list_arrays creates multiple structs that contains lists and write_struct_array should group them together.
|
|
||
| // Make sure the big array was not written to the file | ||
| let file_size_bytes = store.size(&path).await.unwrap(); | ||
| assert!(file_size_bytes < 1_000); |
There was a problem hiding this comment.
Yea, i mean 2000 elements is good enough for test. and it is faster in CI
