Arrow c data interface

[Alex-PLACET]

Fix #69

[paleolimbot]

Very cool! Just some notes from writing a few things along these lines in nanoarrow 🙂

[paleolimbot]

I would highly recommend avoiding std::unique_ptr<ArrowSchema/ArrowArray>, unless used with a deleter that ensures that any unreleased array that it is pointing to is released when the unique pointer is deleted. In cudf they use a unique pointer with a deleter:

https://github.com/rapidsai/cudf/blob/e58838b6cc820fc89f1f67eb9117a3ee6ddeaa47/cpp/src/interop/to_arrow_device.cu#L601-L604

...in nanoarrow we have a UniqueArray/Schema class that wraps a stack-allocated ArrowArray/Schema:

https://github.com/apache/arrow-nanoarrow/blob/a0632177c27cb52880c62533fcd441613f41815b/src/nanoarrow/nanoarrow.hpp#L265-L272

..but the idea is the same: always make a home for the struct such that it is guaranteed to be cleaned up, and "move" it between safe homes (e.g., using something like https://github.com/apache/arrow-nanoarrow/blob/a0632177c27cb52880c62533fcd441613f41815b/src/nanoarrow/nanoarrow_types.h#L334-L340 ).

(Apologies if there's some C++ magic going on here that already ensures this)

[Alex-PLACET]

Sorry for the delay in replying.
Thank you for your comments. The PR was improved and use a deleter 👍

[paleolimbot]

All of the implementations of an ArrowArray/ArrowSchema I've read choose to own their own memory for the ArrowArray/ArrowSchema structs. In this case, it might mean an std::move() of the std::vector<std::unique_ptr<ArrowArray>> into the private data and rely on the C++ deleter to call the release callbacks (see my comment below about always making sure that std::unique_ptr<ArrowThing> is guaranteed to call the release callback if present).

Alternatively, one could use new ArrowArray[] and something like https://github.com/apache/arrow-nanoarrow/blob/a0632177c27cb52880c62533fcd441613f41815b/src/nanoarrow/nanoarrow_types.h#L334-L340 to "move" the struct from foreign memory.

[JohanMabille]

Would it be possible to split the declaration and implementation of public API functions? Ideally all the public API functions should be at the top of the file, so one can quicky check the API without scroling the whole file.

[paleolimbot]

I think that it may still be needed to include:

#ifndef ARROW_C_DATA_INTERFACE
#define ARROW_C_DATA_INTERFACE

...to prevent a duplicate definition in the case where somebody does:

#include <adbc.h>
#include <sparrow/c_interface.hpp>

[Alex-PLACET]

Definitely, I fixed that

[paleolimbot]

I should have noticed this on the first round, but if one includes adbc.h first, the helpful initializers you've added to the definition won't be there (I am not sure if this affects the code you've written here but might have unexpected consequences for users of this header).

[paleolimbot]

Should this be std::vector<std::unique_ptr<ArrowArray, arrow_array_custom_deleter>>? (It may help reduce the number of lines of code where an exception or early return results in leaked ArrowArrays)

[Alex-PLACET]

You are right. Fixed

[paleolimbot]

Should this be std::vector<std::unique_ptr<ArrowSchema, arrow_schema_custom_deleter>>? (It may help reduce the number of lines of code where an exception or early return results in leaked ArrowSchemas)

[paleolimbot]

Awesome!