You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by al...@apache.org on 2022/12/01 12:15:17 UTC
[arrow-datafusion] branch master updated: fix: do NOT convert errors to strings (#4436)
This is an automated email from the ASF dual-hosted git repository.
alamb pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow-datafusion.git
The following commit(s) were added to refs/heads/master by this push:
new ffe97e5a9 fix: do NOT convert errors to strings (#4436)
ffe97e5a9 is described below
commit ffe97e5a9459990b8167b66705b7a413f5289f35
Author: Marco Neumann <ma...@crepererum.net>
AuthorDate: Thu Dec 1 12:15:11 2022 +0000
fix: do NOT convert errors to strings (#4436)
Wrap them into proper containers instead.
Fixes #4434.
---
datafusion-cli/src/command.rs | 12 +++---------
datafusion-cli/src/object_storage.rs | 4 ++--
datafusion-cli/src/print_format.rs | 6 +++---
datafusion/core/src/datasource/listing_table_factory.rs | 6 +++---
datafusion/core/src/physical_plan/repartition.rs | 17 ++++++++++++-----
datafusion/physical-expr/src/regex_expressions.rs | 10 +++++-----
6 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/datafusion-cli/src/command.rs b/datafusion-cli/src/command.rs
index f1b6f67e3..d9704f3ba 100644
--- a/datafusion-cli/src/command.rs
+++ b/datafusion-cli/src/command.rs
@@ -59,22 +59,16 @@ impl Command {
) -> Result<()> {
let now = Instant::now();
match self {
- Self::Help => print_options
- .print_batches(&[all_commands_info()], now)
- .map_err(|e| DataFusionError::Execution(e.to_string())),
+ Self::Help => print_options.print_batches(&[all_commands_info()], now),
Self::ListTables => {
let df = ctx.sql("SHOW TABLES").await?;
let batches = df.collect().await?;
- print_options
- .print_batches(&batches, now)
- .map_err(|e| DataFusionError::Execution(e.to_string()))
+ print_options.print_batches(&batches, now)
}
Self::DescribeTable(name) => {
let df = ctx.sql(&format!("SHOW COLUMNS FROM {}", name)).await?;
let batches = df.collect().await?;
- print_options
- .print_batches(&batches, now)
- .map_err(|e| DataFusionError::Execution(e.to_string()))
+ print_options.print_batches(&batches, now)
}
Self::Include(filename) => {
if let Some(filename) = filename {
diff --git a/datafusion-cli/src/object_storage.rs b/datafusion-cli/src/object_storage.rs
index 64c48840e..177b12717 100644
--- a/datafusion-cli/src/object_storage.rs
+++ b/datafusion-cli/src/object_storage.rs
@@ -60,7 +60,7 @@ fn build_s3_object_store(url: &Url) -> Result<Arc<dyn object_store::ObjectStore>
let host = get_host_name(url)?;
match AmazonS3Builder::from_env().with_bucket_name(host).build() {
Ok(s3) => Ok(Arc::new(s3)),
- Err(err) => Err(DataFusionError::Execution(err.to_string())),
+ Err(err) => Err(DataFusionError::External(Box::new(err))),
}
}
@@ -73,7 +73,7 @@ fn build_gcs_object_store(url: &Url) -> Result<Arc<dyn object_store::ObjectStore
}
match builder.build() {
Ok(gcs) => Ok(Arc::new(gcs)),
- Err(err) => Err(DataFusionError::Execution(err.to_string())),
+ Err(err) => Err(DataFusionError::External(Box::new(err))),
}
}
diff --git a/datafusion-cli/src/print_format.rs b/datafusion-cli/src/print_format.rs
index c3eb09605..6f23efb1a 100644
--- a/datafusion-cli/src/print_format.rs
+++ b/datafusion-cli/src/print_format.rs
@@ -49,7 +49,7 @@ macro_rules! batches_to_json {
writer.write_batches($batches)?;
writer.finish()?;
}
- String::from_utf8(bytes).map_err(|e| DataFusionError::Execution(e.to_string()))?
+ String::from_utf8(bytes).map_err(|e| DataFusionError::External(Box::new(e)))?
}};
}
@@ -64,8 +64,8 @@ fn print_batches_with_sep(batches: &[RecordBatch], delimiter: u8) -> Result<Stri
writer.write(batch)?;
}
}
- let formatted = String::from_utf8(bytes)
- .map_err(|e| DataFusionError::Execution(e.to_string()))?;
+ let formatted =
+ String::from_utf8(bytes).map_err(|e| DataFusionError::External(Box::new(e)))?;
Ok(formatted)
}
diff --git a/datafusion/core/src/datasource/listing_table_factory.rs b/datafusion/core/src/datasource/listing_table_factory.rs
index dbb8fdc78..9a38e1768 100644
--- a/datafusion/core/src/datasource/listing_table_factory.rs
+++ b/datafusion/core/src/datasource/listing_table_factory.rs
@@ -103,9 +103,9 @@ impl TableProviderFactory for ListingTableFactory {
.table_partition_cols
.iter()
.map(|col| {
- schema.field_with_name(col).map_err(|arrow_err| {
- DataFusionError::Execution(arrow_err.to_string())
- })
+ schema
+ .field_with_name(col)
+ .map_err(DataFusionError::ArrowError)
})
.collect::<datafusion_common::Result<Vec<_>>>()?
.into_iter()
diff --git a/datafusion/core/src/physical_plan/repartition.rs b/datafusion/core/src/physical_plan/repartition.rs
index fdcb448d2..9492fb749 100644
--- a/datafusion/core/src/physical_plan/repartition.rs
+++ b/datafusion/core/src/physical_plan/repartition.rs
@@ -30,7 +30,7 @@ use crate::physical_plan::{
};
use arrow::array::{ArrayRef, UInt64Builder};
use arrow::datatypes::SchemaRef;
-use arrow::error::Result as ArrowResult;
+use arrow::error::{ArrowError, Result as ArrowResult};
use arrow::record_batch::RecordBatch;
use log::debug;
use tokio_stream::wrappers::UnboundedReceiverStream;
@@ -482,18 +482,25 @@ impl RepartitionExec {
match input_task.await {
// Error in joining task
Err(e) => {
+ let e = Arc::new(e);
+
for (_, tx) in txs {
- let err = DataFusionError::Execution(format!("Join Error: {}", e));
- let err = Err(err.into());
+ let err = Err(ArrowError::ExternalError(Box::new(
+ DataFusionError::Context(
+ "Join Error".to_string(),
+ Box::new(DataFusionError::External(Box::new(Arc::clone(&e)))),
+ ),
+ )));
tx.send(Some(err)).ok();
}
}
// Error from running input task
Ok(Err(e)) => {
+ let e = Arc::new(e);
+
for (_, tx) in txs {
// wrap it because need to send error to all output partitions
- let err = DataFusionError::Execution(e.to_string());
- let err = Err(err.into());
+ let err = Err(ArrowError::ExternalError(Box::new(e.clone())));
tx.send(Some(err)).ok();
}
}
diff --git a/datafusion/physical-expr/src/regex_expressions.rs b/datafusion/physical-expr/src/regex_expressions.rs
index a0e754664..bdf61d5d0 100644
--- a/datafusion/physical-expr/src/regex_expressions.rs
+++ b/datafusion/physical-expr/src/regex_expressions.rs
@@ -136,7 +136,7 @@ pub fn regexp_replace<T: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<ArrayRef>
patterns.insert(pattern.to_string(), re.clone());
Ok(re)
},
- Err(err) => Err(DataFusionError::Execution(err.to_string())),
+ Err(err) => Err(DataFusionError::External(Box::new(err))),
}
}
};
@@ -182,7 +182,7 @@ pub fn regexp_replace<T: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<ArrayRef>
patterns.insert(pattern, re.clone());
Ok(re)
},
- Err(err) => Err(DataFusionError::Execution(err.to_string())),
+ Err(err) => Err(DataFusionError::External(Box::new(err))),
}
}
};
@@ -254,8 +254,8 @@ fn _regexp_replace_static_pattern_replace<T: OffsetSizeTrait>(
None => (pattern.to_string(), 1),
};
- let re = Regex::new(&pattern)
- .map_err(|err| DataFusionError::Execution(err.to_string()))?;
+ let re =
+ Regex::new(&pattern).map_err(|err| DataFusionError::External(Box::new(err)))?;
// Replaces the posix groups in the replacement string
// with rust ones.
@@ -522,7 +522,7 @@ mod tests {
let pattern_err = re.expect_err("broken pattern should have failed");
assert_eq!(
pattern_err.to_string(),
- "Execution error: regex parse error:\n [\n ^\nerror: unclosed character class"
+ "External error: regex parse error:\n [\n ^\nerror: unclosed character class"
);
}