From c77d523f77d256c262ab5a601f244e0932f702bb Mon Sep 17 00:00:00 2001 From: comphead Date: Sat, 8 Jun 2024 12:53:10 -0700 Subject: [PATCH 1/4] fix error message --- core/src/execution/datafusion/planner.rs | 11 ++++++++++- core/src/execution/operators/mod.rs | 8 ++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/core/src/execution/datafusion/planner.rs b/core/src/execution/datafusion/planner.rs index 6c7ea0de4..cc5943093 100644 --- a/core/src/execution/datafusion/planner.rs +++ b/core/src/execution/datafusion/planner.rs @@ -1382,7 +1382,7 @@ impl PhysicalPlanner { impl From for ExecutionError { fn from(value: DataFusionError) -> Self { - ExecutionError::DataFusionError(value.to_string()) + ExecutionError::DataFusionError(value.message().to_string()) } } @@ -1554,6 +1554,7 @@ mod tests { spark_operator, }; + use crate::execution::operators::ExecutionError; use spark_expression::expr::ExprStruct::*; use spark_operator::{operator::OpStruct, Operator}; @@ -1743,6 +1744,14 @@ mod tests { assert!(output.is_empty()); } + #[tokio::test()] + async fn from_datafusion_error_to_comet() { + let err_msg = "exec error"; + let err = datafusion_common::DataFusionError::Execution(err_msg.to_string()); + let comet_err: ExecutionError = err.into(); + assert_eq!(comet_err.to_string(), "Error from DataFusion: exec error."); + } + // Creates a filter operator which takes an `Int32Array` and selects rows that are equal to // `value`. fn create_filter(child_op: spark_operator::Operator, value: i32) -> spark_operator::Operator { diff --git a/core/src/execution/operators/mod.rs b/core/src/execution/operators/mod.rs index 5d05fdb8d..13a0d9627 100644 --- a/core/src/execution/operators/mod.rs +++ b/core/src/execution/operators/mod.rs @@ -38,19 +38,19 @@ pub use copy::*; pub enum ExecutionError { /// Simple error #[allow(dead_code)] - #[error("General execution error with reason {0}.")] + #[error("General execution error with reason: {0}.")] GeneralError(String), /// Error when deserializing an operator. - #[error("Fail to deserialize to native operator with reason {0}.")] + #[error("Fail to deserialize to native operator with reason: {0}.")] DeserializeError(String), /// Error when processing Arrow array. - #[error("Fail to process Arrow array with reason {0}.")] + #[error("Fail to process Arrow array with reason: {0}.")] ArrowError(String), /// DataFusion error - #[error("Error from DataFusion {0}.")] + #[error("Error from DataFusion: {0}.")] DataFusionError(String), } From 98fd9c0da5b3a765dc0242855716d24e934072cc Mon Sep 17 00:00:00 2001 From: comphead Date: Sun, 9 Jun 2024 15:07:28 -0700 Subject: [PATCH 2/4] fix test --- spark/src/test/scala/org/apache/comet/CometCastSuite.scala | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala b/spark/src/test/scala/org/apache/comet/CometCastSuite.scala index 25343f933..71134e550 100644 --- a/spark/src/test/scala/org/apache/comet/CometCastSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometCastSuite.scala @@ -984,9 +984,7 @@ class CometCastSuite extends CometTestBase with AdaptiveSparkPlanHelper { // both systems threw an exception so we make sure they are the same val sparkMessage = if (sparkException.getCause != null) sparkException.getCause.getMessage else null - // We have to workaround https://github.com/apache/datafusion-comet/issues/293 here by - // removing the "Execution error: " error message prefix that is added by DataFusion - val cometMessage = cometException.getCause.getMessage.replace("Execution error: ", "") + val cometMessage = cometException.getCause.getMessage if (CometSparkSessionExtensions.isSpark40Plus) { // for Spark 4 we expect to sparkException carries the message assert( From fc1fabd0d9ccaf071596788336f2041ac6fe3292 Mon Sep 17 00:00:00 2001 From: comphead Date: Mon, 10 Jun 2024 09:02:55 -0700 Subject: [PATCH 3/4] fix test --- core/src/errors.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/core/src/errors.rs b/core/src/errors.rs index af4fd2697..05b18f0ef 100644 --- a/core/src/errors.rs +++ b/core/src/errors.rs @@ -152,9 +152,10 @@ pub enum CometError { #[error("{msg}")] Panic { msg: String }, - #[error(transparent)] + #[error("{msg}")] DataFusion { - #[from] + msg: String, + #[source] source: DataFusionError, }, @@ -184,11 +185,18 @@ impl convert::From> for CometError { } } } - +impl From for CometError { + fn from(value: DataFusionError) -> Self { + CometError::DataFusion { + msg: value.message().to_string(), + source: value, + } + } +} impl From for DataFusionError { fn from(value: CometError) -> Self { match value { - CometError::DataFusion { source } => source, + CometError::DataFusion { msg: _, source } => source, _ => DataFusionError::Execution(value.to_string()), } } From 98bfa15ae5f7fd988de0606227ccb4b51d69c40f Mon Sep 17 00:00:00 2001 From: Oleks V Date: Mon, 10 Jun 2024 10:51:23 -0700 Subject: [PATCH 4/4] Update core/src/errors.rs Co-authored-by: Andy Grove --- core/src/errors.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/core/src/errors.rs b/core/src/errors.rs index 05b18f0ef..493880c3e 100644 --- a/core/src/errors.rs +++ b/core/src/errors.rs @@ -185,6 +185,7 @@ impl convert::From> for CometError { } } } + impl From for CometError { fn from(value: DataFusionError) -> Self { CometError::DataFusion { @@ -193,6 +194,7 @@ impl From for CometError { } } } + impl From for DataFusionError { fn from(value: CometError) -> Self { match value {