tfeda commented on code in PR #1720:
URL: https://github.com/apache/arrow-rs/pull/1720#discussion_r878891034
##########
arrow/src/compute/kernels/concat.rs:
##########
@@ -102,6 +102,25 @@ pub fn concat(arrays: &[&dyn Array]) -> Result<ArrayRef> {
Ok(make_array(mutable.freeze()))
}
+// Elementwise concatenation of StringArrays
+pub fn string_concat<Offset: OffsetSizeTrait>(
+ left: &GenericStringArray<Offset>,
+ right: &GenericStringArray<Offset>,
+) -> Result<GenericStringArray<Offset>> {
+ let left_bitmap = left.data().null_bitmap().unwrap();
+ let right_bitmap = right.data().null_bitmap().unwrap();
+ let concat_bitmap = (left_bitmap & right_bitmap).unwrap();
+ Ok((0..left.len().max(right.len()))
+ .map(|i| {
+ if concat_bitmap.is_set(i) {
+ Some(left.value(i).to_owned() + right.value(i))
+ } else {
+ None
+ }
+ })
+ .collect::<GenericStringArray<Offset>>())
Review Comment:
A couple thoughts, because I spent some time digging into this:
1. @alamb's example in #1720 suggests that "valid_str" + `None` => `None`,
where both implementations in this PR go for "valid_str" + `None` =>
"valid_str". As a user, I'd prefer the later, but I thought I'd make a note of
it. If the former is chosen, then `compute::util` has
[`combine_option_bitmap()`](https://github.com/apache/arrow-rs/blob/4de689598df6ea284452e687d69c7654b5a71762/arrow/src/compute/util.rs#L31)
which works nicely for the null handling
3. How do we handle cases when concatenating two arrays results in the
offsets overflowing? There should probably be a test case for that.
--
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]
