This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 67bc67ed9f feature: add safe version of `ArrayData::from_pyarrow`
(#10894)
67bc67ed9f is described below
commit 67bc67ed9f8733680fc5bcb4583415071b7ee6cf
Author: RIchard Baah <[email protected]>
AuthorDate: Mon Oct 5 20:10:50 2026 +0000
feature: add safe version of `ArrayData::from_pyarrow` (#10894)
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and
enhancements and this helps us generate change logs for our releases.
You can link an issue to this PR using the GitHub syntax.
-->
- Closes #3672.
# Rationale for this change
like #3672 mentions, `ArrayData::from_pyarrow` does not validate its
arguments and its filled with unsafe code. this can introduce invalidate
data into arrow-rs that is very hard to trace.
<!--
Why are you proposing this change? If this is already explained clearly
in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand
your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
split `ArrayData::from_pyarrow` into two separate functions. one safe +
one unsafe. the safe function validate that data produced from the
unsafe function and returns an error if invalid.
<!--
There is no need to duplicate the description in the issue here but it
is sometimes worth providing a summary of the individual changes in this
PR.
-->
# Are these changes tested?
I don't think this warrants a test.
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
If this PR claims a performance improvement, please include evidence
such as benchmark results.
-->
# Are there any user-facing changes?
yes, new unsafe api. the safe API now is truly safe.
<!--
If there are user-facing changes then we may require documentation to be
updated before approving the PR.
If there are any breaking changes to public APIs, please call them out.
-->
---
arrow-pyarrow/src/lib.rs | 150 +++++++++++++++++++++++++++++------------------
1 file changed, 94 insertions(+), 56 deletions(-)
diff --git a/arrow-pyarrow/src/lib.rs b/arrow-pyarrow/src/lib.rs
index 49ddd35c32..a159c9f1c4 100644
--- a/arrow-pyarrow/src/lib.rs
+++ b/arrow-pyarrow/src/lib.rs
@@ -132,6 +132,19 @@ const ARRAY_STREAM_INPUT_TYPE: PyStaticExpr =
type_hint_union!(
type_hint_identifier!("pyarrow", "Table")
);
+/// Trait for converting Python objects to arrow-rs types without validation.
+///
+/// Prefer [`FromPyArrow`] where possible, as it validates the result before
returning.
+pub trait FromPyArrowUnchecked: Sized {
+ /// Convert a Python object to an arrow-rs type without validating the
result.
+ ///
+ /// # Safety
+ ///
+ /// The data exported by the Python object must satisfy all Arrow spec
invariants
+ /// (valid offsets, buffer sizes, UTF-8, etc.) that
[`ArrayData::validate_full`] checks.
+ unsafe fn from_pyarrow_bound_unchecked(value: &Bound<PyAny>) ->
PyResult<Self>;
+}
+
/// Trait for converting Python objects to arrow-rs types.
pub trait FromPyArrow: Sized {
/// The Python type this conversion accepts, as a type hint.
@@ -321,10 +334,8 @@ impl ToPyArrow for Schema {
}
}
-impl FromPyArrow for ArrayData {
- type_hint!(INPUT_TYPE = type_hint_identifier!("pyarrow", "Array"));
-
- fn from_pyarrow_bound(value: &Bound<PyAny>) -> PyResult<Self> {
+impl FromPyArrowUnchecked for ArrayData {
+ unsafe fn from_pyarrow_bound_unchecked(value: &Bound<PyAny>) ->
PyResult<Self> {
// Newer versions of PyArrow as well as other libraries with Arrow
data implement this
// method, so prefer it over _export_to_c.
// See
https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html
@@ -357,6 +368,16 @@ impl FromPyArrow for ArrayData {
}
}
+impl FromPyArrow for ArrayData {
+ type_hint!(INPUT_TYPE = type_hint_identifier!("pyarrow", "Array"));
+
+ fn from_pyarrow_bound(value: &Bound<PyAny>) -> PyResult<Self> {
+ let data = unsafe { ArrayData::from_pyarrow_bound_unchecked(value)? };
+ data.validate_full().map_err(to_py_err)?;
+ Ok(data)
+ }
+}
+
impl ToPyArrow for ArrayData {
type_hint!(OUTPUT_TYPE = type_hint_identifier!("pyarrow", "Array"));
@@ -403,63 +424,80 @@ impl<T: ToPyArrow> ToPyArrow for Vec<T> {
}
}
+fn record_batch_from_pyarrow_bound_impl(
+ value: &Bound<PyAny>,
+ validate: bool,
+) -> PyResult<RecordBatch> {
+ // Newer versions of PyArrow as well as other libraries with Arrow data
implement this
+ // method, so prefer it over _export_to_c.
+ // See
https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html
+ if let Some((schema_capsule, array_capsule)) =
call_arrow_c_array_method_if_exists(value)? {
+ let schema_ptr = extract_capsule(&schema_capsule, c"arrow_schema",
"__arrow_c_array__")?;
+ let array_ptr = extract_capsule(&array_capsule, c"arrow_array",
"__arrow_c_array__")?;
+ let ffi_array = unsafe { FFI_ArrowArray::from_raw(array_ptr.as_ptr())
};
+ let array_data =
+ unsafe { ffi::from_ffi(ffi_array, schema_ptr.as_ref())
}.map_err(to_py_err)?;
+ if validate {
+ array_data.validate_full().map_err(to_py_err)?;
+ }
+ if !matches!(array_data.data_type(), DataType::Struct(_)) {
+ return Err(PyTypeError::new_err(format!(
+ "Expected Struct type from __arrow_c_array__, found {}.",
+ array_data.data_type()
+ )));
+ }
+ let options =
RecordBatchOptions::default().with_row_count(Some(array_data.len()));
+ let array = StructArray::from(array_data);
+ // StructArray does not embed metadata from schema. We need to override
+ // the output schema with the schema from the capsule.
+ let schema = unsafe {
Arc::new(Schema::try_from(schema_ptr.as_ref()).map_err(to_py_err)?) };
+ let (_fields, columns, nulls) = array.into_parts();
+ if nulls.map(|n| n.null_count()).unwrap_or_default() != 0 {
+ return Err(PyValueError::new_err(
+ "Cannot convert nullable StructArray to RecordBatch, see
StructArray documentation",
+ ));
+ }
+ return RecordBatch::try_new_with_options(schema, columns,
&options).map_err(to_py_err);
+ }
+
+ validate_class(record_batch_class(value.py())?, value)?;
+ // TODO(kszucs): implement the FFI conversions in arrow-rs for
RecordBatches
+ let schema = value.getattr("schema")?;
+ let schema = Arc::new(Schema::from_pyarrow_bound(&schema)?);
+
+ let arrays = value.getattr("columns")?;
+ let arrays = arrays
+ .cast::<PyList>()?
+ .iter()
+ .map(|a| {
+ let data = unsafe { ArrayData::from_pyarrow_bound_unchecked(&a)? };
+ if validate {
+ data.validate_full().map_err(to_py_err)?;
+ }
+ Ok(make_array(data))
+ })
+ .collect::<PyResult<_>>()?;
+
+ let row_count = value
+ .getattr("num_rows")
+ .ok()
+ .and_then(|x| x.extract().ok());
+ let options = RecordBatchOptions::default().with_row_count(row_count);
+
+ RecordBatch::try_new_with_options(schema, arrays,
&options).map_err(to_py_err)
+}
+
impl FromPyArrow for RecordBatch {
type_hint!(INPUT_TYPE = type_hint_identifier!("pyarrow", "RecordBatch"));
fn from_pyarrow_bound(value: &Bound<PyAny>) -> PyResult<Self> {
- // Newer versions of PyArrow as well as other libraries with Arrow
data implement this
- // method, so prefer it over _export_to_c.
- // See
https://arrow.apache.org/docs/format/CDataInterface/PyCapsuleInterface.html
-
- if let Some((schema_capsule, array_capsule)) =
call_arrow_c_array_method_if_exists(value)? {
- let schema_ptr =
- extract_capsule(&schema_capsule, c"arrow_schema",
"__arrow_c_array__")?;
- let array_ptr = extract_capsule(&array_capsule, c"arrow_array",
"__arrow_c_array__")?;
- let ffi_array = unsafe {
FFI_ArrowArray::from_raw(array_ptr.as_ptr()) };
- let array_data =
- unsafe { ffi::from_ffi(ffi_array, schema_ptr.as_ref())
}.map_err(to_py_err)?;
- if !matches!(array_data.data_type(), DataType::Struct(_)) {
- return Err(PyTypeError::new_err(format!(
- "Expected Struct type from __arrow_c_array__, found {}.",
- array_data.data_type()
- )));
- }
- let options =
RecordBatchOptions::default().with_row_count(Some(array_data.len()));
- let array = StructArray::from(array_data);
- // StructArray does not embed metadata from schema. We need to
override
- // the output schema with the schema from the capsule.
- let schema =
- unsafe {
Arc::new(Schema::try_from(schema_ptr.as_ref()).map_err(to_py_err)?) };
- let (_fields, columns, nulls) = array.into_parts();
- if nulls.map(|n| n.null_count()).unwrap_or_default() != 0 {
- return Err(PyValueError::new_err(
- "Cannot convert nullable StructArray to RecordBatch, see
StructArray documentation",
- ));
- }
- return RecordBatch::try_new_with_options(schema, columns,
&options).map_err(to_py_err);
- }
+ record_batch_from_pyarrow_bound_impl(value, true)
+ }
+}
- validate_class(record_batch_class(value.py())?, value)?;
- // TODO(kszucs): implement the FFI conversions in arrow-rs for
RecordBatches
- let schema = value.getattr("schema")?;
- let schema = Arc::new(Schema::from_pyarrow_bound(&schema)?);
-
- let arrays = value.getattr("columns")?;
- let arrays = arrays
- .cast::<PyList>()?
- .iter()
- .map(|a| Ok(make_array(ArrayData::from_pyarrow_bound(&a)?)))
- .collect::<PyResult<_>>()?;
-
- let row_count = value
- .getattr("num_rows")
- .ok()
- .and_then(|x| x.extract().ok());
- let options = RecordBatchOptions::default().with_row_count(row_count);
-
- let batch =
- RecordBatch::try_new_with_options(schema, arrays,
&options).map_err(to_py_err)?;
- Ok(batch)
+impl FromPyArrowUnchecked for RecordBatch {
+ unsafe fn from_pyarrow_bound_unchecked(value: &Bound<PyAny>) ->
PyResult<Self> {
+ record_batch_from_pyarrow_bound_impl(value, false)
}
}
