Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail at rowSelection and_then method #2925

Closed
Ted-Jiang opened this issue Oct 25, 2022 · 3 comments · Fixed by #2928
Closed

Fail at rowSelection and_then method #2925

Ted-Jiang opened this issue Oct 25, 2022 · 3 comments · Fixed by #2928
Labels
bug parquet Changes to the parquet crate

Comments

@Ted-Jiang
Copy link
Member

Describe the bug

let a = RowSelection::from(vec![
            RowSelector::select(3),
            RowSelector::skip(33),
            RowSelector::select(3),
            RowSelector::skip(33),
        ]);
        let b = RowSelection::from(vec![RowSelector::select(36), RowSelector::skip(36)]);

        assert_eq!(
            a.and_then(&b).selectors,
            vec![RowSelector::select(3), RowSelector::skip(69)]
        );

got

---- arrow::arrow_reader::selection::tests::test_and_2 stdout ----
thread 'arrow::arrow_reader::selection::tests::test_and_2' panicked at 'called `Option::unwrap()` on a `None` value', parquet/src/arrow/arrow_reader/selection.rs:233:38
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

To Reproduce

Expected behavior

Additional context

@tustvold
Copy link
Contributor

This appears to be caused by the second row selector being longer than expected, a only selects 6 rows and so b should only be 6 long, instead of 72.

This is a bug that should be fixed, but shouldn't impact correctness

@viirya
Copy link
Member

viirya commented Oct 26, 2022

Hmm, if and_then takes the conjunction from two row selections, why b should be only 6 long? It looks like b must be in the selection range of a.

viirya pushed a commit that referenced this issue Oct 26, 2022
* Improve panic messages for RowSelection::and_then (#2925)

* Review feedback
@alamb
Copy link
Contributor

alamb commented Oct 28, 2022

label_issue.py automatically added labels {'parquet'} from #2924

@alamb alamb added the parquet Changes to the parquet crate label Oct 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug parquet Changes to the parquet crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants