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

Start to make Rust node API typed using arrow #353

Merged
merged 35 commits into from
Oct 26, 2023
Merged

Conversation

phil-opp
Copy link
Collaborator

@phil-opp phil-opp commented Sep 6, 2023

TODO:

  • update c++ api
  • provide convenience functions for constructing single-value arrays
  • provide convenience functions for downcasting received values to concrete primitives (or even higher-level Rust types)

Open questions:

  • How can we construct arrow arrays from Rust in an ergonomic way?
  • Is zero-copy construction possible?
  • How can we convert received data to higher-level types without copying?

@haixuanTao
Copy link
Collaborator

I think that this looks good in general. It was what I had in mind for a POC.

@haixuanTao haixuanTao mentioned this pull request Oct 19, 2023
10 tasks
- If input has no data, return an empty `NullArray` instead of an empty byte array.
- If input contains Vec data (instead of shared memory data), use safe `arrow::buffer::Buffer::from_vec` constructor, which is also zero-copy.
Empty `Vec`s have a dummy pointer that is set to the alignment of the item type. Thus, the pointer for empty `Vec<u8>` data is `0x1`. Using this pointer to construct a `ArrayData` instance can lead to alignment errors if the arrow array is of a type with larger alignment, e.g. float64.

This commit fixes the alignment error by checking for an empty raw buffer and constructing an empty `ArrayData` instance in this case.

Fixes #362
Ensures that all raw data has at least an alignment of 128.
@phil-opp phil-opp force-pushed the rust-typed-input branch 2 times, most recently from 6c90a54 to d2c8239 Compare October 25, 2023 10:45
@phil-opp phil-opp marked this pull request as ready for review October 25, 2023 12:23
Copy link
Collaborator

@haixuanTao haixuanTao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect. Works very well thanks!

@phil-opp
Copy link
Collaborator Author

Great, thanks for reviewing!

@phil-opp phil-opp merged commit 0ef994e into main Oct 26, 2023
17 checks passed
@phil-opp phil-opp deleted the rust-typed-input branch October 26, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants