Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Fixed error in computing projection in io::ipc::read::reader::FileReader #596

Merged
merged 3 commits into from
Nov 10, 2021
+3 −5
Merged

Fixed error in computing projection in io::ipc::read::reader::FileReader #596

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
594a8af
Allow projection to be set to 0 in io::ipc::read::reader::FileReader:…
illumination-k Nov 10, 2021
acfa396
fix: for_each returns ();
illumination-k Nov 10, 2021
83b8db0
change 1 to 0 in /~https://github.com/jorgecarleitao/arrow2/blob/main/t…
illumination-k Nov 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/io/ipc/read/reader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,11 @@ impl<R: Read + Seek> FileReader<R> {
/// Panics iff the projection is not in increasing order (e.g. `[1, 0]` nor `[0, 1, 1]` are valid)
pub fn new(reader: R, metadata: FileMetadata, projection: Option<Vec<usize>>) -> Self {
if let Some(projection) = projection.as_ref() {
let _ = projection.iter().fold(0, |mut acc, v| {
projection.windows(2).for_each(|x| {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks wonderful! Maybe we can add a test for this to make sure it doesn't panic now?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointer for the tests: /~https://github.com/jorgecarleitao/arrow2/blob/main/tests/it/io/ipc/read/file.rs#L181 (changing from 1 to 0 should be enough :) )

assert!(
*v > acc,
x[0] < x[1],
"The projection on IPC must be ordered and non-overlapping"
);
acc = *v;
acc
});
}
let projection = projection.map(|projection| {
Expand Down