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

Introducing ArrowNativeTypeOp made it impossible to call kernels from generics #2839

Closed
bjchambers opened this issue Oct 6, 2022 · 0 comments · Fixed by #2840
Closed

Introducing ArrowNativeTypeOp made it impossible to call kernels from generics #2839

bjchambers opened this issue Oct 6, 2022 · 0 comments · Fixed by #2840
Assignees
Labels
arrow Changes to the arrow crate bug

Comments

@bjchambers
Copy link
Contributor

I previously had generic code along the lines of the following (it was actually in evaluator structs, as part of implementing a query evaluator):

fn<T: ArrowNumericType> perform_math(lhs: &PrimitiveArray<T>, rhs: &PrimitiveArray<T>) -> anyhow::Result<ArrayRef>
  where T::Native: std::ops::Add<Output = T::Native> {
  Ok(Arc::new(arrow::compute::add::<T>(lhs.as_ref(), rhs.as_ref())?)
}

Now, the arrow::compute::add kernel (and many others) require T::Native: ArrowNativeTypeOp. I tried to update my code to add that to the where clause, but it isn't possible, since the ArrowNativeTypeOp is a public trait inside a crate-scoped module.

As far as I can tell this makes it impossible to call any of the kernels that require ArrowNativeTypeOp from within code that is generic over the numeric type... which seems like it would be pretty useful. Could the ArrowNativeTypeOp be re-exported to support declaring such constraints?

@bjchambers bjchambers added the bug label Oct 6, 2022
@viirya viirya self-assigned this Oct 7, 2022
@alamb alamb added the arrow Changes to the arrow crate label Oct 14, 2022
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