JasonLi-cn commented on code in PR #9732:
URL: https://github.com/apache/arrow-datafusion/pull/9732#discussion_r1545191216
##########
datafusion/physical-expr/src/string_expressions.rs:
##########
@@ -227,6 +229,132 @@ pub fn concat(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
}
}
+enum ColumnarValueRef<'a> {
+ Scalar(&'a [u8]),
+ Array(&'a StringArray),
+}
+
+impl<'a> ColumnarValueRef<'a> {
+ #[inline]
+ fn is_valid(&self, i: usize) -> bool {
+ match &self {
+ Self::Scalar(_) => true,
+ Self::Array(array) => array.is_valid(i),
+ }
+ }
+
+ #[inline]
+ fn nulls(&self) -> Option<NullBuffer> {
+ match &self {
+ Self::Scalar(_) => None,
+ Self::Array(array) => array.nulls().map(|b| b.clone()),
+ }
+ }
+}
+
+struct StringArrayBuilder {
+ offsets_buffer: MutableBuffer,
+ value_buffer: MutableBuffer,
+}
+
+impl StringArrayBuilder {
+ fn with_capacity(item_capacity: usize, data_capacity: usize) -> Self {
+ let mut offsets_buffer = MutableBuffer::with_capacity(
+ (item_capacity + 1) * std::mem::size_of::<i32>(),
+ );
+ unsafe { offsets_buffer.push_unchecked(0_i32) };
+ Self {
+ offsets_buffer,
+ value_buffer: MutableBuffer::with_capacity(data_capacity),
+ }
+ }
+
+ fn write<const CHECK_VALID: bool>(&mut self, column: &ColumnarValueRef, i:
usize) {
+ match column {
+ ColumnarValueRef::Scalar(s) => {
+ self.value_buffer.extend_from_slice(s);
Review Comment:
1. My initial intention for optimizing this function was to avoid creating a
new String and then using `push_str` each time in `concat` function, like:
```rust
let mut owned_string: String = "".to_owned();
for arg in args {
match arg {
ColumnarValue::Scalar(ScalarValue::Utf8(maybe_value)) => {
if let Some(value) = maybe_value {
owned_string.push_str(value);
}
}
ColumnarValue::Array(v) => {
if v.is_valid(index) {
let v = as_string_array(v).unwrap();
owned_string.push_str(v.value(index));
}
}
_ => unreachable!(),
}
}
Some(owned_string)
```
3. Additionally, by precalculating the expected length of the result, I
avoided the need to reallocate memory.
4. I used the `extend_from_slice` function because I referred to the
`append_slice` function of `BufferBuilder`.
```rust
#[inline]
pub fn append_slice(&mut self, slice: &[T]) {
self.buffer.extend_from_slice(slice);
self.len += slice.len();
}
```
--
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]
