pitrou commented on PR #35996: URL: https://github.com/apache/arrow/pull/35996#issuecomment-1582941222
Hmm, so there are two (unrelated?) changes here. The first change (removing the `cArray->release != null` checks in export functions) looks fine. The second change is IMHO the symptom of an implementation issue. `ImportedArrowArray` currently stores the given raw pointer `CArrowArray* _cArray`. Instead, it should probably store its own `CArrowArray` struct like the C++ importer does: https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/bridge.cc#L1241-L1261 Then, importing an array _moves_ the user-supplied structure to the owned structure, like this: https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/bridge.cc#L1285-L1287 Therefore, the imported array structure does not depend on the lifetime of the user-supplied pointer. By the way, moving is cheap so you shouldn't fear any inefficiency here: https://github.com/apache/arrow/blob/e920bed4cc0f7826cf979a36283fceed403d2860/cpp/src/arrow/c/helpers.h#L66-L75 -- 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]
