Omega359 commented on code in PR #6777:
URL: https://github.com/apache/arrow-rs/pull/6777#discussion_r1855228248
##########
arrow-array/src/builder/generic_bytes_view_builder.rs:
##########
@@ -281,7 +283,28 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
/// - String length exceeds `u32::MAX`
#[inline]
pub fn append_value(&mut self, value: impl AsRef<T::Native>) {
- let v: &[u8] = value.as_ref().as_ref();
+ // SAFETY: the value is a valid native type (e.g. `&str` or `&[u8]`)
+ unsafe { self.append_bytes(value.as_ref().as_ref()) }
+ }
+
+ /// Appends a &str into the builder
+ ///
+ /// # Panics
+ ///
+ /// Panics if
+ /// - String buffer count exceeds `u32::MAX`
+ /// - String length exceeds `u32::MAX`
+ #[inline]
+ pub fn append_str(&mut self, value: &str) {
+ // SAFETY: UTF8 bytes are valid for both string and binaary
Review Comment:
```suggestion
// SAFETY: UTF8 bytes are valid for both string and binary
```
##########
arrow-array/src/builder/generic_bytes_view_builder.rs:
##########
@@ -553,6 +639,56 @@ pub fn make_view(data: &[u8], block_id: u32, offset: u32)
-> u128 {
}
}
+/// See [GenericByteViewBuilder::formatter] for more details
+#[derive(Debug)]
+pub struct ByteViewFormatter<'a, T>
+where
+ T: ByteViewType + ?Sized,
+{
+ /// reference to the builder
+ inner: &'a mut GenericByteViewBuilder<T>,
+ /// buffer where the in progress string is written
+ buffer: String,
+}
+
+impl<'a, T> ByteViewFormatter<'a, T>
+where
+ T: ByteViewType + ?Sized,
+{
+ fn new(inner: &'a mut GenericByteViewBuilder<T>) -> Self {
+ let buffer = match std::mem::take(&mut inner.temp_buffer) {
+ Some(mut buffer) => {
+ buffer.clear();
+ buffer
+ }
+ None => String::new(),
+ };
+
+ Self { inner, buffer }
+ }
+}
+
+impl<'a, T> std::fmt::Write for ByteViewFormatter<'a, T>
+where
+ T: ByteViewType + ?Sized,
+{
+ fn write_str(&mut self, s: &str) -> std::fmt::Result {
+ self.buffer.write_str(s)
+ }
+}
+
+/// When a StringViewWriter is dropped, it writes the the value to the
StringViewBuilder
Review Comment:
```suggestion
/// When a StringViewWriter is dropped, it writes the value to the
StringViewBuilder
```
##########
arrow-array/src/builder/generic_bytes_view_builder.rs:
##########
@@ -406,6 +429,12 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
};
buffer_size + in_progress + tracker + views + null
}
+
+ /// Return a structure that implements `std::fmt::Write` to write stirngs
Review Comment:
```suggestion
/// Return a structure that implements `std::fmt::Write` to write strings
```
##########
arrow-array/src/builder/generic_bytes_view_builder.rs:
##########
@@ -281,7 +283,28 @@ impl<T: ByteViewType + ?Sized> GenericByteViewBuilder<T> {
/// - String length exceeds `u32::MAX`
#[inline]
pub fn append_value(&mut self, value: impl AsRef<T::Native>) {
- let v: &[u8] = value.as_ref().as_ref();
+ // SAFETY: the value is a valid native type (e.g. `&str` or `&[u8]`)
+ unsafe { self.append_bytes(value.as_ref().as_ref()) }
+ }
+
+ /// Appends a &str into the builder
+ ///
+ /// # Panics
+ ///
+ /// Panics if
+ /// - String buffer count exceeds `u32::MAX`
+ /// - String length exceeds `u32::MAX`
Review Comment:
You would likely run out of stack space first :)
--
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]
