alamb commented on code in PR #7664:
URL: https://github.com/apache/arrow-rs/pull/7664#discussion_r2150864787
##########
arrow-array/src/record_batch.rs:
##########
@@ -402,6 +404,14 @@ impl RecordBatch {
&self.schema
}
+ /// Mutable access to the metadata of the schema.
+ ///
+ /// This allows you to modify [`Schema::metadata`] of [`Self::schema`] in
a convenient and fast way.
Review Comment:
I think we should add some tests here -- perhaps as a doc comment
```suggestion
/// This allows you to modify [`Schema::metadata`] of [`Self::schema`]
in a convenient and fast way.
///
/// Note this will clone the entire underlying `Schema` object if it is
currently shared
///
/// # Example
/// ```
/// # use std::sync::Arc;
/// # use arrow_array::{record_batch, RecordBatch};
/// let mut batch = record_batch!(("a", Int32, [1, 2, 3])).unwrap();
/// // Initially, the metadata is empty
/// assert!(batch.schema().metadata().get("key").is_none());
/// // Insert a key-value pair into the metadata
/// batch.schema_metadata_mut().insert("key".into(), "value".into());
/// assert_eq!(batch.schema().metadata().get("key"),
Some(&String::from("value")));
/// ```
```
##########
arrow-schema/src/field.rs:
##########
@@ -328,6 +328,12 @@ impl Field {
&self.metadata
}
+ /// Returns a mutable reference to the `Field`'s optional custom metadata.
Review Comment:
Maybe we can also add an example here (though I think it is less necessary
for just this accessor)
##########
arrow-array/src/record_batch.rs:
##########
@@ -402,6 +404,14 @@ impl RecordBatch {
&self.schema
}
+ /// Mutable access to the metadata of the schema.
+ ///
+ /// This allows you to modify [`Schema::metadata`] of [`Self::schema`] in
a convenient and fast way.
+ pub fn schema_metadata_mut(&mut self) -> &mut
std::collections::HashMap<String, String> {
+ let schema = Arc::make_mut(&mut self.schema);
+ &mut schema.metadata
Review Comment:
What do you think about also adding `Schema::metadata_mut` for symmetry and
using it here?
Something like
```rust
pub fn schema_metadata_mut(&mut self) -> &mut
std::collections::HashMap<String, String> {
Arc::make_mut(&mut self.schema).schema_mut()
}
```
--
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]
