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

Regression: Incorrect Sorting of *ListArray in 46.0.0 #4746

Closed
ChristianBeilschmidt opened this issue Aug 29, 2023 · 2 comments · Fixed by #4747
Closed

Regression: Incorrect Sorting of *ListArray in 46.0.0 #4746

ChristianBeilschmidt opened this issue Aug 29, 2023 · 2 comments · Fixed by #4747
Assignees
Labels
arrow Changes to the arrow crate bug

Comments

@ChristianBeilschmidt
Copy link
Contributor

Describe the bug

Arrow 46.0 leads to a change in sort_to_indices for FixedSizeListArrays. From my point of view, this is wrong.

To Reproduce

I created a minimal example:

fn sort_example() {
    let a = {
        let mut builder = FixedSizeListBuilder::new(Int64Builder::new(), 2);

        for value in [[1, 5], [0, 3], [1, 3]] {
            builder.values().append_slice(&value);
            builder.append(true);
        }

        builder.finish()
    };

    // dbg!(&a);

    let sort_options = Some(arrow::compute::SortOptions {
        descending: false,
        nulls_first: false,
    });

    let sort_indices = arrow::compute::sort_to_indices(&a, sort_options, None).unwrap();

    let array_ref = arrow::compute::take(&a, &sort_indices, None).unwrap();

    let b: &FixedSizeListArray = array_ref.as_fixed_size_list();

    // dbg!(&b);

    let c = {
        let mut builder = FixedSizeListBuilder::new(Int64Builder::new(), 2);

        for value in [[0, 3], [1, 3], [1, 5]] {
            builder.values().append_slice(&value);
            builder.append(true);
        }

        builder.finish()
    };

    assert_eq!(b, &c);
}

…leads for b being…

FixedSizeListArray<2>
[
  PrimitiveArray<Int64>
[
  0,
  3,
],
  PrimitiveArray<Int64>
[
  1,
  5,
],
  PrimitiveArray<Int64>
[
  1,
  3,
],
]

Expected behavior

b should be…

FixedSizeListArray<2>
[
  PrimitiveArray<Int64>
[
  0,
  3,
],
  PrimitiveArray<Int64>
[
  1,
  3,
],
  PrimitiveArray<Int64>
[
  1,
  5,
],
]

…as it was in arrow 45.0.

Additional context

I haven't checked if it is sort_to_indices or take, but I assume the first one.

@tustvold
Copy link
Contributor

tustvold commented Aug 29, 2023

Possibly introduced by a81da6c

Edit: This issue also applies to ListArray

@tustvold tustvold changed the title sort_to_indices for FixedSizeListArray does not sort ascendingly in 46.0 Regression: Incorrect Sorting of *ListArray in 46.0.0 Aug 29, 2023
@tustvold tustvold self-assigned this Aug 29, 2023
@tustvold tustvold added the arrow Changes to the arrow crate label Sep 18, 2023
@tustvold
Copy link
Contributor

label_issue.py automatically added labels {'arrow'} from #4747

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants