comphead commented on code in PR #12227:
URL: https://github.com/apache/datafusion/pull/12227#discussion_r1738905707


##########
datafusion/functions/src/datetime/to_date.rs:
##########
@@ -47,27 +48,24 @@ impl ToDateFunc {
         match args.len() {
             1 => handle::<Date32Type, _, Date32Type>(
                 args,
-                |s| {
-                    string_to_timestamp_nanos_shim(s)
-                        .map(|n| n / (1_000_000 * 24 * 60 * 60 * 1_000))
-                        .and_then(|v| {
-                            v.try_into().map_err(|_| {
-                                internal_datafusion_err!("Unable to cast to 
Date32 for converting from i64 to i32 failed")
-                            })
-                        })
+                // For most cases 'Date32Type::parse' would also work here, as 
it semantically parses
+                // assuming a YYYY-MM-DD format. However 'Date32Type::parse' 
cannot handle negative
+                // values for year (e.g. years in BC). 
Date32Type::parse_formatted can handle also these.
+                |s| match Date32Type::parse_formatted(s, "%Y-%m-%d") {

Review Comment:
   I'm not sure the string format will be exactly that format? 



-- 
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