-
Notifications
You must be signed in to change notification settings - Fork 101
Implement missing FFI functions for array writing #5304
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
base: develop
Are you sure you want to change the base?
Implement missing FFI functions for array writing #5304
Conversation
Implements FFI builder functions for creating various Vortex array types: - **Primitive arrays**: Support for all primitive types (u8, u16, u32, u64, i8, i16, i32, i64, f16, f32, f64) with optional validity masks - **Bool arrays**: Boolean array creation with validity support - **Decimal arrays**: Decimal128 array creation with precision and scale - **Null arrays**: Simple null array construction - **UTF8/Binary arrays**: Builder-pattern API for variable-length string and binary data - **Struct arrays**: Construction from field arrays and struct dtype Adds `write-vortex.c` example demonstrating: - Creating arrays of different types using the new builder APIs - Building complex struct arrays with multiple fields - Writing arrays to Vortex files using the sink API - Proper resource management and error handling Resolves #5275
Codecov Report❌ Patch coverage is
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| printf("Creating i32 array (ages)...\n"); | ||
| int32_t ages_data[] = {25, 30, 35, 40, 45}; | ||
| bool ages_validity[] = {true, true, false, true, true}; // 3rd value is null | ||
| size_t row_count = sizeof(ages_data) / sizeof(ages_data[0]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't aware you could sizeof an expression! Does it have to be a valid index? does sizeof ages_data[10] work?
| data: *const bool, | ||
| len: usize, | ||
| validity: *const bool, | ||
| error_out: *mut *mut vx_error, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the need for this has to be my least favorite part of C
|
|
||
| for (i, &value) in data_slice.iter().enumerate() { | ||
| if !validity.is_null() { | ||
| let validity_slice = unsafe { slice::from_raw_parts(validity, len) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this free? It seems at least for legibility's sake, we should lift it maybe to line 101?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhhhhhh ass, right, pointers. Can we switch on null-ness here and have two loops? That avoids the branch in the loop in the non-null case.
|
This was fully Claude-coded and has not yet been reviewed by me, I'll invest some time in this today to hopefully get it ready to be a real PR |
|
This PR is still missing the header file declarations for cinlude. |
Implements FFI builder functions for creating various Vortex array types:
Adds
write-vortex.cexample demonstrating:Resolves #5275