comphead commented on code in PR #12227: URL: https://github.com/apache/datafusion/pull/12227#discussion_r1738900184
########## 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") { + Some(v) => Ok(v), + None => Err(internal_datafusion_err!( + "Unable to cast to Date32 for converting from i64 to i32 failed" + )), }, "to_date", ), 2.. => handle_multiple::<Date32Type, _, Date32Type, _>( args, - |s, format| { - string_to_timestamp_nanos_formatted(s, format) - .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") - }) - }) + |s, format| match Date32Type::parse_formatted(s, format) { + Some(v) => Ok(v), + None => Err(internal_datafusion_err!( Review Comment: ```suggestion None => internal_err!( ``` -- 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