Implement concat_elements_dyn kernel #3763
Conversation
| right.data_type() | ||
| ))); | ||
| } | ||
| match (left.data_type(), right.data_type()) { |
There was a problem hiding this comment.
Perhaps we could also handle when the arrays are both LargeUtf8? Otherwise this is not all that different from the normal kernel?
There was a problem hiding this comment.
Certainly, you are correct. I plan to address LargeUtf8 at a later time.
There was a problem hiding this comment.
Do I need to write a separate concat function for LargeUtf8 or is it unnecessary since concat_elements_utf8 can already handle LargeUtf8?
| let left = left.as_any().downcast_ref::<StringArray>().unwrap(); | ||
| let right = right.as_any().downcast_ref::<StringArray>().unwrap(); | ||
| Ok(Arc::new(concat_elements_utf8(left, right).unwrap())) | ||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| (DataType::LargeUtf8, DataType::LargeUtf8) => { | |
| let left = left.as_any().downcast_ref::<LargeStringArray>().unwrap(); | |
| let right = right.as_any().downcast_ref::<LargeStringArray>().unwrap(); | |
| Ok(Arc::new(concat_elements_utf8(left, right).unwrap())) | |
| } |
Weijun-H
force-pushed
the
concat_elements_dyn
branch
from
19f8fb9 to
f371239
Compare
Weijun-H
force-pushed
the
concat_elements_dyn
branch
from
f371239 to
bc6a5ec
Compare
| left: &GenericBinaryArray<Offset>, | ||
| right: &GenericBinaryArray<Offset>, | ||
| ) -> Result<GenericBinaryArray<Offset>, ArrowError> { | ||
| if left.len() != right.len() { |
There was a problem hiding this comment.
I think this logic can be combined into a concat_element_bytes I'll see what I can come up with in a follow on PR
|
Benchmark runs are scheduled for baseline = 661bbad and contender = f8abb04. f8abb04 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #1755
Rationale for this change
Explained in #1755
What changes are included in this PR?
Are there any user-facing changes?