friendlymatthew commented on code in PR #15361:
URL: https://github.com/apache/datafusion/pull/15361#discussion_r2009208756


##########
datafusion/functions/src/datetime/to_char.rs:
##########
@@ -277,7 +282,25 @@ fn _to_char_array(args: &[ColumnarValue]) -> 
Result<ColumnarValue> {
         let result = formatter.value(idx).try_to_string();
         match result {
             Ok(value) => results.push(Some(value)),
-            Err(e) => return exec_err!("{}", e),
+            Err(e) => {
+                if data_type == &Date32 {

Review Comment:
   > I don't understand why this is handling errors -- shouldn't we just 
dispach on type when building the format options?
   
   Sorry. 
   
   I assume you mean detect whether the format options contain time specifiers 
ahead of time and if so cast the `Date32` into a `Date64`. 
   
   Or are you saying avoid the cast altogether? A 
`FormatOption.with_datetime_format()` won't work with `Date32`, as Arrow 
doesn't consider the `datetime_format` field when formatting a `Date32`. 
   
   ```rs
   #[test]
   fn unaffected_format() {
       let date_format = "%Y-%m %H";
       
       let cast_options = CastOptions {
           safe: false,
           format_options: 
FormatOptions::default().with_datetime_format(Some(date_format)),
       };
       
       let array = Date32Array::from(vec![10000, 17890]);
       let res = cast_with_options(&array, &DataType::Utf8, &cast_options);
       dbg!(&res); // StringArray(["1997-05-19", "2018-12-25"])
   }
   ```
   
   Initially, I figured if the initial format attempt fails with a `Date32`, 
it'll retry as a `Date64`. This is similar to how [Postgres tries to parse an 
interval string under the normal standard, and if that errs, it tries to parse 
the string again using the ISO8601 
standard.](https://github.com/postgres/postgres/blob/58fdca2204de5f683f025df37553e5e69cb6adb1/src/backend/utils/adt/timestamp.c#L920-L929)
   
   Regardless, I'll be happy to refactor this.
   



-- 
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-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to