refactor

query_engine/src/logical_optimizer/type_conversion.rs

@@ -187,7 +187,10 @@ impl<'a> TypeRewriter<'a> {
     fn cast_scalar_value(value: &ScalarValue, data_type: &DataType) -> Result<ScalarValue> {
         if let DataType::Timestamp(_, _) = data_type {
             if let ScalarValue::Utf8(Some(v)) = value {
-                return string_to_timestamp_ms(v);
+                return match string_to_timestamp_ms_workaround(v) {
+                    Ok(v) => Ok(v),
+                    _ => string_to_timestamp_ms(v),
+                };
             }
         }
 
@@ -282,7 +285,14 @@ impl<'a> ExprRewriter for TypeRewriter<'a> {
 }
 
 fn string_to_timestamp_ms(string: &str) -> Result<ScalarValue> {
-    // TODO(lee): remove following codes after PR(https://github.com/apache/arrow-rs/pull/3787) merged
+    let ts = string_to_timestamp_nanos(string)
+        .map(|t| t / 1_000_000)
+        .map_err(DataFusionError::from)?;
+    Ok(ScalarValue::TimestampMillisecond(Some(ts), None))
+}
+
+// TODO(lee): remove following codes after PR(https://github.com/apache/arrow-rs/pull/3787) merged
+fn string_to_timestamp_ms_workaround(string: &str) -> Result<ScalarValue> {
     // Because function `string_to_timestamp_nanos` returns a NaiveDateTime's
     // nanoseconds from a string without a specify time zone, We need to convert
     // it to local timestamp.
@@ -301,10 +311,10 @@ fn string_to_timestamp_ms(string: &str) -> Result<ScalarValue> {
         return Ok(ScalarValue::TimestampMillisecond(Some(mills), None));
     }
 
-    let ts = string_to_timestamp_nanos(string)
-        .map(|t| t / 1_000_000)
-        .map_err(DataFusionError::from)?;
-    Ok(ScalarValue::TimestampMillisecond(Some(ts), None))
+    Err(ArrowError::CastError(format!(
+        "Error parsing '{string}' as timestamp: local time representation is invalid"
+    )))
+    .map_err(DataFusionError::from)
 }
 
 /// Converts the naive datetime (which has no specific timezone) to a
@@ -570,28 +580,24 @@ mod tests {
     }
 
     #[test]
-    fn test_string_to_timestamp_ms() {
+    fn test_string_to_timestamp_ms_workaround() {
         let date_string = [
             "2021-09-07T16:00:00+08:00",
             "2021-09-07 16:00:00+08:00",
             "2021-09-07T16:00:00Z",
             "2021-09-07 16:00:00Z",
         ];
-        let expects: [i64; 4] = [1631001600000, 1631001600000, 1631030400000, 1631030400000];
-        for (index, &string) in date_string.iter().enumerate() {
-            let result = type_conversion::string_to_timestamp_ms(string);
-            if let Ok(ScalarValue::TimestampMillisecond(Some(mills), _)) = result {
-                let expect = *expects.get(index).unwrap();
-                assert_eq!(mills, expect)
-            }
+        for string in date_string {
+            let result = type_conversion::string_to_timestamp_ms_workaround(string);
+            assert!(result.is_err());
         }
 
         let date_string = "2021-09-07 16:00:00".to_string();
         let d = NaiveDate::from_ymd_opt(2021, 9, 7).unwrap();
         let t = NaiveTime::from_hms_milli_opt(16, 0, 0, 0).unwrap();
         let dt = NaiveDateTime::new(d, t);
         let expect = naive_datetime_to_timestamp(&date_string, dt).unwrap();
-        let result = type_conversion::string_to_timestamp_ms(&date_string);
+        let result = type_conversion::string_to_timestamp_ms_workaround(&date_string);
         if let Ok(ScalarValue::TimestampMillisecond(Some(mills), _)) = result {
             assert_eq!(mills, expect)
         }