Skip to content

Replace usages of internal_err with exec_err where appropriate #9241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 27, 2024
25 changes: 13 additions & 12 deletions datafusion/physical-expr/src/aggregate/build_in.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@

use std::sync::Arc;

use arrow::datatypes::Schema;

use datafusion_common::{exec_err, not_impl_err, DataFusionError, Result};
use datafusion_expr::AggregateFunction;

use crate::aggregate::regr::RegrType;
use crate::expressions::{self, Literal};
use crate::{AggregateExpr, PhysicalExpr, PhysicalSortExpr};

use arrow::datatypes::Schema;
use datafusion_common::{internal_err, not_impl_err, DataFusionError, Result};
use datafusion_expr::AggregateFunction;

/// Create a physical aggregation expression.
/// This function errors when `input_phy_exprs`' can't be coerced to a valid argument type of the aggregation function.
pub fn create_aggregate_expr(
Expand Down Expand Up @@ -379,9 +380,7 @@ pub fn create_aggregate_expr(
.downcast_ref::<Literal>()
.map(|literal| literal.value())
else {
return internal_err!(
"Second argument of NTH_VALUE needs to be a literal"
);
return exec_err!("Second argument of NTH_VALUE needs to be a literal");
};
let nullable = expr.nullable(input_schema)?;
Arc::new(expressions::NthValueAgg::new(
Expand Down Expand Up @@ -415,17 +414,19 @@ pub fn create_aggregate_expr(

#[cfg(test)]
mod tests {
use super::*;
use arrow::datatypes::{DataType, Field};

use datafusion_common::{plan_err, ScalarValue};
use datafusion_expr::type_coercion::aggregates::NUMERICS;
use datafusion_expr::{type_coercion, Signature};

use crate::expressions::{
try_cast, ApproxDistinct, ApproxMedian, ApproxPercentileCont, ArrayAgg, Avg,
BitAnd, BitOr, BitXor, BoolAnd, BoolOr, Correlation, Count, Covariance,
DistinctArrayAgg, DistinctCount, Max, Min, Stddev, Sum, Variance,
};

use arrow::datatypes::{DataType, Field};
use datafusion_common::{plan_err, ScalarValue};
use datafusion_expr::type_coercion::aggregates::NUMERICS;
use datafusion_expr::{type_coercion, Signature};
use super::*;

#[test]
fn test_count_arragg_approx_expr() -> Result<()> {
Expand Down
56 changes: 34 additions & 22 deletions datafusion/physical-expr/src/array_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ pub fn array_element(args: &[ArrayRef]) -> Result<ArrayRef> {
let indexes = as_int64_array(&args[1])?;
general_array_element::<i64>(array, indexes)
}
_ => exec_err!(
_ => not_impl_err!(
"array_element does not support type: {:?}",
args[0].data_type()
),
Expand Down Expand Up @@ -513,7 +513,7 @@ pub fn array_except(args: &[ArrayRef]) -> Result<ArrayRef> {
Ok(Arc::new(result))
}
(dt1, dt2) => {
internal_err!("array_except got unexpected types: {dt1:?} and {dt2:?}")
not_impl_err!("array_except got unexpected types: {dt1:?} and {dt2:?}")
}
}
}
Expand Down Expand Up @@ -561,7 +561,7 @@ pub fn array_slice(args: &[ArrayRef]) -> Result<ArrayRef> {
let to_array = as_int64_array(&args[2])?;
general_array_slice::<i64>(array, from_array, to_array, stride)
}
_ => exec_err!("array_slice does not support type: {:?}", array_data_type),
_ => not_impl_err!("array_slice does not support type: {:?}", array_data_type),
}
}

Expand Down Expand Up @@ -791,7 +791,7 @@ pub fn array_pop_front(args: &[ArrayRef]) -> Result<ArrayRef> {
let array = as_large_list_array(&args[0])?;
general_pop_front_list::<i64>(array)
}
_ => exec_err!(
_ => not_impl_err!(
"array_pop_front does not support type: {:?}",
array_data_type
),
Expand All @@ -814,7 +814,7 @@ pub fn array_pop_back(args: &[ArrayRef]) -> Result<ArrayRef> {
let array = as_large_list_array(&args[0])?;
general_pop_back_list::<i64>(array)
}
_ => exec_err!(
_ => not_impl_err!(
"array_pop_back does not support type: {:?}",
array_data_type
),
Expand Down Expand Up @@ -1226,7 +1226,7 @@ pub fn array_empty(args: &[ArrayRef]) -> Result<ArrayRef> {
match array_type {
DataType::List(_) => array_empty_dispatch::<i32>(&args[0]),
DataType::LargeList(_) => array_empty_dispatch::<i64>(&args[0]),
_ => exec_err!("array_empty does not support type '{array_type:?}'."),
_ => not_impl_err!("array_empty does not support type '{array_type:?}'."),
}
}

Expand Down Expand Up @@ -1393,7 +1393,9 @@ pub fn array_position(args: &[ArrayRef]) -> Result<ArrayRef> {
match &args[0].data_type() {
DataType::List(_) => general_position_dispatch::<i32>(args),
DataType::LargeList(_) => general_position_dispatch::<i64>(args),
array_type => exec_err!("array_position does not support type '{array_type:?}'."),
array_type => {
not_impl_err!("array_position does not support type '{array_type:?}'.")
}
}
}
fn general_position_dispatch<O: OffsetSizeTrait>(args: &[ArrayRef]) -> Result<ArrayRef> {
Expand Down Expand Up @@ -1479,7 +1481,7 @@ pub fn array_positions(args: &[ArrayRef]) -> Result<ArrayRef> {
general_positions::<i64>(arr, element)
}
array_type => {
exec_err!("array_positions does not support type '{array_type:?}'.")
not_impl_err!("array_positions does not support type '{array_type:?}'.")
}
}
}
Expand Down Expand Up @@ -1620,7 +1622,7 @@ fn array_remove_internal(
general_remove::<i64>(list_array, element_array, arr_n)
}
array_type => {
exec_err!("array_remove_all does not support type '{array_type:?}'.")
not_impl_err!("array_remove_all does not support type '{array_type:?}'.")
}
}
}
Expand Down Expand Up @@ -1780,7 +1782,9 @@ pub fn array_replace(args: &[ArrayRef]) -> Result<ArrayRef> {
let list_array = array.as_list::<i64>();
general_replace::<i64>(list_array, &args[1], &args[2], arr_n)
}
array_type => exec_err!("array_replace does not support type '{array_type:?}'."),
array_type => {
not_impl_err!("array_replace does not support type '{array_type:?}'.")
}
}
}

Expand All @@ -1802,7 +1806,7 @@ pub fn array_replace_n(args: &[ArrayRef]) -> Result<ArrayRef> {
general_replace::<i64>(list_array, &args[1], &args[2], arr_n)
}
array_type => {
exec_err!("array_replace_n does not support type '{array_type:?}'.")
not_impl_err!("array_replace_n does not support type '{array_type:?}'.")
}
}
}
Expand All @@ -1825,7 +1829,7 @@ pub fn array_replace_all(args: &[ArrayRef]) -> Result<ArrayRef> {
general_replace::<i64>(list_array, &args[1], &args[2], arr_n)
}
array_type => {
exec_err!("array_replace_all does not support type '{array_type:?}'.")
not_impl_err!("array_replace_all does not support type '{array_type:?}'.")
}
}
}
Expand Down Expand Up @@ -2013,7 +2017,7 @@ pub fn cardinality(args: &[ArrayRef]) -> Result<ArrayRef> {
generic_list_cardinality::<i64>(list_array)
}
other => {
exec_err!("cardinality does not support type '{:?}'", other)
not_impl_err!("cardinality does not support type '{:?}'", other)
}
}
}
Expand Down Expand Up @@ -2095,7 +2099,7 @@ pub fn flatten(args: &[ArrayRef]) -> Result<ArrayRef> {
}
DataType::Null => Ok(args[0].clone()),
_ => {
exec_err!("flatten does not support type '{array_type:?}'")
not_impl_err!("flatten does not support type '{array_type:?}'")
}
}

Expand Down Expand Up @@ -2129,7 +2133,9 @@ pub fn array_length(args: &[ArrayRef]) -> Result<ArrayRef> {
match &args[0].data_type() {
DataType::List(_) => array_length_dispatch::<i32>(args),
DataType::LargeList(_) => array_length_dispatch::<i64>(args),
array_type => exec_err!("array_length does not support type '{array_type:?}'"),
array_type => {
not_impl_err!("array_length does not support type '{array_type:?}'")
}
}
}

Expand All @@ -2155,7 +2161,7 @@ pub fn array_dims(args: &[ArrayRef]) -> Result<ArrayRef> {
.collect::<Result<Vec<_>>>()?
}
array_type => {
return exec_err!("array_dims does not support type '{array_type:?}'");
return not_impl_err!("array_dims does not support type '{array_type:?}'");
}
};

Expand Down Expand Up @@ -2285,7 +2291,7 @@ pub fn array_has(args: &[ArrayRef]) -> Result<ArrayRef> {
DataType::LargeList(_) => {
general_array_has_dispatch::<i64>(&args[0], &args[1], ComparisonType::Single)
}
_ => exec_err!("array_has does not support type '{array_type:?}'."),
_ => not_impl_err!("array_has does not support type '{array_type:?}'."),
}
}

Expand All @@ -2304,7 +2310,7 @@ pub fn array_has_any(args: &[ArrayRef]) -> Result<ArrayRef> {
DataType::LargeList(_) => {
general_array_has_dispatch::<i64>(&args[0], &args[1], ComparisonType::Any)
}
_ => exec_err!("array_has_any does not support type '{array_type:?}'."),
_ => not_impl_err!("array_has_any does not support type '{array_type:?}'."),
}
}

Expand All @@ -2323,7 +2329,7 @@ pub fn array_has_all(args: &[ArrayRef]) -> Result<ArrayRef> {
DataType::LargeList(_) => {
general_array_has_dispatch::<i64>(&args[0], &args[1], ComparisonType::All)
}
_ => exec_err!("array_has_all does not support type '{array_type:?}'."),
_ => not_impl_err!("array_has_all does not support type '{array_type:?}'."),
}
}

Expand Down Expand Up @@ -2474,7 +2480,9 @@ pub fn array_distinct(args: &[ArrayRef]) -> Result<ArrayRef> {
let array = as_large_list_array(&args[0])?;
general_array_distinct(array, field)
}
array_type => exec_err!("array_distinct does not support type '{array_type:?}'"),
array_type => {
not_impl_err!("array_distinct does not support type '{array_type:?}'")
}
}
}

Expand All @@ -2500,7 +2508,9 @@ pub fn array_resize(arg: &[ArrayRef]) -> Result<ArrayRef> {
let array = as_large_list_array(&arg[0])?;
general_list_resize::<i64>(array, new_len, field, new_element)
}
array_type => exec_err!("array_resize does not support type '{array_type:?}'."),
array_type => {
not_impl_err!("array_resize does not support type '{array_type:?}'.")
}
}
}

Expand Down Expand Up @@ -2588,7 +2598,9 @@ pub fn array_reverse(arg: &[ArrayRef]) -> Result<ArrayRef> {
general_array_reverse::<i64>(array, field)
}
DataType::Null => Ok(arg[0].clone()),
array_type => exec_err!("array_reverse does not support type '{array_type:?}'."),
array_type => {
not_impl_err!("array_reverse does not support type '{array_type:?}'.")
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions datafusion/physical-expr/src/conditional_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ use arrow::array::{new_null_array, Array, BooleanArray};
use arrow::compute::kernels::zip::zip;
use arrow::compute::{and, is_not_null, is_null};

use datafusion_common::{internal_err, DataFusionError, Result};
use datafusion_common::{exec_err, DataFusionError, Result};
use datafusion_expr::ColumnarValue;

/// coalesce evaluates to the first value which is not NULL
pub fn coalesce(args: &[ColumnarValue]) -> Result<ColumnarValue> {
// do not accept 0 arguments.
if args.is_empty() {
return internal_err!(
return exec_err!(
"coalesce was called with {} arguments. It requires at least 1.",
args.len()
);
Expand Down
18 changes: 9 additions & 9 deletions datafusion/physical-expr/src/crypto_expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ use arrow::{
};
use blake2::{Blake2b512, Blake2s256, Digest};
use blake3::Hasher as Blake3;
use datafusion_common::ScalarValue;
use datafusion_common::{
cast::{as_binary_array, as_generic_binary_array, as_generic_string_array},
plan_err,
not_impl_err, plan_err,
};
use datafusion_common::{exec_err, ScalarValue};
use datafusion_common::{internal_err, DataFusionError, Result};
use datafusion_expr::ColumnarValue;
use md5::Md5;
Expand Down Expand Up @@ -66,7 +66,7 @@ fn digest_process(
DataType::LargeBinary => {
digest_algorithm.digest_binary_array::<i64>(a.as_ref())
}
other => internal_err!(
other => not_impl_err!(
"Unsupported data type {other:?} for function {digest_algorithm}"
),
},
Expand All @@ -77,7 +77,7 @@ fn digest_process(
}
ScalarValue::Binary(a) | ScalarValue::LargeBinary(a) => Ok(digest_algorithm
.digest_scalar(a.as_ref().map(|v: &Vec<u8>| v.as_slice()))),
other => internal_err!(
other => not_impl_err!(
"Unsupported data type {other:?} for function {digest_algorithm}"
),
},
Expand Down Expand Up @@ -238,7 +238,7 @@ macro_rules! define_digest_function {
#[doc = $DOC]
pub fn $NAME(args: &[ColumnarValue]) -> Result<ColumnarValue> {
if args.len() != 1 {
return internal_err!(
return exec_err!(
"{:?} args were supplied but {} takes exactly one argument",
args.len(),
DigestAlgorithm::$METHOD.to_string()
Expand All @@ -264,7 +264,7 @@ fn hex_encode<T: AsRef<[u8]>>(data: T) -> String {
/// computes md5 hash digest of the given input
pub fn md5(args: &[ColumnarValue]) -> Result<ColumnarValue> {
if args.len() != 1 {
return internal_err!(
return exec_err!(
"{:?} args were supplied but {} takes exactly one argument",
args.len(),
DigestAlgorithm::Md5
Expand All @@ -284,7 +284,7 @@ pub fn md5(args: &[ColumnarValue]) -> Result<ColumnarValue> {
ColumnarValue::Scalar(ScalarValue::Binary(opt)) => {
ColumnarValue::Scalar(ScalarValue::Utf8(opt.map(hex_encode::<_>)))
}
_ => return internal_err!("Impossibly got invalid results from digest"),
_ => return exec_err!("Impossibly got invalid results from digest"),
})
}

Expand Down Expand Up @@ -329,7 +329,7 @@ define_digest_function!(
/// Standard algorithms are md5, sha1, sha224, sha256, sha384 and sha512.
pub fn digest(args: &[ColumnarValue]) -> Result<ColumnarValue> {
if args.len() != 2 {
return internal_err!(
return exec_err!(
"{:?} args were supplied but digest takes exactly two arguments",
args.len()
);
Expand All @@ -339,7 +339,7 @@ pub fn digest(args: &[ColumnarValue]) -> Result<ColumnarValue> {
ScalarValue::Utf8(Some(method)) | ScalarValue::LargeUtf8(Some(method)) => {
method.parse::<DigestAlgorithm>()
}
other => internal_err!("Unsupported data type {other:?} for function digest"),
other => not_impl_err!("Unsupported data type {other:?} for function digest"),
},
ColumnarValue::Array(_) => {
internal_err!("Digest using dynamically decided method is not yet supported")
Expand Down
Loading