From 436fdbaed9cb7874959df9bc254a97a051ce7004 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Sat, 22 Jan 2022 05:50:48 -0500 Subject: [PATCH 1/4] Cleanup DataFusion error conversion + handling --- datafusion/src/error.rs | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/datafusion/src/error.rs b/datafusion/src/error.rs index bf9df5077c90..edced80153de 100644 --- a/datafusion/src/error.rs +++ b/datafusion/src/error.rs @@ -74,6 +74,8 @@ pub enum DataFusionError { impl DataFusionError { /// Wraps this [DataFusionError] as an [arrow::error::ArrowError]. + /// + /// TODO this can be removed in favor if the conversion below pub fn into_arrow_external_error(self) -> ArrowError { ArrowError::from_external_error(Box::new(self)) } @@ -155,3 +157,40 @@ impl Display for DataFusionError { } impl error::Error for DataFusionError {} + +#[cfg(test)] +mod test { + use arrow::error::ArrowError; + use crate::error::DataFusionError; + + #[test] + fn arrow_error_to_datafusion() { + let res = return_arrow_error().unwrap_err(); + assert_eq!(res.to_string(), "DD"); + } + + #[test] + fn datafusion_error_to_arrow() { + let res = return_datafusion_error().unwrap_err(); + assert_eq!(res.to_string(), "DD"); + } + + + + /// Model what happens when implementing SendableRecrordBatchStream: + /// DataFusion code needs to return an ArrowError + fn return_arrow_error() -> arrow::error::Result<()> { + // Expect the '?' to work + let _foo = Err(DataFusionError::Internal("foo".to_string()))?; + Ok(()) + } + + + /// Model what happens when using arrow kernels in DataFusion + /// code: need to turn an ArrowError into a DataFusionError + fn return_datafusion_error() -> crate::error::Result<()> { + // Expect the '?' to work + let _bar = Err(ArrowError::SchemaError("bar".to_string()))?; + Ok(()) + } +} From b75b062cb43435fa2228e4b236763b0a72df9a8f Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Sat, 22 Jan 2022 06:41:19 -0500 Subject: [PATCH 2/4] add conversion --- datafusion/src/error.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/datafusion/src/error.rs b/datafusion/src/error.rs index edced80153de..cfcabf6a4fde 100644 --- a/datafusion/src/error.rs +++ b/datafusion/src/error.rs @@ -93,6 +93,16 @@ impl From for DataFusionError { } } +impl From for ArrowError { + fn from(e: DataFusionError) -> Self { + match e { + DataFusionError::ArrowError(e) => e, + DataFusionError::External(e) => ArrowError::ExternalError(e), + other => ArrowError::ExternalError(Box::new(other)), + } + } +} + impl From for DataFusionError { fn from(e: ParquetError) -> Self { DataFusionError::ParquetError(e) From 6014600088fe0e7adddc12ada5c0feef1f827b8e Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Sat, 22 Jan 2022 06:43:18 -0500 Subject: [PATCH 3/4] clean up tests --- datafusion/src/error.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/datafusion/src/error.rs b/datafusion/src/error.rs index cfcabf6a4fde..2126491e7b3c 100644 --- a/datafusion/src/error.rs +++ b/datafusion/src/error.rs @@ -176,22 +176,20 @@ mod test { #[test] fn arrow_error_to_datafusion() { let res = return_arrow_error().unwrap_err(); - assert_eq!(res.to_string(), "DD"); + assert_eq!(res.to_string(), "External error: Error during planning: foo"); } #[test] fn datafusion_error_to_arrow() { let res = return_datafusion_error().unwrap_err(); - assert_eq!(res.to_string(), "DD"); + assert_eq!(res.to_string(), "Arrow error: Schema error: bar"); } - - /// Model what happens when implementing SendableRecrordBatchStream: /// DataFusion code needs to return an ArrowError fn return_arrow_error() -> arrow::error::Result<()> { // Expect the '?' to work - let _foo = Err(DataFusionError::Internal("foo".to_string()))?; + let _foo = Err(DataFusionError::Plan("foo".to_string()))?; Ok(()) } From c0782fe1712328eea4b15ee7c9615f7a05c60177 Mon Sep 17 00:00:00 2001 From: Andrew Lamb Date: Sat, 22 Jan 2022 06:58:30 -0500 Subject: [PATCH 4/4] clippy --- datafusion/src/error.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/datafusion/src/error.rs b/datafusion/src/error.rs index 2126491e7b3c..b7a8f45b2e67 100644 --- a/datafusion/src/error.rs +++ b/datafusion/src/error.rs @@ -170,13 +170,16 @@ impl error::Error for DataFusionError {} #[cfg(test)] mod test { - use arrow::error::ArrowError; use crate::error::DataFusionError; + use arrow::error::ArrowError; #[test] fn arrow_error_to_datafusion() { let res = return_arrow_error().unwrap_err(); - assert_eq!(res.to_string(), "External error: Error during planning: foo"); + assert_eq!( + res.to_string(), + "External error: Error during planning: foo" + ); } #[test] @@ -187,15 +190,16 @@ mod test { /// Model what happens when implementing SendableRecrordBatchStream: /// DataFusion code needs to return an ArrowError + #[allow(clippy::try_err)] fn return_arrow_error() -> arrow::error::Result<()> { // Expect the '?' to work let _foo = Err(DataFusionError::Plan("foo".to_string()))?; Ok(()) } - /// Model what happens when using arrow kernels in DataFusion /// code: need to turn an ArrowError into a DataFusionError + #[allow(clippy::try_err)] fn return_datafusion_error() -> crate::error::Result<()> { // Expect the '?' to work let _bar = Err(ArrowError::SchemaError("bar".to_string()))?;