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.

Sign up for GitHub

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

Jump to bottom

Add a parquet writer using Rust and CXX for FFI #194

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add a parquet writer using Rust and CXX for FFI #194

wants to merge 12 commits into from

Conversation

the80srobot
Copy link
Contributor

@the80srobot the80srobot commented Dec 31, 2024 •
edited
Loading

This adds a fairly MVP writer for Parquet files implemented in Rust and usable from C++. An actual Santa logger is just a stub, because this diff is already large. (And yet, I managed to shave off 1000 lines compared to my original PR from 2023.)

This XL PR could be split into one that adds Rust build support and one that adds the Parquet writer, but both of those things informed each other, so it actually seems easier to review them together. Sorry for the size, I won't make it a habit.

Some notes:

  • The patch for cxx is unfortunate, however the WORKSPACE -> MODULE migration seemingly broke everything and I couldn't find a combination of rules_rust and cxx that was both stable and easy to use with MODULE.bazel. The good news is that rules_rust 0.57.0 should fix everything, allowing me to update cxx and drop the patch. (Probably in a few weeks.)
  • The writer is fairly low-level. Pete's experiments AFAIK are with the more high-level arrow API. We should chat about trade-offs. I don't have a strong opinion on which is better. This PR does not lock us into one approach.

Test plan:

There is an e2e test shell script, which requires Docker.

@github-actions github-actions bot added docs Improvements or additions to documentation comp/santad Issues or PRs related to the daemon size/m Size: medium size/l Size: large size/xl Size: extra large and removed size/m Size: medium size/l Size: large labels Dec 31, 2024
@the80srobot
Add Rust build support
80bdb07 
This should be most of what we need to have some
packages written in Rust. It also manages to
depend on the parquet2 Crate, and proves that the
FFI works and Rust functions can be statically
linked with objc++ code.
@the80srobot
Depend on cxx.rs for C++/Rust interop
d1b5915
@the80srobot
Import cxx:bridge as a proof of concept
6381974
@the80srobot
Get rust-analyzer working by default
faf49a2
@the80srobot
Check in a rustfmt config
e417599
@the80srobot
Basic parquet2 logger + test
0a954f2
@the80srobot
Add support for parquet strings
bfcc9d9
@the80srobot
Full MVP implementation of a parquet writer
dfd1f0b 
This adds a proper parquet writer in Rust. The CPP
header is empty, and all the API is generated as a
Cxx bridge from cpp_api.rs, which I find rather
nice.
@the80srobot
Add a release config for Rust code
998627d
@the80srobot
Add an e2e test for the Parquet writer
d177819
@the80srobot the80srobot marked this pull request as ready for review December 31, 2024 19:03
@the80srobot the80srobot requested a review from a team as a code owner December 31, 2024 19:03
@the80srobot the80srobot changed the title DRAFT/WIP - Add parquet2 Rust crate for parquet output Add parquet2 Rust crate for parquet output Dec 31, 2024
@the80srobot the80srobot changed the title Add parquet2 Rust crate for parquet output Add a parquet writer using Rust and CXX for FFI Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comp/santad Issues or PRs related to the daemon docs Improvements or additions to documentation size/xl Size: extra large
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@the80srobot