samtalki commented on PR #603: URL: https://github.com/apache/arrow-julia/pull/603#issuecomment-4918743454
@kou threw Fable 5 and force pushed one more revision before review and restacked. Everything in my previous comment, including my question, still stands. The change in this revision is that I made the import ownership model match the conventions from https://github.com/apache/arrow-rs and https://github.com/apache/arrow-nanoarrow . ### summary of final state - `Arrow.from_c_data` now moves the base `ArrowSchema`/`ArrowArray` structures into Julia owned storage and marks the sources released (`release = NULL`) without calling their callbacks, per the spec's [moving an array](https://arrow.apache.org/docs/format/CDataInterface.html#moving-an-array) semantics. - This matches arrow-rs (`from_raw` via `ptr::replace`), nanoarrow (`ArrowArrayMove`), and Arrow C++/PyArrow. - Previously the importer borrowed the caller's struct addresses until release, so the common pattern of exporting into stack allocated structs and freeing them after the handoff would have released through a dangling pointer at finalization. - Only the **two fixed size headers are copied** and data buffers are still viewed in place. ### new test coverage - sources marked released at import without their callbacks running, - source structures reusable immediately after import, - exactly one producer release call, - rejection of re-importing a moved-from source. ### conclusion The rest of the stacked PRs are also rebased on this with very minor changes. All C Data tests, the full test suite with bounds checks, the C producer smoke test, and the GC stress tests pass on every branch of the stack. - one change was made to the export PR: https://github.com/apache/arrow-julia/pull/605 it now also additionally releases already built children when a later child fails to build, so a failed nested export cannot leak owner registry entries. - I considered accepting `null_count == -1` with a NULL validity bitmap. I decided to keep the strict behavior that matches the validation behavior in [nanoarrow](https://github.com/apache/arrow-nanoarrow), and the Arrow spec's rule that a NULL validity buffer requires null_count 0. https://arrow.apache.org/rust/arrow_buffer/buffer/struct.NullBuffer.html -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
