Fixed memory corruption in data frame queries (#489) and added support

for converting an OracleDataFrame object to a foreign data frame object
multiple times (#470).

Author: anthony-tuininga

doc/src/release_notes.rst

@@ -36,6 +36,11 @@ Common Changes
     (`issue 493 <https://github.com/oracle/python-oracledb/issues/493>`__).
 #)  Miscellaneous grammar and spelling fixes by John Bampton
     (`PR 479 <https://github.com/oracle/python-oracledb/pull/479>`__).
+#)  Fixed memory corruption in DataFrame queries
+    (`issue 489 <https://github.com/oracle/python-oracledb/issues/489>`__).
+#)  Added support for converting an OracleDataFrame object to a foreign data
+    frame object more than once
+    (`issue 470 <https://github.com/oracle/python-oracledb/issues/470>`__)
 
 
 oracledb 3.1.0 (April 2025)

src/oracledb/interchange/nanoarrow_bridge.pxd

@@ -41,6 +41,9 @@ cdef extern from "nanoarrow.h":
         int64_t null_count
         int64_t offset
         int64_t n_buffers
+        int64_t n_children
+        ArrowArray** children
+        const void** buffers
         void (*release)(ArrowArray*)
 
     cdef struct ArrowSchema:
@@ -57,6 +60,7 @@ cdef extern from "nanoarrow.h":
         NANOARROW_TYPE_LARGE_STRING
         NANOARROW_TYPE_STRING
         NANOARROW_TYPE_TIMESTAMP
+        NANOARROW_TYPE_UNINITIALIZED
 
     cpdef enum ArrowTimeUnit:
         NANOARROW_TIME_UNIT_SECOND
@@ -87,7 +91,6 @@ cdef class OracleArrowArray:
         double factor
         ArrowArray *arrow_array
         ArrowSchema *arrow_schema
-        void (*actual_array_release)(ArrowArray*) noexcept
 
     cdef str _schema_to_string(self)
     cdef int append_bytes(self, void* ptr, int64_t num_bytes) except -1

src/oracledb/interchange/nanoarrow_bridge.pyx

@@ -31,17 +31,22 @@ cimport cpython
 
 from libc.stdint cimport uintptr_t
 from libc.string cimport memcpy, strlen, strchr
-from cpython.pycapsule cimport PyCapsule_New
 
 from .. import errors
 
 cdef extern from "nanoarrow/nanoarrow.c":
 
     ctypedef int ArrowErrorCode
 
+    ctypedef void (*ArrowBufferDeallocatorCallback)
+
+    cdef struct ArrowBufferAllocator:
+        void *private_data
+
     cdef struct ArrowBuffer:
         uint8_t *data
         int64_t size_bytes
+        ArrowBufferAllocator allocator
 
     cdef union ArrowBufferViewData:
         const void* data
@@ -65,6 +70,8 @@ cdef extern from "nanoarrow/nanoarrow.c":
 
     cdef ArrowErrorCode NANOARROW_OK
 
+    ArrowErrorCode ArrowArrayAllocateChildren(ArrowArray *array,
+                                              int64_t n_children)
     ArrowErrorCode ArrowArrayAppendBytes(ArrowArray* array,
                                          ArrowBufferView value)
     ArrowErrorCode ArrowArrayAppendDecimal(ArrowArray* array,
@@ -88,11 +95,15 @@ cdef extern from "nanoarrow/nanoarrow.c":
                                           const ArrowArray* array,
                                           ArrowError* error)
     int8_t ArrowBitGet(const uint8_t* bits, int64_t i)
+    ArrowBufferAllocator ArrowBufferDeallocator(ArrowBufferDeallocatorCallback,
+                                                void *private_data)
     void ArrowDecimalInit(ArrowDecimal* decimal, int32_t bitwidth,
                           int32_t precision, int32_t scale)
     void ArrowDecimalSetBytes(ArrowDecimal *decimal, const uint8_t* value)
     ArrowErrorCode ArrowDecimalSetDigits(ArrowDecimal* decimal,
                                          ArrowStringView value)
+    ArrowErrorCode ArrowSchemaDeepCopy(const ArrowSchema *schema,
+                                       ArrowSchema *schema_out)
     void ArrowSchemaInit(ArrowSchema* schema)
     ArrowErrorCode ArrowSchemaInitFromType(ArrowSchema* schema, ArrowType type)
     void ArrowSchemaRelease(ArrowSchema *schema)
@@ -117,22 +128,13 @@ cdef int _check_nanoarrow(int code) except -1:
         errors._raise_err(errors.ERR_ARROW_C_API_ERROR, code=code)
 
 
-cdef void array_deleter(ArrowArray *array) noexcept:
-    """
-    Called when an external library calls the release for an Arrow array. This
-    method simply marks the release as completed but doesn't actually do it, so
-    that the handling of duplicate rows can still make use of the array, even
-    if the external library no longer requires it!
-    """
-    array.release = NULL
-
-
 cdef void pycapsule_array_deleter(object array_capsule) noexcept:
     cdef ArrowArray* array = <ArrowArray*> cpython.PyCapsule_GetPointer(
         array_capsule, "arrow_array"
     )
     if array.release != NULL:
         ArrowArrayRelease(array)
+    cpython.PyMem_Free(array)
 
 
 cdef void pycapsule_schema_deleter(object schema_capsule) noexcept:
@@ -141,6 +143,65 @@ cdef void pycapsule_schema_deleter(object schema_capsule) noexcept:
     )
     if schema.release != NULL:
         ArrowSchemaRelease(schema)
+    cpython.PyMem_Free(schema)
+
+
+cdef void arrow_buffer_dealloc_callback(ArrowBufferAllocator *allocator,
+                                        uint8_t *ptr, int64_t size):
+    """
+    ArrowBufferDeallocatorCallback for an ArrowBuffer borrowed from
+    OracleArrowArray
+    """
+    cpython.Py_DECREF(<OracleArrowArray> allocator.private_data)
+
+
+cdef int copy_arrow_array(OracleArrowArray oracle_arrow_array,
+                          ArrowArray *src, ArrowArray *dest) except -1:
+    """
+    Shallow copy source ArrowArray to destination ArrowArray. The source
+    ArrowArray belongs to the wrapper OracleArrowArray. The shallow copy idea
+    is borrowed from nanoarrow:
+    https://github.com/apache/arrow-nanoarrow/main/blob/python
+    """
+    cdef:
+        ArrowBuffer *dest_buffer
+        ssize_t i
+    _check_nanoarrow(
+        ArrowArrayInitFromType(
+            dest, NANOARROW_TYPE_UNINITIALIZED
+        )
+    )
+
+    # Copy metadata
+    dest.length = src.length
+    dest.offset = src.offset
+    dest.null_count = src.null_count
+
+    # Borrow an ArrowBuffer belonging to OracleArrowArray. The ArrowBuffer can
+    # belong to an immediate ArrowArray or a child (in case of nested types).
+    # Either way, we PY_INCREF(oracle_arrow_array), so that it is not
+    # prematurely garbage collected. The corresponding PY_DECREF happens in the
+    # ArrowBufferDeAllocator callback.
+    for i in range(src.n_buffers):
+        if src.buffers[i] != NULL:
+            dest_buffer = ArrowArrayBuffer(dest, i)
+            dest_buffer.data = <uint8_t *> src.buffers[i]
+            dest_buffer.size_bytes = 0
+            dest_buffer.allocator = ArrowBufferDeallocator(
+                <ArrowBufferDeallocatorCallback> arrow_buffer_dealloc_callback,
+                <void *> oracle_arrow_array
+            )
+            cpython.Py_INCREF(oracle_arrow_array)
+        dest.buffers[i] = src.buffers[i]
+    dest.n_buffers = src.n_buffers
+
+    # shallow copy of children (recursive call)
+    if src.n_children > 0:
+        _check_nanoarrow(ArrowArrayAllocateChildren(dest, src.n_children))
+        for i in range(src.n_children):
+            copy_arrow_array(
+                oracle_arrow_array, src.children[i], dest.children[i]
+            )
 
 
 cdef class OracleArrowArray:
@@ -187,8 +248,6 @@ cdef class OracleArrowArray:
 
     def __dealloc__(self):
         if self.arrow_array != NULL:
-            if self.arrow_array.release == NULL:
-                self.arrow_array.release = self.actual_array_release
             if self.arrow_array.release != NULL:
                 ArrowArrayRelease(self.arrow_array)
             cpython.PyMem_Free(self.arrow_array)
@@ -409,6 +468,26 @@ cdef class OracleArrowArray:
     def offset(self) -> int:
         return self.arrow_array.offset
 
+    def __arrow_c_schema__(self):
+        """
+        Export an ArrowSchema PyCapsule
+        """
+        cdef ArrowSchema *exported_schema = \
+            <ArrowSchema*> cpython.PyMem_Malloc(sizeof(ArrowSchema))
+        try:
+            _check_nanoarrow(
+                ArrowSchemaDeepCopy(
+                    self.arrow_schema,
+                    exported_schema
+                )
+            )
+        except:
+            cpython.PyMem_Free(exported_schema)
+            raise
+        return cpython.PyCapsule_New(
+            exported_schema, 'arrow_schema', &pycapsule_schema_deleter
+        )
+
     def __arrow_c_array__(self, requested_schema=None):
         """
         Returns
@@ -419,13 +498,14 @@ cdef class OracleArrowArray:
         """
         if requested_schema is not None:
             raise NotImplementedError("requested_schema")
-
-        array_capsule = PyCapsule_New(
-            self.arrow_array, 'arrow_array', &pycapsule_array_deleter
-        )
-        self.actual_array_release = self.arrow_array.release
-        self.arrow_array.release = array_deleter
-        schema_capsule = PyCapsule_New(
-            self.arrow_schema, "arrow_schema", &pycapsule_schema_deleter
-        )
-        return schema_capsule, array_capsule
+        cdef ArrowArray *exported_array = \
+            <ArrowArray *> cpython.PyMem_Malloc(sizeof(ArrowArray))
+        try:
+            copy_arrow_array(self, self.arrow_array, exported_array)
+            array_capsule = cpython.PyCapsule_New(
+                exported_array, 'arrow_array', &pycapsule_array_deleter
+            )
+        except:
+            cpython.PyMem_Free(exported_array)
+            raise
+        return self.__arrow_c_schema__(), array_capsule

tests/test_8000_dataframe.py

@@ -409,18 +409,13 @@ def test_8009(self):
         self.__test_df_batches_interop(DATASET_4, batch_size=5, num_batches=2)
 
     def test_8010(self):
-        "8010 - verify passing Arrow arrays twice fails"
+        "8010 - verify passing Arrow arrays twice works"
         self.__check_interop()
         self.__populate_table(DATASET_1)
         statement = "select * from TestDataFrame order by Id"
         ora_df = self.conn.fetch_df_all(statement)
-        pyarrow.Table.from_arrays(
-            ora_df.column_arrays(), names=ora_df.column_names()
-        )
-        with self.assertRaises(pyarrow.lib.ArrowInvalid):
-            pyarrow.Table.from_arrays(
-                ora_df.column_arrays(), names=ora_df.column_names()
-            )
+        self.__validate_df(ora_df, DATASET_1)
+        self.__validate_df(ora_df, DATASET_1)
 
     def test_8011(self):
         "8011 - verify empty data set"

tests/test_8100_dataframe_async.py

@@ -420,18 +420,13 @@ async def test_8109(self):
         )
 
     async def test_8110(self):
-        "8110 - verify passing Arrow arrays twice fails"
+        "8110 - verify passing Arrow arrays twice works"
         self.__check_interop()
         await self.__populate_table(DATASET_1)
         statement = "select * from TestDataFrame order by Id"
         ora_df = await self.conn.fetch_df_all(statement)
-        pyarrow.Table.from_arrays(
-            ora_df.column_arrays(), names=ora_df.column_names()
-        )
-        with self.assertRaises(pyarrow.lib.ArrowInvalid):
-            pyarrow.Table.from_arrays(
-                ora_df.column_arrays(), names=ora_df.column_names()
-            )
+        self.__validate_df(ora_df, DATASET_1)
+        self.__validate_df(ora_df, DATASET_1)
 
     async def test_8111(self):
         "8111 - verify empty data set"