[CHORE] Implement thiserror::Error for DaftError and arrow2::Error (#…

…2785)

# Overview
- implemented `thiserror::Error` for `common::error::DaftError` and
`arrow2::error::Error`
- removed many `impl From<X> for DaftError` and many `impl From<X> for
arrow2::Error`

Cargo.toml

@@ -164,6 +164,7 @@ snafu = {version = "0.7.4", features = ["futures"]}
 sqlparser = "0.49.0"
 sysinfo = "0.30.12"
 test-log = "0.2.16"
+thiserror = "1.0.63"
 tiktoken-rs = "0.5.9"
 tokio = {version = "1.37.0", features = [
   "net",

src/arrow2/Cargo.toml

@@ -70,6 +70,7 @@ simdutf8 = "0.1.4"
 streaming-iterator = {version = "0.1", optional = true}
 # for division/remainder optimization at runtime
 strength_reduce = {version = "0.2", optional = true}
+thiserror = {workspace = true}
 zstd = {version = "0.12", optional = true}
 
 # parquet support

src/arrow2/src/error.rs

@@ -1,24 +1,47 @@
 //! Defines [`Error`], representing all errors returned by this crate.
-use std::fmt::{Debug, Display, Formatter};
+
+use thiserror::Error;
 
 /// Enum with all errors in this crate.
-#[derive(Debug)]
+#[derive(Debug, Error)]
 #[non_exhaustive]
 pub enum Error {
     /// Returned when functionality is not yet available.
+    #[error("Not yet implemented: {0}")]
     NotYetImplemented(String),
+
+    #[error("{0}")]
+    Utf8Error(#[from] simdutf8::basic::Utf8Error),
+
+    #[error("{0}")]
+    StdUtf8Error(#[from] std::str::Utf8Error),
+
+    #[error("{0}")]
+    TryReserveError(#[from] std::collections::TryReserveError),
+
     /// Wrapper for an error triggered by a dependency
+    #[error("External error{0}: {1}")]
     External(String, Box<dyn std::error::Error + Send + Sync>),
+
     /// Wrapper for IO errors
-    Io(std::io::Error),
+    #[error("Io error: {0}")]
+    Io(#[from] std::io::Error),
+
     /// When an invalid argument is passed to a function.
+    #[error("Invalid argument error: {0}")]
     InvalidArgumentError(String),
+
     /// Error during import or export to/from a format
+    #[error("External format error: {0}")]
     ExternalFormat(String),
+
     /// Whenever pushing to a container fails because it does not support more entries.
     /// The solution is usually to use a higher-capacity container-backing type.
+    #[error("Operation overflew the backing container.")]
     Overflow,
+
     /// Whenever incoming data from the C data interface, IPC or Flight does not fulfil the Arrow specification.
+    #[error("{0}")]
     OutOfSpec(String),
 }
 
@@ -32,69 +55,10 @@ impl Error {
         Self::OutOfSpec(msg.into())
     }
 
-    #[allow(dead_code)]
     pub(crate) fn nyi<A: Into<String>>(msg: A) -> Self {
         Self::NotYetImplemented(msg.into())
     }
 }
 
-impl From<::std::io::Error> for Error {
-    fn from(error: std::io::Error) -> Self {
-        Error::Io(error)
-    }
-}
-
-impl From<std::str::Utf8Error> for Error {
-    fn from(error: std::str::Utf8Error) -> Self {
-        Error::External("".to_string(), Box::new(error))
-    }
-}
-
-impl From<std::string::FromUtf8Error> for Error {
-    fn from(error: std::string::FromUtf8Error) -> Self {
-        Error::External("".to_string(), Box::new(error))
-    }
-}
-
-impl From<simdutf8::basic::Utf8Error> for Error {
-    fn from(error: simdutf8::basic::Utf8Error) -> Self {
-        Error::External("".to_string(), Box::new(error))
-    }
-}
-
-impl From<std::collections::TryReserveError> for Error {
-    fn from(_: std::collections::TryReserveError) -> Error {
-        Error::Overflow
-    }
-}
-
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Error::NotYetImplemented(source) => {
-                write!(f, "Not yet implemented: {}", &source)
-            }
-            Error::External(message, source) => {
-                write!(f, "External error{}: {}", message, &source)
-            }
-            Error::Io(desc) => write!(f, "Io error: {desc}"),
-            Error::InvalidArgumentError(desc) => {
-                write!(f, "Invalid argument error: {desc}")
-            }
-            Error::ExternalFormat(desc) => {
-                write!(f, "External format error: {desc}")
-            }
-            Error::Overflow => {
-                write!(f, "Operation overflew the backing container.")
-            }
-            Error::OutOfSpec(message) => {
-                write!(f, "{message}")
-            }
-        }
-    }
-}
-
-impl std::error::Error for Error {}
-
 /// Typedef for a [`std::result::Result`] of an [`Error`].
 pub type Result<T> = std::result::Result<T, Error>;

src/common/error/Cargo.toml

@@ -3,6 +3,7 @@ arrow2 = {workspace = true}
 pyo3 = {workspace = true, optional = true}
 regex = {workspace = true}
 serde_json = {workspace = true}
+thiserror = {workspace = true}
 
 [features]
 python = ["dep:pyo3"]

src/common/error/src/error.rs

@@ -1,113 +1,45 @@
-use std::{
-    fmt::{Display, Formatter, Result},
-    io,
-};
+use thiserror::Error;
 
+pub type DaftResult<T> = std::result::Result<T, DaftError>;
 pub type GenericError = Box<dyn std::error::Error + Send + Sync>;
 
-#[derive(Debug)]
+#[derive(Debug, Error)]
 pub enum DaftError {
+    #[error("DaftError::FieldNotFound {0}")]
     FieldNotFound(String),
+    #[error("DaftError::SchemaMismatch {0}")]
     SchemaMismatch(String),
+    #[error("DaftError::TypeError {0}")]
     TypeError(String),
+    #[error("DaftError::ComputeError {0}")]
     ComputeError(String),
-    ArrowError(String),
+    #[error("DaftError::ArrowError {0}")]
+    ArrowError(#[from] arrow2::error::Error),
+    #[error("DaftError::ValueError {0}")]
     ValueError(String),
     #[cfg(feature = "python")]
-    PyO3Error(pyo3::PyErr),
-    IoError(io::Error),
-    FileNotFound {
-        path: String,
-        source: GenericError,
-    },
+    #[error("DaftError::PyO3Error {0}")]
+    PyO3Error(#[from] pyo3::PyErr),
+    #[error("DaftError::IoError {0}")]
+    IoError(#[from] std::io::Error),
+    #[error("DaftError::FileNotFound {path}: {source}")]
+    FileNotFound { path: String, source: GenericError },
+    #[error("DaftError::InternalError {0}")]
     InternalError(String),
+    #[error("ConnectTimeout {0}")]
     ConnectTimeout(GenericError),
+    #[error("ReadTimeout {0}")]
     ReadTimeout(GenericError),
+    #[error("ByteStreamError {0}")]
     ByteStreamError(GenericError),
+    #[error("SocketError {0}")]
     SocketError(GenericError),
+    #[error("DaftError::External {0}")]
     External(GenericError),
-}
-
-impl std::error::Error for DaftError {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            DaftError::FieldNotFound(_)
-            | DaftError::SchemaMismatch(_)
-            | DaftError::TypeError(_)
-            | DaftError::ComputeError(_)
-            | DaftError::ArrowError(_)
-            | DaftError::ValueError(_)
-            | DaftError::InternalError(_) => None,
-            DaftError::IoError(io_error) => Some(io_error),
-            DaftError::FileNotFound { source, .. }
-            | DaftError::SocketError(source)
-            | DaftError::External(source)
-            | DaftError::ReadTimeout(source)
-            | DaftError::ConnectTimeout(source)
-            | DaftError::ByteStreamError(source) => Some(&**source),
-            #[cfg(feature = "python")]
-            DaftError::PyO3Error(pyerr) => Some(pyerr),
-        }
-    }
-}
-
-impl From<arrow2::error::Error> for DaftError {
-    fn from(error: arrow2::error::Error) -> Self {
-        match error {
-            arrow2::error::Error::Io(_) => DaftError::ByteStreamError(error.into()),
-            _ => DaftError::ArrowError(error.to_string()),
-        }
-    }
-}
-
-impl From<serde_json::Error> for DaftError {
-    fn from(error: serde_json::Error) -> Self {
-        DaftError::IoError(error.into())
-    }
-}
-
-impl From<io::Error> for DaftError {
-    fn from(error: io::Error) -> Self {
-        DaftError::IoError(error)
-    }
-}
-
-impl From<regex::Error> for DaftError {
-    fn from(error: regex::Error) -> Self {
-        DaftError::ValueError(error.to_string())
-    }
-}
-
-impl From<std::fmt::Error> for DaftError {
-    fn from(error: std::fmt::Error) -> Self {
-        DaftError::ComputeError(error.to_string())
-    }
-}
-
-pub type DaftResult<T> = std::result::Result<T, DaftError>;
-
-impl Display for DaftError {
-    // `f` is a buffer, and this method must write the formatted string into it
-    fn fmt(&self, f: &mut Formatter) -> Result {
-        match self {
-            Self::FieldNotFound(s) => write!(f, "DaftError::FieldNotFound {s}"),
-            Self::SchemaMismatch(s) => write!(f, "DaftError::SchemaMismatch {s}"),
-            Self::TypeError(s) => write!(f, "DaftError::TypeError {s}"),
-            Self::ComputeError(s) => write!(f, "DaftError::ComputeError {s}"),
-            Self::ArrowError(s) => write!(f, "DaftError::ArrowError {s}"),
-            Self::ValueError(s) => write!(f, "DaftError::ValueError {s}"),
-            Self::InternalError(s) => write!(f, "DaftError::InternalError {s}"),
-            #[cfg(feature = "python")]
-            Self::PyO3Error(e) => write!(f, "DaftError::PyO3Error {e}"),
-            Self::IoError(e) => write!(f, "DaftError::IoError {e}"),
-            Self::External(e) => write!(f, "DaftError::External {}", e),
-            Self::FileNotFound { path, source } => {
-                write!(f, "DaftError::FileNotFound {path}: {source}")
-            }
-            Self::ByteStreamError(e) => write!(f, "ByteStreamError: {}", e),
-            Self::ConnectTimeout(e) => write!(f, "ConnectTimeout: {}", e),
-            Self::ReadTimeout(e) => write!(f, "ReadTimeout: {}", e),
-            Self::SocketError(e) => write!(f, "SocketError: {}", e),
-        }
-    }
+    #[error("DaftError::SerdeJsonError {0}")]
+    SerdeJsonError(#[from] serde_json::Error),
+    #[error("DaftError::FmtError {0}")]
+    FmtError(#[from] std::fmt::Error),
+    #[error("DaftError::RegexError {0}")]
+    RegexError(#[from] regex::Error),
 }

src/common/error/src/python.rs

@@ -1,13 +1,8 @@
+use pyo3::exceptions::PyFileNotFoundError;
 use pyo3::import_exception;
 
 use crate::DaftError;
 
-impl From<pyo3::PyErr> for DaftError {
-    fn from(error: pyo3::PyErr) -> Self {
-        DaftError::PyO3Error(error)
-    }
-}
-
 import_exception!(daft.exceptions, DaftCoreException);
 import_exception!(daft.exceptions, DaftTypeError);
 import_exception!(daft.exceptions, ConnectTimeoutError);
@@ -17,8 +12,6 @@ import_exception!(daft.exceptions, SocketError);
 
 impl std::convert::From<DaftError> for pyo3::PyErr {
     fn from(err: DaftError) -> pyo3::PyErr {
-        use pyo3::exceptions::PyFileNotFoundError;
-
         match err {
             DaftError::PyO3Error(pyerr) => pyerr,
             DaftError::FileNotFound { path, source } => {

src/daft-core/src/array/ops/utf8.rs

@@ -804,7 +804,7 @@ impl Utf8Array {
         }
 
         let self_iter = create_broadcasted_str_iter(self, expected_size);
-        let result = match nchars.len() {
+        let result: Utf8Array = match nchars.len() {
             1 => {
                 let n = nchars.get(0).unwrap();
                 let n: usize = NumCast::from(n).ok_or_else(|| {
@@ -868,7 +868,7 @@ impl Utf8Array {
         }
 
         let self_iter = create_broadcasted_str_iter(self, expected_size);
-        let result = match nchars.len() {
+        let result: Utf8Array = match nchars.len() {
             1 => {
                 let n = nchars.get(0).unwrap();
                 let n: usize = NumCast::from(n).ok_or_else(|| {
@@ -1009,7 +1009,7 @@ impl Utf8Array {
         }
 
         let self_iter = create_broadcasted_str_iter(self, expected_size);
-        let result = match n.len() {
+        let result: Utf8Array = match n.len() {
             1 => {
                 let n = n.get(0).unwrap();
                 let n: usize = NumCast::from(n).ok_or_else(|| {
@@ -1256,7 +1256,7 @@ impl Utf8Array {
 
         let self_iter = create_broadcasted_str_iter(self, expected_size);
         let padchar_iter = create_broadcasted_str_iter(padchar, expected_size);
-        let result = match length.len() {
+        let result: Utf8Array = match length.len() {
             1 => {
                 let len = length.get(0).unwrap();
                 let len: usize = NumCast::from(len).ok_or_else(|| {

src/parquet2/Cargo.toml

@@ -11,6 +11,7 @@ seq-macro = {version = "0.3", default-features = false}
 serde = {version = "^1.0", features = ["derive"]}
 snap = {version = "^1.1", optional = true}
 streaming-decompression = "0.1"
+thiserror = {workspace = true}
 xxhash-rust = {version = "0.8", optional = true, features = ["xxh64"]}
 zstd = {version = "^0.12", optional = true, default-features = false}