Skip to content

Commit

Permalink
Improve error messages for incorrect zero argument signatures (#13881)
Browse files Browse the repository at this point in the history 
* Improve error messages for incorrect zero argument signatures

* fix errors

* fix fmt
  • Loading branch information
@alamb
alamb authored Dec 23, 2024
1 parent 8fd792f commit 405b99c
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 37 deletions.
9 changes: 9 additions & 0 deletions datafusion/expr-common/src/signature.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,15 @@ impl TypeSignature {
}
}

/// Returns true if the signature currently supports or used to supported 0
/// input arguments in a previous version of DataFusion.
pub fn used_to_support_zero_arguments(&self) -> bool {
match &self {
TypeSignature::Any(num) => *num == 0,
_ => self.supports_zero_argument(),
}
}

/// get all possible types for the given `TypeSignature`
pub fn get_possible_types(&self) -> Vec<Vec<DataType>> {
match self {
Expand Down
71 changes: 34 additions & 37 deletions datafusion/expr/src/type_coercion/functions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,21 @@ pub fn data_types_with_scalar_udf(
func: &ScalarUDF,
) -> Result<Vec<DataType>> {
let signature = func.signature();
let type_signature = &signature.type_signature;

if current_types.is_empty() {
if signature.type_signature.supports_zero_argument() {
if type_signature.supports_zero_argument() {
return Ok(vec![]);
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763
return plan_err!("{} does not support zero arguments. Use TypeSignature::Nullary for zero arguments.", func.name());
} else {
return plan_err!("{} does not support zero arguments.", func.name());
}
}

let valid_types =
get_valid_types_with_scalar_udf(&signature.type_signature, current_types, func)?;
get_valid_types_with_scalar_udf(type_signature, current_types, func)?;

if valid_types
.iter()
Expand All @@ -69,12 +73,7 @@ pub fn data_types_with_scalar_udf(
return Ok(current_types.to_vec());
}

try_coerce_types(
func.name(),
valid_types,
current_types,
&signature.type_signature,
)
try_coerce_types(func.name(), valid_types, current_types, type_signature)
}

/// Performs type coercion for aggregate function arguments.
Expand All @@ -89,33 +88,29 @@ pub fn data_types_with_aggregate_udf(
func: &AggregateUDF,
) -> Result<Vec<DataType>> {
let signature = func.signature();
let type_signature = &signature.type_signature;

if current_types.is_empty() {
if signature.type_signature.supports_zero_argument() {
if type_signature.supports_zero_argument() {
return Ok(vec![]);
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763
return plan_err!("{} does not support zero arguments. Use TypeSignature::Nullary for zero arguments.", func.name());
} else {
return plan_err!("{} does not support zero arguments.", func.name());
}
}

let valid_types = get_valid_types_with_aggregate_udf(
&signature.type_signature,
current_types,
func,
)?;
let valid_types =
get_valid_types_with_aggregate_udf(type_signature, current_types, func)?;
if valid_types
.iter()
.any(|data_type| data_type == current_types)
{
return Ok(current_types.to_vec());
}

try_coerce_types(
func.name(),
valid_types,
current_types,
&signature.type_signature,
)
try_coerce_types(func.name(), valid_types, current_types, type_signature)
}

/// Performs type coercion for window function arguments.
Expand All @@ -130,30 +125,29 @@ pub fn data_types_with_window_udf(
func: &WindowUDF,
) -> Result<Vec<DataType>> {
let signature = func.signature();
let type_signature = &signature.type_signature;

if current_types.is_empty() {
if signature.type_signature.supports_zero_argument() {
if type_signature.supports_zero_argument() {
return Ok(vec![]);
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763
return plan_err!("{} does not support zero arguments. Use TypeSignature::Nullary for zero arguments.", func.name());
} else {
return plan_err!("{} does not support zero arguments.", func.name());
}
}

let valid_types =
get_valid_types_with_window_udf(&signature.type_signature, current_types, func)?;
get_valid_types_with_window_udf(type_signature, current_types, func)?;
if valid_types
.iter()
.any(|data_type| data_type == current_types)
{
return Ok(current_types.to_vec());
}

try_coerce_types(
func.name(),
valid_types,
current_types,
&signature.type_signature,
)
try_coerce_types(func.name(), valid_types, current_types, type_signature)
}

/// Performs type coercion for function arguments.
Expand All @@ -168,31 +162,34 @@ pub fn data_types(
current_types: &[DataType],
signature: &Signature,
) -> Result<Vec<DataType>> {
let type_signature = &signature.type_signature;

if current_types.is_empty() {
if signature.type_signature.supports_zero_argument() {
if type_signature.supports_zero_argument() {
return Ok(vec![]);
} else if type_signature.used_to_support_zero_arguments() {
// Special error to help during upgrade: https://github.com/apache/datafusion/issues/13763
return plan_err!(
"signature {:?} does not support zero arguments. Use TypeSignature::Nullary for zero arguments.",
type_signature
);
} else {
return plan_err!(
"signature {:?} does not support zero arguments.",
&signature.type_signature
type_signature
);
}
}

let valid_types = get_valid_types(&signature.type_signature, current_types)?;
let valid_types = get_valid_types(type_signature, current_types)?;
if valid_types
.iter()
.any(|data_type| data_type == current_types)
{
return Ok(current_types.to_vec());
}

try_coerce_types(
function_name,
valid_types,
current_types,
&signature.type_signature,
)
try_coerce_types(function_name, valid_types, current_types, type_signature)
}

fn is_well_supported_signature(type_signature: &TypeSignature) -> bool {
Expand Down

0 comments on commit 405b99c

Please sign in to comment.