You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "kosiew (via GitHub)" <gi...@apache.org> on 2025/03/03 05:29:30 UTC
[PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
kosiew opened a new pull request, #14970:
URL: https://github.com/apache/datafusion/pull/14970
## Which issue does this PR close?
Closes #14969.
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
## Rationale for this change
The `to_char` function was incorrectly returning `NULL` for valid `DATE` values due to improper handling of the `DATE` type. This fix ensures that `to_char` correctly formats non-null `DATE` values while preserving `NULL` values as expected.
## What changes are included in this PR?
- Fixed the `to_char` function to properly process `DATE` values.
- Added slt test cases to verify correct behavior when formatting `DATE` values.
## Are these changes tested?
Yes, new test cases have been added to confirm that:
1. `to_char` correctly formats valid `DATE` values.
2. `NULL` values remain unaffected.
## Are there any user-facing changes?
No breaking changes. Users will now see correctly formatted `DATE` values when using `to_char`, instead of unexpected `NULL` results.
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#issuecomment-2702350112
Thank you @alamb for the review.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#issuecomment-2702149965
Thanks again @kosiew
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#discussion_r1976899548
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -678,31 +669,4 @@ mod tests {
"Execution error: Format for `to_char` must be non-null Utf8, received Timestamp(Nanosecond, None)"
);
}
-
- #[test]
- fn test_to_char_input_none_array() {
- let date_array = Arc::new(Date32Array::from(vec![Some(18506), None])) as ArrayRef;
- let format_array =
- StringArray::from(vec!["%Y-%m-%d".to_string(), "%Y-%m-%d".to_string()]);
- let args = datafusion_expr::ScalarFunctionArgs {
- args: vec![
- ColumnarValue::Array(date_array),
- ColumnarValue::Array(Arc::new(format_array) as ArrayRef),
- ],
- number_rows: 2,
- return_type: &DataType::Utf8,
- };
- let result = ToCharFunc::new()
- .invoke_with_args(args)
- .expect("Expected no error");
- if let ColumnarValue::Array(result) = result {
- let result = result.as_any().downcast_ref::<StringArray>().unwrap();
- assert_eq!(result.len(), 2);
- // The first element is valid, second is null.
- assert!(!result.is_null(0));
- assert!(result.is_null(1));
- } else {
- panic!("Expected an array value");
- }
- }
Review Comment:
Remove test which was added in #14908
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#discussion_r1976901484
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -260,13 +258,6 @@ fn _to_char_array(args: &[ColumnarValue]) -> Result<ColumnarValue> {
let data_type = arrays[0].data_type();
for idx in 0..arrays[0].len() {
- // fix https://github.com/apache/datafusion/issues/14884
- // If the date/time value is null, push None.
- if arrays[0].is_null(idx) {
- results.push(None);
- continue;
- }
-
Review Comment:
undo #14908
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#discussion_r1979911036
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -234,15 +226,21 @@ fn _to_char_scalar(
};
let formatter = ArrayFormatter::try_new(array.as_ref(), &format_options)?;
- let formatted: Result<Vec<_>, ArrowError> = (0..array.len())
- .map(|i| formatter.value(i).try_to_string())
+ let formatted: Result<Vec<Option<String>>, ArrowError> = (0..array.len())
Review Comment:
👍
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #14970:
URL: https://github.com/apache/datafusion/pull/14970
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#discussion_r1976899548
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -678,31 +669,4 @@ mod tests {
"Execution error: Format for `to_char` must be non-null Utf8, received Timestamp(Nanosecond, None)"
);
}
-
- #[test]
- fn test_to_char_input_none_array() {
- let date_array = Arc::new(Date32Array::from(vec![Some(18506), None])) as ArrayRef;
- let format_array =
- StringArray::from(vec!["%Y-%m-%d".to_string(), "%Y-%m-%d".to_string()]);
- let args = datafusion_expr::ScalarFunctionArgs {
- args: vec![
- ColumnarValue::Array(date_array),
- ColumnarValue::Array(Arc::new(format_array) as ArrayRef),
- ],
- number_rows: 2,
- return_type: &DataType::Utf8,
- };
- let result = ToCharFunc::new()
- .invoke_with_args(args)
- .expect("Expected no error");
- if let ColumnarValue::Array(result) = result {
- let result = result.as_any().downcast_ref::<StringArray>().unwrap();
- assert_eq!(result.len(), 2);
- // The first element is valid, second is null.
- assert!(!result.is_null(0));
- assert!(result.is_null(1));
- } else {
- panic!("Expected an array value");
- }
- }
Review Comment:
Removing this test which was added in #14908
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Fix: to_char Function Now Correctly Handles DATE Values in DataFusion [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #14970:
URL: https://github.com/apache/datafusion/pull/14970#discussion_r1976901204
##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -212,14 +212,6 @@ fn _to_char_scalar(
let is_scalar_expression = matches!(&expression, ColumnarValue::Scalar(_));
let array = expression.into_array(1)?;
- // fix https://github.com/apache/datafusion/issues/14884
- // If the input date/time is null, return a null Utf8 result.
- if array.is_null(0) {
- return Ok(match is_scalar_expression {
- true => ColumnarValue::Scalar(ScalarValue::Utf8(None)),
- false => ColumnarValue::Array(new_null_array(&Utf8, array.len())),
- });
- }
Review Comment:
Undo #14908
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org