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

arrow2::io::ipc::read::reader::FileReader cannot set 0 as projection #594

Closed
illumination-k opened this issue Nov 10, 2021 · 3 comments · Fixed by #596
Closed

arrow2::io::ipc::read::reader::FileReader cannot set 0 as projection #594

illumination-k opened this issue Nov 10, 2021 · 3 comments · Fixed by #596
Labels
bug Something isn't working no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog

Comments

@illumination-k
Copy link
Contributor

Hi, Thanks for the very excellent project!

I would like to parse IPC files, but I cannot set 0 as projection like arrow2::io::ipc::read::reader::FileReader::new(&mut reader, metadata, vec![0, 1]). The cause of this seems to come from

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| {
assert!(
*v > acc,
"The projection on IPC must be ordered and non-overlapping"
);

If projection is vec![0, 1], both acc and v equal 0, so error occurred.

It seems that code like this is correct,

let _ = projection.iter().fold(0, |mut acc, v| {
    if !(acc == 0 && *v == 0) {
        assert!( 
            *v > acc, 
            "The projection on IPC must be ordered and non-overlapping" 
        ); 
    }
    acc = *v;
    acc
});

Can I create the PR?

@jorgecarleitao
Copy link
Owner

Hey! Thanks for the report. Sure, shoot it and I will give it a review :)

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Nov 10, 2021
@Dandandan
Copy link
Collaborator

What about changing the initial acc to -1 ;)?

Even better / idiomatic would be to use windows(2) over the slice.

@illumination-k
Copy link
Contributor Author

Thanks for your kind comments. I open the pr according to the comments!

@jorgecarleitao jorgecarleitao added the no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog label Nov 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants