Skip to content

Commit

Permalink
Improve implementation and do the right thing for null
Browse files Browse the repository at this point in the history
  • Loading branch information
alamb committed Aug 21, 2024
1 parent cc32d5e commit 90d2122
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 12 deletions.
20 changes: 15 additions & 5 deletions datafusion/physical-expr/src/expressions/binary/kernels.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,22 @@ pub fn concat_elements_utf8view(
.map(|(b1, b2)| b1.len() + b2.len())
.sum();
let mut result = StringViewBuilder::with_capacity(capacity);

// Avoid reallocations by writing to a reused buffer (note we
// could be even more efficient r by creating the view directly
// here and avoid the buffer but that would be more complex)
let mut buffer = String::new();

for (left, right) in left.iter().zip(right.iter()) {
match (left, right) {
(None, None) => result.append_null(),
_ => {
result.append_value(left.unwrap_or("").to_string() + right.unwrap_or(""))
}
if let (Some(left), Some(right)) = (left, right) {
use std::fmt::Write;
buffer.clear();
write!(&mut buffer, "{left}{right}")
.expect("writing into string buffer failed");
result.append_value(&buffer);
} else {
// at least one of the values is null, so the output is also null
result.append_null()
}
}
Ok(result.finish())
Expand Down
12 changes: 5 additions & 7 deletions datafusion/sqllogictest/test_files/string_view.slt
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,7 @@ FROM test;
NULL

# || mixed types
# expect all results to be the same for each row as they all have the same values
query TTTTTTTT
SELECT
column1_utf8view || column2_utf8view,
Expand All @@ -1162,9 +1163,10 @@ FROM test;
AndrewX AndrewX AndrewX AndrewX XAndrew XAndrew XAndrew XAndrew
XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng
RaphaelR RaphaelR RaphaelR RaphaelR RRaphael RRaphael RRaphael RRaphael
R R R R R R R R
NULL NULL NULL NULL NULL NULL NULL NULL

# || constants
# expect all results to be the same for each row as they all have the same values
query TTTTTTTT
SELECT
column1_utf8view || 'foo',
Expand All @@ -1181,12 +1183,10 @@ FROM test;
Andrewfoo Andrewfoo Andrewfoo Andrewfoo fooAndrew fooAndrew fooAndrew fooAndrew
Xiangpengfoo Xiangpengfoo Xiangpengfoo Xiangpengfoo fooXiangpeng fooXiangpeng fooXiangpeng fooXiangpeng
Raphaelfoo Raphaelfoo Raphaelfoo Raphaelfoo fooRaphael fooRaphael fooRaphael fooRaphael
foo NULL NULL NULL foo NULL NULL NULL



NULL NULL NULL NULL NULL NULL NULL NULL

# || same type (column1 has null, so also tests NULL || NULL)
# expect all results to be the same for each row as they all have the same values
query TTT
SELECT
column1_utf8view || column1_utf8view,
Expand All @@ -1201,8 +1201,6 @@ XiangpengXiangpeng XiangpengXiangpeng XiangpengXiangpeng
RaphaelRaphael RaphaelRaphael RaphaelRaphael
NULL NULL NULL



statement ok
drop table test;

Expand Down

0 comments on commit 90d2122

Please sign in to comment.