alamb commented on code in PR #7405:
URL: https://github.com/apache/arrow-rs/pull/7405#discussion_r2039312086
##########
arrow-array/src/record_batch.rs:
##########
@@ -359,11 +359,12 @@ impl RecordBatch {
})
}
- /// Overrides the schema of this [`RecordBatch`]
- /// without additional schema checks. Note, however, that this pushes all
the schema compatibility responsibilities
- /// to the caller site. In particular, the caller guarantees that `schema`
is a superset
- /// of the current schema as determined by [`Schema::contains`].
- pub fn with_schema_unchecked(self, schema: SchemaRef) -> Result<Self,
ArrowError> {
+ /// Overrides the schema of this [`RecordBatch`] without additional schema
checks.
+ ///
+ /// # Safety
+ ///
+ /// `schema` must be a superset of the current schema as determined by
[`Schema::contains`]
+ pub unsafe fn with_schema_unchecked(self, schema: SchemaRef) ->
Result<Self, ArrowError> {
Review Comment:
I agree this API seems awkward and not consistent with the other APIs in
this crate
What I suggest is:
1. Remove this API
2. Add APIs for destructing a RecordBatch into its constituent parts:
3. Add an `unsafe` construction API (inline with the other parts of arrow)
So something like
```rust
impl RecordBatch {
/// Creates a RecordBatch without any consistency checks
pub unsafe fn new_unchecked(schema: Schema, arrays: vec<ArrayRef>,
row_count) -> Self {
...
}
/// Return the inner fields of this RecordBatch, consuming sekf
pub fn into_parts(self) -> (Schema, Vec<ArrayRef>, usize) {
...
}
}
```
--
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]
