Skip to content

Commit 451c79f

Browse files
authored
fix: Fix array_to_string with columnar third arg (#20536)
## Which issue does this PR close? - Closes #20535 ## Rationale for this change The previous coding used the `null_string` value for the first row as the value for the remainder of the rows. This is wrong if `null_string` is columnar. ## What changes are included in this PR? ## Are these changes tested? Yes; added new SLT test. ## Are there any user-facing changes? No, other than fixing the behavior of `array_to_string` in this scenario.
1 parent 7ef62b9 commit 451c79f

File tree

2 files changed

+41
-27
lines changed

2 files changed

+41
-27
lines changed

datafusion/functions-nested/src/string.rs

Lines changed: 22 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -347,21 +347,20 @@ fn array_to_string_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
347347
}
348348
};
349349

350-
let mut null_string = String::from("");
351-
let mut with_null_string = false;
352-
if args.len() == 3 {
353-
null_string = match args[2].data_type() {
354-
Utf8 => args[2].as_string::<i32>().value(0).to_string(),
355-
Utf8View => args[2].as_string_view().value(0).to_string(),
356-
LargeUtf8 => args[2].as_string::<i64>().value(0).to_string(),
350+
let null_strings = if args.len() == 3 {
351+
Some(match args[2].data_type() {
352+
Utf8 => args[2].as_string::<i32>().iter().collect(),
353+
Utf8View => args[2].as_string_view().iter().collect(),
354+
LargeUtf8 => args[2].as_string::<i64>().iter().collect(),
357355
other => {
358356
return exec_err!(
359-
"unsupported type for second argument to array_to_string function as {other:?}"
357+
"unsupported type for third argument to array_to_string function as {other:?}"
360358
);
361359
}
362-
};
363-
with_null_string = true;
364-
}
360+
})
361+
} else {
362+
None
363+
};
365364

366365
/// Creates a single string from single element of a ListArray (which is
367366
/// itself another Array)
@@ -469,18 +468,24 @@ fn array_to_string_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
469468
fn generate_string_array<O: OffsetSizeTrait>(
470469
list_arr: &GenericListArray<O>,
471470
delimiters: &[Option<&str>],
472-
null_string: &str,
473-
with_null_string: bool,
471+
null_strings: &Option<Vec<Option<&str>>>,
474472
) -> Result<StringArray> {
475473
let mut res: Vec<Option<String>> = Vec::new();
476-
for (arr, &delimiter) in list_arr.iter().zip(delimiters.iter()) {
474+
for (i, (arr, &delimiter)) in list_arr.iter().zip(delimiters.iter()).enumerate() {
477475
if let (Some(arr), Some(delimiter)) = (arr, delimiter) {
476+
let (null_string, with_null_string) = match null_strings {
477+
Some(ns) => match ns[i] {
478+
Some(s) => (s.to_string(), true),
479+
None => (String::new(), false),
480+
},
481+
None => (String::new(), false),
482+
};
478483
let mut arg = String::from("");
479484
let s = compute_array_to_string(
480485
&mut arg,
481486
&arr,
482487
delimiter.to_string(),
483-
null_string.to_string(),
488+
null_string,
484489
with_null_string,
485490
)?
486491
.clone();
@@ -501,21 +506,11 @@ fn array_to_string_inner(args: &[ArrayRef]) -> Result<ArrayRef> {
501506
let string_arr = match arr.data_type() {
502507
List(_) => {
503508
let list_array = as_list_array(&arr)?;
504-
generate_string_array::<i32>(
505-
list_array,
506-
&delimiters,
507-
&null_string,
508-
with_null_string,
509-
)?
509+
generate_string_array::<i32>(list_array, &delimiters, &null_strings)?
510510
}
511511
LargeList(_) => {
512512
let list_array = as_large_list_array(&arr)?;
513-
generate_string_array::<i64>(
514-
list_array,
515-
&delimiters,
516-
&null_string,
517-
with_null_string,
518-
)?
513+
generate_string_array::<i64>(list_array, &delimiters, &null_strings)?
519514
}
520515
// Signature guards against this arm
521516
_ => return exec_err!("array_to_string expects list as first argument"),

datafusion/sqllogictest/test_files/array.slt

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5248,6 +5248,25 @@ NULL 1.2.3
52485248
51_52_*_54_55_56_57_58_59_60 1.2.3
52495249
61_62_63_64_65_66_67_68_69_70 1.2.3
52505250

5251+
# array_to_string with per-row null_string column
5252+
statement ok
5253+
CREATE TABLE test_null_str_col AS VALUES
5254+
(make_array(1, NULL, 3), ',', 'N/A'),
5255+
(make_array(NULL, 5, NULL), ',', 'MISSING'),
5256+
(make_array(10, NULL, 12), '-', 'X'),
5257+
(make_array(20, NULL, 21), '-', NULL);
5258+
5259+
query T
5260+
SELECT array_to_string(column1, column2, column3) FROM test_null_str_col;
5261+
----
5262+
1,N/A,3
5263+
MISSING,5,MISSING
5264+
10-X-12
5265+
20-21
5266+
5267+
statement ok
5268+
DROP TABLE test_null_str_col;
5269+
52515270
## cardinality
52525271

52535272
# cardinality scalar function

0 commit comments

Comments
 (0)