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

compute::binary_mut returns Err(PrimitiveArray<T>) only with certain arrays #6374

Closed
MaxenceMaire opened this issue Sep 9, 2024 · 4 comments · Fixed by #6396
Closed

compute::binary_mut returns Err(PrimitiveArray<T>) only with certain arrays #6374

MaxenceMaire opened this issue Sep 9, 2024 · 4 comments · Fixed by #6396
Assignees
Labels
arrow Changes to the arrow crate bug

Comments

@MaxenceMaire
Copy link

Describe the bug
compute::binary_mut returns Err(PrimitiveArray<T>) in cases when the underlying buffer is not shared (as described in the documentation). It only happens when passing certain arrays.

To Reproduce

  • Err case:
let a = Int32Array::from(vec![
    Some(3),
    Some(4),
    Some(5),
    Some(6),
    None,
]);

let b = Int32Array::from(vec![
    Some(10),
    Some(11),
    Some(12),
    Some(13),
    Some(14),
]);

let a = binary_mut(a, &b, |a, b| a + b);

produces

Err(PrimitiveArray<Int32>
[
  3,
  4,
  5,
  6,
  null,
])
  • Ok case:
let a = Int32Array::from(vec![
    Some(3),
    Some(4),
    Some(5),
    Some(6),
    None,
]);

let b = Int32Array::from(vec![
    Some(10),
    Some(11),
    None,
    Some(13),
    Some(14),
]);

let a = binary_mut(a, &b, |a, b| a + b);

produces

Ok(Ok(PrimitiveArray<Int32>
[
  13,
  15,
  null,
  19,
  null,
]))

Expected behavior
I would have expected the first case to produce

Ok(Ok(PrimitiveArray<Int32>
[
  13,
  15,
  17,
  19,
  null,
]))

Additional context

Additional cases
  • Err case:
let a = Int32Array::from(vec![
    Some(3),
    Some(4),
    Some(5),
    Some(6),
    None,
    Some(8)
]);

let b = Int32Array::from(vec![
    Some(10),
    Some(11),
    Some(12),
    Some(13),
    Some(14),
    Some(15),
]);

let a = binary_mut(a, &b, |a, b| a + b);

produces

Err(PrimitiveArray<Int32>
[
  3,
  4,
  5,
  6,
  null,
  8,
])
  • Ok case:
let a = Int32Array::from(vec![
    Some(3),
    Some(4),
    Some(5),
    Some(6),
    None,
    Some(8)
]);

let b = Int32Array::from(vec![
    Some(10),
    Some(11),
    Some(12),
    Some(13),
    Some(14),
    None,
]);

let a = binary_mut(a, &b, |a, b| a + b);

produces

Ok(Ok(PrimitiveArray<Int32>
[
  13,
  15,
  17,
  19,
  null,
  null,
]))

The errors seem to be associated with an uneven number of null values.

@MaxenceMaire
Copy link
Author

Note that I get the expected result when constructing the array with a separate null buffer:

let a = Int32Array::new(
    vec![3, 4, 5, 6, 7].into(),
    Some(vec![true, true, true, true, false].into()),
);

let b = Int32Array::new(
    vec![10, 11, 12, 13, 14].into(),
    Some(vec![true, true, true, true, true].into()),
);

let a = binary_mut(a, &b, |a, b| a + b);

produces:

Ok(Ok(PrimitiveArray<Int32>
[
  13,
  15,
  17,
  19,
  null,
]))

Unfortunately I haven't had time to dig deeper into it yet, but it would seem to indicate that the null buffer references the scalar buffer when using the From<Vec<Option<T>>> trait to construct the array, which in turns causes the shared buffer error on binary_mut?

@alamb
Copy link
Contributor

alamb commented Sep 13, 2024

this sounds like a good find. Perhaps @viirya has some thoughts / ideas on this (as I think Comet uses binary_mut a lot)

@viirya
Copy link
Member

viirya commented Sep 13, 2024

Looking into this.

@viirya viirya self-assigned this Sep 14, 2024
@alamb alamb added the arrow Changes to the arrow crate label Oct 2, 2024
@alamb
Copy link
Contributor

alamb commented Oct 2, 2024

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

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.

3 participants