Change to use internal chrono errors in parsing datetime (gluesql#1010)

We previously added an error that mimics chrono::format::ParseErrorKind.
You may need to react to chrono updates, but there are unreachable branches that can go unnoticed.

But here chrono doesn't provide a public constructor for ParseError.
https://github.com/chronotope/chrono/blob/d147e9a2152d1d961372bf5a6a261c644746efdf/src/format/mod.rs#L347-L348
Therefore, it is not compatible with the existing test method.

So we change the way we test

core/src/executor/evaluate/error.rs

@@ -1,14 +1,15 @@
 use {
     crate::ast::{Aggregate, Expr},
-    serde::Serialize,
+    serde::{Serialize, Serializer},
     std::fmt::Debug,
     thiserror::Error,
 };
 
 #[derive(Error, Serialize, Debug, PartialEq)]
 pub enum EvaluateError {
     #[error(transparent)]
-    ChronoFormat(#[from] ChronoFormatError),
+    #[serde(serialize_with = "error_serialize")]
+    FormatParseError(#[from] chrono::format::ParseError),
 
     #[error("literal add on non-numeric")]
     LiteralAddOnNonNumeric,
@@ -74,68 +75,10 @@ pub enum EvaluateError {
     ChrFunctionRequiresIntegerValueInRange0To255,
 }
 
-#[derive(Error, Serialize, Debug, PartialEq)]
-pub enum ChronoFormatError {
-    /// Given field is out of permitted range.
-    #[error("given field is out of permitted range")]
-    OutOfRange,
-
-    /// There is no possible date and time value with given set of fields.
-    ///
-    /// This does not include the out-of-range conditions, which are trivially invalid.
-    /// It includes the case that there are one or more fields that are inconsistent to each other.
-    #[error("the given date and time value is impossible to be formmated")]
-    Impossible,
-
-    /// Given set of fields is not enough to make a requested date and time value.
-    ///
-    /// Note that there *may* be a case that given fields constrain the possible values so much
-    /// that there is a unique possible value. Chrono only tries to be correct for
-    /// most useful sets of fields however, as such constraint solving can be expensive.
-    #[error("given set of field is not enough to be formatted")]
-    NotEnough,
-
-    /// The input string has some invalid character sequence for given formatting items.
-    #[error("given format string has invalid specifier")]
-    Invalid,
-
-    /// The input string has been prematurely ended.
-    #[error("input string has been permaturely ended")]
-    TooShort,
-
-    /// All formatting items have been read but there is a remaining input.
-    #[error("given format string is missing some specifier")]
-    TooLong,
-
-    /// There was an error on the formatting string, or there were non-supported formating items.
-    #[error("given format string includes non-supported formmating item")]
-    BadFormat,
-
-    // TODO: Change this- to `#[non_exhaustive]` (on the enum) when MSRV is increased
-    #[error("unreachable chrono format error")]
-    Unreachable,
-}
-
-impl ChronoFormatError {
-    pub fn err_into(error: chrono::format::ParseError) -> crate::result::Error {
-        let error: ChronoFormatError = error.into();
-        let error: EvaluateError = error.into();
-        error.into()
-    }
-}
-
-impl From<chrono::format::ParseError> for ChronoFormatError {
-    fn from(error: chrono::format::ParseError) -> ChronoFormatError {
-        use chrono::format::ParseErrorKind::*;
-        match error.kind() {
-            OutOfRange => ChronoFormatError::OutOfRange,
-            Impossible => ChronoFormatError::Impossible,
-            NotEnough => ChronoFormatError::NotEnough,
-            Invalid => ChronoFormatError::Invalid,
-            TooShort => ChronoFormatError::TooShort,
-            TooLong => ChronoFormatError::TooLong,
-            BadFormat => ChronoFormatError::BadFormat,
-            __Nonexhaustive => ChronoFormatError::Unreachable,
-        }
-    }
+fn error_serialize<S>(error: &chrono::format::ParseError, serializer: S) -> Result<S::Ok, S::Error>
+where
+    S: Serializer,
+{
+    let display = format!("{}", error);
+    serializer.serialize_str(&display)
 }

core/src/executor/evaluate/function.rs

@@ -1,5 +1,5 @@
 use {
-    super::{ChronoFormatError, EvaluateError, Evaluated},
+    super::{EvaluateError, Evaluated},
     crate::{
         ast::{DataType, DateTimeField, TrimWhereField},
         data::Value,
@@ -541,10 +541,14 @@ pub fn to_date<'a>(
     match expr.try_into()? {
         Value::Str(expr) => {
             let format = eval_to_str!(name, format);
+
             chrono::NaiveDate::parse_from_str(&expr, &format)
-                .map_err(ChronoFormatError::err_into)
                 .map(Value::Date)
                 .map(Evaluated::from)
+                .map_err(|err| {
+                    let err: EvaluateError = err.into();
+                    err.into()
+                })
         }
         _ => Err(EvaluateError::FunctionRequiresStringValue(name).into()),
     }
@@ -558,10 +562,14 @@ pub fn to_timestamp<'a>(
     match expr.try_into()? {
         Value::Str(expr) => {
             let format = eval_to_str!(name, format);
+
             chrono::NaiveDateTime::parse_from_str(&expr, &format)
-                .map_err(ChronoFormatError::err_into)
                 .map(Value::Timestamp)
                 .map(Evaluated::from)
+                .map_err(|err| {
+                    let err: EvaluateError = err.into();
+                    err.into()
+                })
         }
         _ => Err(EvaluateError::FunctionRequiresStringValue(name).into()),
     }
@@ -575,10 +583,14 @@ pub fn to_time<'a>(
     match expr.try_into()? {
         Value::Str(expr) => {
             let format = eval_to_str!(name, format);
+
             chrono::NaiveTime::parse_from_str(&expr, &format)
                 .map(Value::Time)
-                .map_err(ChronoFormatError::err_into)
                 .map(Evaluated::from)
+                .map_err(|err| {
+                    let err: EvaluateError = err.into();
+                    err.into()
+                })
         }
         _ => Err(EvaluateError::FunctionRequiresStringValue(name).into()),
     }

core/src/executor/evaluate/mod.rs

@@ -22,11 +22,7 @@ use {
     std::{borrow::Cow, rc::Rc},
 };
 
-pub use {
-    error::{ChronoFormatError, EvaluateError},
-    evaluated::Evaluated,
-    stateless::evaluate_stateless,
-};
+pub use {error::EvaluateError, evaluated::Evaluated, stateless::evaluate_stateless};
 
 #[async_recursion(?Send)]
 pub async fn evaluate<'a>(

core/src/executor/mod.rs

@@ -15,7 +15,7 @@ mod validate;
 pub use {
     aggregate::AggregateError,
     alter::AlterError,
-    evaluate::{evaluate_stateless, ChronoFormatError, EvaluateError},
+    evaluate::{evaluate_stateless, EvaluateError},
     execute::{ExecuteError, Payload, PayloadVariable},
     fetch::FetchError,
     select::SelectError,

core/src/result.rs

@@ -81,7 +81,7 @@ pub enum Error {
     Plan(#[from] PlanError),
 }
 
-pub type Result<T> = std::result::Result<T, Error>;
+pub type Result<T, E = Error> = std::result::Result<T, E>;
 pub type MutResult<T, U> = std::result::Result<(T, U), (T, Error)>;
 
 impl PartialEq for Error {

test-suite/src/function/to_date.rs

@@ -1,13 +1,23 @@
 use crate::*;
+
 test_case!(to_date, async move {
     use {
         chrono::{NaiveDate, NaiveTime},
-        gluesql_core::{
-            executor::{ChronoFormatError, EvaluateError},
-            prelude::Value::*,
-        },
+        gluesql_core::{executor::EvaluateError, prelude::Value::*},
     };
 
+    fn assert_chrono_error_kind_eq(
+        error: gluesql_core::result::Error,
+        kind: chrono::format::ParseErrorKind,
+    ) {
+        match error {
+            gluesql_core::result::Error::Evaluate(EvaluateError::FormatParseError(err)) => {
+                assert_eq!(err.kind(), kind)
+            }
+            _ => panic!("invalid error: {error}"),
+        }
+    }
+
     let test_cases = vec![
         (
             "VALUES(TO_DATE('2017-06-15', '%Y-%m-%d'))",
@@ -66,47 +76,63 @@ test_case!(to_date, async move {
             )),
         ),
         (
-            "SELECT TO_DATE('2015-09-05', '%Y-%m') AS date",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::TooLong).into()),
+            "SELECT TO_DATE(DATE '2017-06-15','%Y-%m-%d') AS date",
+            Err(EvaluateError::FunctionRequiresStringValue("TO_DATE".to_owned()).into()),
         ),
         (
-            "SELECT TO_TIME('23:56', '%H:%M:%S') AS time",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::TooShort).into()),
+            "SELECT TO_TIMESTAMP(TIMESTAMP '2015-09-05 23:56:04','%Y-%m-%d') AS date",
+            Err(EvaluateError::FunctionRequiresStringValue("TO_TIMESTAMP".to_owned()).into()),
         ),
         (
-            "SELECT TO_TIMESTAMP('2015-05 23', '%Y-%d %H') AS timestamp",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::NotEnough).into()),
+            "SELECT TO_TIME(TIME '23:56:04','%H:%M:%S') AS date",
+            Err(EvaluateError::FunctionRequiresStringValue("TO_TIME".to_owned()).into()),
+        ),
+    ];
+
+    for (sql, expected) in test_cases {
+        test!(sql, expected);
+    }
+
+    let error_cases = [
+        (
+            run_err!("SELECT TO_DATE('2015-09-05', '%Y-%m') AS date"),
+            chrono::format::ParseErrorKind::TooLong,
         ),
         (
-            "SELECT TO_TIMESTAMP('2015-14-05 23:56:12','%Y-%m-%d %H:%M:%S') AS timestamp;",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::OutOfRange).into()),
+            run_err!("SELECT TO_TIME('23:56', '%H:%M:%S') AS time"),
+            chrono::format::ParseErrorKind::TooShort,
         ),
         (
-            "SELECT TO_TIMESTAMP('2015-14-05 23:56:12','%Y-%m-%d %H:%M:%%S') AS timestamp;",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::Invalid).into()),
+            run_err!("SELECT TO_TIMESTAMP('2015-05 23', '%Y-%d %H') AS timestamp"),
+            chrono::format::ParseErrorKind::NotEnough,
         ),
         (
-            "SELECT TO_TIMESTAMP('2015-09-05 23:56:04', '%Y-%m-%d %H:%M:%M') AS timestamp",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::Impossible).into()),
+            run_err!("SELECT TO_TIMESTAMP('2015-14-05 23:56:12','%Y-%m-%d %H:%M:%S') AS timestamp"),
+            chrono::format::ParseErrorKind::OutOfRange,
         ),
         (
-            "SELECT TO_TIMESTAMP('2015-09-05 23:56:04', '%Y-%m-%d %H:%M:%') AS timestamp",
-            Err(EvaluateError::ChronoFormat(ChronoFormatError::BadFormat).into()),
+            run_err!("SELECT TO_TIMESTAMP('2015-14-05 23:56:12','%Y-%m-%d %H:%M:%S') AS timestamp"),
+            chrono::format::ParseErrorKind::OutOfRange,
         ),
         (
-            "SELECT TO_DATE(DATE '2017-06-15','%Y-%m-%d') AS date",
-            Err(EvaluateError::FunctionRequiresStringValue("TO_DATE".to_owned()).into()),
+            run_err!(
+                "SELECT TO_TIMESTAMP('2015-14-05 23:56:12','%Y-%m-%d %H:%M:%%S') AS timestamp;"
+            ),
+            chrono::format::ParseErrorKind::Invalid,
         ),
         (
-            "SELECT TO_TIMESTAMP(TIMESTAMP '2015-09-05 23:56:04','%Y-%m-%d') AS date",
-            Err(EvaluateError::FunctionRequiresStringValue("TO_TIMESTAMP".to_owned()).into()),
+            run_err!(
+                "SELECT TO_TIMESTAMP('2015-09-05 23:56:04', '%Y-%m-%d %H:%M:%M') AS timestamp"
+            ),
+            chrono::format::ParseErrorKind::Impossible,
         ),
         (
-            "SELECT TO_TIME(TIME '23:56:04','%H:%M:%S') AS date",
-            Err(EvaluateError::FunctionRequiresStringValue("TO_TIME".to_owned()).into()),
+            run_err!("SELECT TO_TIMESTAMP('2015-09-05 23:56:04', '%Y-%m-%d %H:%M:%') AS timestamp"),
+            chrono::format::ParseErrorKind::BadFormat,
         ),
     ];
-    for (sql, expected) in test_cases {
-        test!(sql, expected);
+
+    for (error, kind) in error_cases {
+        assert_chrono_error_kind_eq(error, kind);
     }
 });

test-suite/src/tester/mod.rs

@@ -262,6 +262,13 @@ macro_rules! test_case {
                 };
             }
 
+            #[allow(unused_macros)]
+            macro_rules! run_err {
+                ($sql: expr) => {
+                    $crate::run($sql, glue, None).await.unwrap_err()
+                };
+            }
+
             #[allow(unused_macros)]
             macro_rules! count {
                 ($count: expr, $sql: expr) => {