Skip to content

Commit 8a895d5

Browse files
committed
Fix: Avoid recursive external error wrapping in type conversion.
1 parent 66b4da2 commit 8a895d5

File tree

1 file changed

+36
-2
lines changed

1 file changed

+36
-2
lines changed

datafusion/common/src/error.rs

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,14 @@ impl From<ParserError> for DataFusionError {
293293

294294
impl From<GenericError> for DataFusionError {
295295
fn from(err: GenericError) -> Self {
296-
DataFusionError::External(err)
296+
// If the error is already a DataFusionError, not wrapping it.
297+
if err.is::<DataFusionError>() {
298+
if let Ok(e) = err.downcast::<DataFusionError>() {
299+
*e
300+
} else { unreachable!() }
301+
} else {
302+
DataFusionError::External(err)
303+
}
297304
}
298305
}
299306

@@ -656,7 +663,7 @@ pub fn unqualified_field_not_found(name: &str, schema: &DFSchema) -> DataFusionE
656663
mod test {
657664
use std::sync::Arc;
658665

659-
use crate::error::DataFusionError;
666+
use crate::error::{DataFusionError, GenericError};
660667
use arrow::error::ArrowError;
661668

662669
#[test]
@@ -810,6 +817,33 @@ mod test {
810817
);
811818
}
812819

820+
#[test]
821+
fn external_error() {
822+
// assert not wrapping DataFusionError
823+
let generic_error: GenericError = Box::new(DataFusionError::Plan("test".to_string()));
824+
let datafusion_error: DataFusionError = generic_error.into();
825+
println!("{}", datafusion_error.strip_backtrace());
826+
assert_eq!(datafusion_error.strip_backtrace(), "Error during planning: test");
827+
828+
// assert wrapping other Error
829+
let generic_error: GenericError = Box::new(std::io::Error::new(std::io::ErrorKind::Other, "io error"));
830+
let datafusion_error: DataFusionError = generic_error.into();
831+
println!("{}", datafusion_error.strip_backtrace());
832+
assert_eq!(datafusion_error.strip_backtrace(), "External error: io error");
833+
}
834+
835+
#[test]
836+
fn external_error_no_recursive() {
837+
let generic_error_1: GenericError = Box::new(std::io::Error::new(std::io::ErrorKind::Other, "io error"));
838+
let external_error_1 : DataFusionError = generic_error_1.into();
839+
let generic_error_2 : GenericError = Box::new(external_error_1);
840+
let external_error_2: DataFusionError = generic_error_2.into();
841+
842+
println!("{}", external_error_2);
843+
assert!(external_error_2.to_string().starts_with("External error: io error"));
844+
}
845+
846+
813847
/// Model what happens when implementing SendableRecordBatchStream:
814848
/// DataFusion code needs to return an ArrowError
815849
fn return_arrow_error() -> arrow::error::Result<()> {

0 commit comments

Comments
 (0)