diff --git a/datafusion/functions/src/datetime/to_time.rs b/datafusion/functions/src/datetime/to_time.rs index 94aa49fbbad2f..45664e9416f04 100644 --- a/datafusion/functions/src/datetime/to_time.rs +++ b/datafusion/functions/src/datetime/to_time.rs @@ -22,7 +22,7 @@ use arrow::array::types::Time64NanosecondType; use arrow::array::{Array, PrimitiveArray, StringArrayType}; use arrow::datatypes::DataType; use arrow::datatypes::DataType::*; -use chrono::NaiveTime; +use chrono::format::{Item, Parsed, StrftimeItems, parse}; use datafusion_common::{Result, ScalarValue, exec_err}; use datafusion_expr::{ ColumnarValue, Documentation, ScalarFunctionArgs, ScalarUDFImpl, Signature, @@ -141,6 +141,7 @@ impl ScalarUDFImpl for ToTimeFunc { /// Convert string arguments to time (standalone function, not a method on ToTimeFunc) fn string_to_time(args: &[ColumnarValue]) -> Result { let formats = collect_formats(args)?; + let formats = compile_formats(&formats); match &args[0] { ColumnarValue::Scalar(ScalarValue::Utf8(s)) @@ -207,10 +208,25 @@ fn timestamp_to_time(arg: &ColumnarValue) -> Result { arg.cast_to(&Time64(arrow::datatypes::TimeUnit::Nanosecond), None) } +struct CompiledTimeFormat<'a> { + source: &'a str, + items: Vec>, +} + +fn compile_formats<'a>(formats: &[&'a str]) -> Vec> { + formats + .iter() + .map(|source| CompiledTimeFormat { + source, + items: StrftimeItems::new(source).collect(), + }) + .collect() +} + /// Parse time array using the provided formats fn parse_time_array<'a, A: StringArrayType<'a>>( array: &A, - formats: &[&str], + formats: &[CompiledTimeFormat<'_>], ) -> Result> { let mut values = Vec::with_capacity(array.len()); for i in 0..array.len() { @@ -224,10 +240,12 @@ fn parse_time_array<'a, A: StringArrayType<'a>>( } /// Parse time string using provided formats -fn parse_time_with_formats(s: &str, formats: &[&str]) -> Result { +fn parse_time_with_formats(s: &str, formats: &[CompiledTimeFormat<'_>]) -> Result { for format in formats { - if let Ok(time) = NaiveTime::parse_from_str(s, format) { - // Use Arrow's time_to_time64ns function instead of custom implementation + let mut parsed = Parsed::new(); + if parse(&mut parsed, s, format.items.iter()).is_ok() + && let Ok(time) = parsed.to_naive_time() + { return Ok(time_to_time64ns(time)); } } @@ -235,5 +253,8 @@ fn parse_time_with_formats(s: &str, formats: &[&str]) -> Result { "Error parsing '{}' as time. Tried formats: {:?}", s, formats + .iter() + .map(|format| format.source) + .collect::>() ) } diff --git a/datafusion/sqllogictest/test_files/datetime/timestamps.slt b/datafusion/sqllogictest/test_files/datetime/timestamps.slt index 9ac00e72b47e6..bf29550cd5779 100644 --- a/datafusion/sqllogictest/test_files/datetime/timestamps.slt +++ b/datafusion/sqllogictest/test_files/datetime/timestamps.slt @@ -3523,6 +3523,28 @@ select to_time(time_str) from time_strings; statement ok drop table time_strings; +# Table input with multiple formats +# `%Q` is intentionally invalid; subsequent formats should still be tried. +query D rowsort +select to_time( + time_str, + '%Q', + '%H:%M:%S', + '%H-%M-%S', + '%H/%M/%S' +) from ( + values + ('12:30:45'), + ('14-25-30'), + ('09/05/01'), + (NULL) +) as formatted_time_strings(time_str); +---- +09:05:01 +12:30:45 +14:25:30 +NULL + # Error cases query error Error parsing 'not_a_time' as time