paleolimbot commented on code in PR #702:
URL: https://github.com/apache/arrow-adbc/pull/702#discussion_r1227065879
##########
python/adbc_driver_manager/adbc_driver_manager/_lib.pyx:
##########
@@ -1132,3 +1180,85 @@ cdef class AdbcStatement(_AdbcHandle):
status = AdbcStatementSetSubstraitPlan(
&self.statement, c_plan, length, &c_error)
check_error(status, &c_error)
+
+
+# Implementation of an ArrowArrayStream that keeps a dependent object valid
+
+
+cdef struct ArrowArrayStreamWrapper:
+ cpython.PyObject* parent_statement
+ CArrowArrayStream* parent_array_stream
Review Comment:
I think you may want this struct to own the memory here rather than just a
pointer (i.e., `CArrowArrayStream parent_array_stream`).
##########
python/adbc_driver_manager/adbc_driver_manager/_lib.pyx:
##########
@@ -1132,3 +1180,85 @@ cdef class AdbcStatement(_AdbcHandle):
status = AdbcStatementSetSubstraitPlan(
&self.statement, c_plan, length, &c_error)
check_error(status, &c_error)
+
+
+# Implementation of an ArrowArrayStream that keeps a dependent object valid
+
+
+cdef struct ArrowArrayStreamWrapper:
+ cpython.PyObject* parent_statement
+ CArrowArrayStream* parent_array_stream
+ bint error_set
+
+
+cdef void wrapper_array_stream_release(CArrowArrayStream* array_stream) nogil
noexcept:
+ cdef ArrowArrayStreamWrapper* data
+
+ if array_stream.private_data != NULL:
+ data = <ArrowArrayStreamWrapper*>array_stream.private_data
+ data.parent_array_stream.release(data.parent_array_stream)
+
+ with gil:
+ cpython.Py_DECREF(<AdbcStatement>data.parent_statement)
+
+ free(array_stream.private_data)
+
+ array_stream.release = NULL
+
+
+cdef const char* wrapper_array_stream_get_last_error(CArrowArrayStream*
array_stream) nogil noexcept:
+ cdef ArrowArrayStreamWrapper* data =
<ArrowArrayStreamWrapper*>array_stream.private_data
+ if data.error_set:
+ return "AdbcStatement already closed"
+ return data.parent_array_stream.get_last_error(data.parent_array_stream)
+
+
+cdef int wrapper_array_stream_get_schema(CArrowArrayStream* array_stream,
CArrowSchema* out) nogil noexcept:
+ cdef ArrowArrayStreamWrapper* data =
<ArrowArrayStreamWrapper*>array_stream.private_data
+ if (<AdbcStatement>data.parent_statement).closed:
+ data.error_set = True
+ return EIO
+ return data.parent_array_stream.get_schema(data.parent_array_stream, out)
+
+
+cdef int wrapper_array_stream_get_next(CArrowArrayStream* array_stream,
CArrowArray* out) nogil noexcept:
+ cdef ArrowArrayStreamWrapper* data =
<ArrowArrayStreamWrapper*>(array_stream.private_data)
+ if (<AdbcStatement>data.parent_statement).closed:
+ data.error_set = True
Review Comment:
I am not sure that you need `error_set`: in theory, the array stream that
you are wrapping should be performing that check before doing something that
might crash (although I get that right now it might not be)?
--
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]
