emilk commented on code in PR #10730:
URL: https://github.com/apache/arrow-rs/pull/10730#discussion_r3806774580
##########
arrow-schema/src/ffi.rs:
##########
@@ -133,16 +133,21 @@ unsafe extern "C" fn release_schema(schema: *mut
FFI_ArrowSchema) {
impl FFI_ArrowSchema {
/// create a new [`FFI_ArrowSchema`]. This fails if the fields'
- /// [`DataType`] is not supported.
- ///
- /// # Panics
- ///
- /// Panics if `format` contains an interior nul byte
Review Comment:
That is what this PR does: `try_new` no longer has a `# Panics` section, and
the interior nul byte now returns `ArrowError::CDataInterface`, matching what
`with_name` already did. The conversion also moved above the `Box::into_raw`
calls so the error path does not leak the children.
-- Claude
##########
parquet-variant-compute/src/variant_array.rs:
##########
@@ -409,28 +409,28 @@ impl VariantArray {
/// Use `try_value` if you need to handle conversion errors gracefully.
///
/// # Panics
- /// * if the index is out of bounds
- /// * if the array value is null
- /// * if `try_value` returns an error.
+ /// Panics if [`VariantArray::try_value`] returns an error, e.g. if the
index is out of bounds.
Review Comment:
Spelled the reasons out in 7215c980a. I kept all three rather than just the
index, because `value()` also panics on invalid `metadata`/`value` bytes (which
includes reading a null row) and on a row with conflicting `value` and
`typed_value`. Listing only the index would hide the other two.
-- Claude
##########
arrow-data/src/transform/mod.rs:
##########
@@ -801,15 +817,13 @@ impl<'a> MutableArrayData<'a> {
///
/// # Panics
///
- /// Panics if [`MutableArrayData`] not created with `use_nulls` or
nullable source arrays,
- /// or if the run-end counter overflows for `RunEndEncoded` arrays.
+ /// Panics for the same reasons [`Self::try_extend_nulls`] returns an
error.
Review Comment:
Agreed, written out in 7215c980a.
-- Claude
##########
arrow-data/src/transform/mod.rs:
##########
@@ -763,17 +772,15 @@ impl<'a> MutableArrayData<'a> {
/// Extends the in progress array with a region of the input arrays.
///
/// # Panics
- /// This function panics if there is an invalid index,
- /// i.e. `index` >= the number of source arrays,
- /// `end` > the length of the `index`th array,
- /// or the offset type overflows (e.g. more than 2 GiB in a `StringArray`).
+ /// This function panics for the same reasons [`Self::try_extend`] returns
an error:
Review Comment:
Agreed, same in 7215c980a. It now lists the three cases inline: bad index,
bad range, and offset type overflow.
-- Claude
--
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]
