Skip to content

Commit b5c452a

Browse files
committed
Fix error formatting
1 parent 3346af7 commit b5c452a

File tree

3 files changed

+31
-22
lines changed

3 files changed

+31
-22
lines changed

datafusion/datasource/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ version.workspace = true
3131
all-features = true
3232

3333
[features]
34+
backtrace = ["datafusion-common/backtrace"]
3435
compression = ["async-compression", "liblzma", "bzip2", "flate2", "zstd", "tokio-util"]
3536
default = ["compression"]
3637

datafusion/datasource/src/file_stream/mod.rs

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,7 @@ mod tests {
175175
use crate::{PartitionedFile, TableSchema};
176176
use arrow::array::{AsArray, RecordBatch};
177177
use arrow::datatypes::{DataType, Field, Int32Type, Schema};
178+
use datafusion_common::DataFusionError;
178179
use datafusion_common::error::Result;
179180
use datafusion_execution::object_store::ObjectStoreUrl;
180181
use datafusion_physical_plan::metrics::ExecutionPlanMetricsSet;
@@ -796,7 +797,7 @@ mod tests {
796797

797798
insta::assert_snapshot!(test.run().await.unwrap(), @r"
798799
----- Output Stream -----
799-
DataFusionError::Internal(io failed while opening file)
800+
Error: io failed while opening file
800801
Done
801802
----- File Stream Events -----
802803
morselize_file: file1.parquet
@@ -822,7 +823,7 @@ mod tests {
822823

823824
insta::assert_snapshot!(test.run().await.unwrap(), @r"
824825
----- Output Stream -----
825-
DataFusionError::Internal(planner failed after io)
826+
Error: planner failed after io
826827
Done
827828
----- File Stream Events -----
828829
morselize_file: file1.parquet
@@ -967,22 +968,15 @@ mod tests {
967968
stream_contents.push(format!("Batch: {batch_id}"));
968969
}
969970
Err(e) => {
970-
// Pull the actual message for common types rather than
971+
// Pull the actual message for exteral errors rather than
971972
// relying on DataFusionError formatting, which changes
972973
// if backtraces are enabled, etc
973-
let message = match e {
974-
datafusion_common::DataFusionError::Execution(msg) => {
975-
format!("DataFusionError::Execution({msg})")
976-
}
977-
datafusion_common::DataFusionError::Internal(msg) => {
978-
format!("DataFusionError::Internal({msg})")
979-
}
980-
datafusion_common::DataFusionError::Plan(msg) => {
981-
format!("DataFusionError::Plan({msg})")
982-
}
983-
other => format!("{other}"),
974+
let message = if let DataFusionError::External(generic) = e {
975+
generic.to_string()
976+
} else {
977+
e.to_string()
984978
};
985-
stream_contents.push(message)
979+
stream_contents.push(format!("Error: {message}"));
986980
}
987981
}
988982
}

datafusion/datasource/src/morsel/mocks.rs

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::PartitionedFile;
2727
use crate::morsel::{Morsel, MorselPlan, MorselPlanner, Morselizer};
2828
use arrow::array::{Int32Array, RecordBatch};
2929
use arrow::datatypes::{DataType, Field, Schema};
30-
use datafusion_common::{Result, internal_datafusion_err};
30+
use datafusion_common::{DataFusionError, Result, internal_datafusion_err};
3131
use futures::Future;
3232
use futures::future::BoxFuture;
3333
use futures::stream::BoxStream;
@@ -54,6 +54,18 @@ pub(crate) struct PollsToResolve(pub usize);
5454
#[derive(Debug, Clone, PartialEq, Eq)]
5555
pub(crate) struct MockError(pub String);
5656

57+
impl Display for MockError {
58+
fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
59+
write!(f, "{}", self.0)
60+
}
61+
}
62+
63+
impl std::error::Error for MockError {
64+
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
65+
None
66+
}
67+
}
68+
5769
/// Scheduler-visible event captured by the mock morsel test harness.
5870
#[derive(Debug, Clone, PartialEq, Eq)]
5971
pub(crate) enum MorselEvent {
@@ -216,7 +228,7 @@ enum PlannerStep {
216228
result: std::result::Result<(), MockError>,
217229
},
218230
Error {
219-
message: String,
231+
error: MockError,
220232
},
221233
None,
222234
}
@@ -289,7 +301,7 @@ impl MockPlannerBuilder {
289301
/// Adds one planning step that fails during CPU planning.
290302
pub(crate) fn return_error(mut self, message: impl Into<String>) -> Self {
291303
self.steps.push(PlannerStep::Error {
292-
message: message.into(),
304+
error: MockError(message.into()),
293305
});
294306
self
295307
}
@@ -413,7 +425,9 @@ impl MorselPlanner for MockMorselPlanner {
413425
)) as BoxFuture<'static, Result<()>>,
414426
)))
415427
}
416-
PlannerStep::Error { message } => Err(internal_datafusion_err!("{message}")),
428+
PlannerStep::Error { error } => {
429+
Err(DataFusionError::External(Box::new(error)))
430+
}
417431
PlannerStep::None => Ok(None),
418432
}
419433
}
@@ -537,13 +551,13 @@ impl Future for MockIoFuture {
537551
});
538552
Poll::Ready(Ok(()))
539553
}
540-
Err(MockError(message)) => {
554+
Err(e) => {
541555
self.observer.push(MorselEvent::IoFutureErrored {
542556
planner_name: self.planner_name.clone(),
543557
io_future_id: self.io_future_id,
544-
message: message.clone(),
558+
message: e.0.clone(),
545559
});
546-
Poll::Ready(Err(internal_datafusion_err!("{message}")))
560+
Poll::Ready(Err(DataFusionError::External(Box::new(e.clone()))))
547561
}
548562
}
549563
}

0 commit comments

Comments
 (0)