Jefffrey commented on code in PR #25700:
URL: https://github.com/apache/datafusion/pull/25700#discussion_r4092107774
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -286,12 +286,15 @@ fn to_char_array(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
}
let result = builder.finish();
+ // Only collapse to a scalar when the whole output is a single row. When
the
+ // format column has more than one row, a scalar `args[0]` was expanded to
a
+ // full array, so returning row 0 would repeat it for every row.
match args[0] {
Review Comment:
```rust
if result.len() == 1 {
// return as scalar
} else {
// return as array
}
```
we dont need to check the first argument, we can just go off the result
length alone
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -286,12 +286,15 @@ fn to_char_array(args: &[ColumnarValue]) ->
Result<ColumnarValue> {
}
let result = builder.finish();
+ // Only collapse to a scalar when the whole output is a single row. When
the
+ // format column has more than one row, a scalar `args[0]` was expanded to
a
+ // full array, so returning row 0 would repeat it for every row.
Review Comment:
```suggestion
```
not necessary to justify, the previous behaviour was an obvious bug
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -360,6 +363,38 @@ mod tests {
}
}
+ #[test]
+ fn test_scalar_datetime_with_format_column_returns_array() {
+ // A scalar datetime with a multi-row format column must format each
+ // row; previously it collapsed to row 0 and repeated one value.
+ let value = ScalarValue::Date32(Some(18506)); // 2020-09-01
+ let format = StringArray::from(vec!["%Y-%m-%d", "%d/%m/%Y"]);
+ let arg_fields = vec![
+ Field::new("a", value.data_type(), false).into(),
+ Field::new("a", format.data_type().to_owned(), false).into(),
+ ];
+ let args = ScalarFunctionArgs {
+ args: vec![
+ ColumnarValue::Scalar(value),
Review Comment:
we dont need this unit test if its covered by SLTs
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
