You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "myandpr (via GitHub)" <gi...@apache.org> on 2026/03/18 15:53:37 UTC
[PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
myandpr opened a new pull request, #21032:
URL: https://github.com/apache/datafusion/pull/21032
## Summary
Fixes #20501.
This PR changes `TypeSignature::OneOf` failures in the UDF coercion path from an `Internal error` into a normal planning error.
Before this change, unsupported calls such as `SUM(Boolean)` could produce user-visible errors with:
- `Internal error: Function 'sum' failed to match any signature`
- internal `TypeSignatureClass::...` details
- repeated branch-specific failure output
After this change, DataFusion returns a normal planning error, and the existing planner diagnostics continue to show the function call and candidate signatures.
## What changed
In `datafusion/expr/src/type_coercion/functions.rs`:
- kept the existing success path when any `OneOf` branch matches
- changed the all-failed `OneOf` case to return:
- `Function '...' failed to match any signature`
- as a planning error rather than an internal error
This keeps the fix narrowly scoped to `TypeSignature::OneOf` error propagation.
## Why
`#20501` reports that after `#18769`, unsupported aggregate calls such as `SUM(Boolean)` started producing misleading internal errors.
The direct cause is that `TypeSignature::OneOf` aggregated branch failures into an `Internal error`, which then became visible during planning.
This PR fixes that specific problem by making the failure user-facing and non-internal.
## Why this does not restore `Sum not supported for Boolean`
The older message came from a more function-specific path.
This PR intentionally does not try to infer a single "best" branch error from `OneOf`, because that can be misleading for overloaded functions with:
- different arities, such as `substr(...)` or `nth_value(...)`
- multiple valid type families, such as `sum(...)`
In those cases, returning a branch-specific error can report the wrong arity or an overly specific type requirement that only reflects one overload.
So this PR prefers a neutral and accurate planning error over a more specific but potentially incorrect branch-specific message.
## Tests
Added unit tests covering:
- `OneOf` mismatches no longer returning `Internal error`
- `OneOf` arity mismatches also returning a planning error rather than an internal error
Updated sqllogictests for affected user-visible errors, including:
- `sum(Boolean)`
- `nth_value(...)`
- `substr(...)`
- `generate_series(...)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2958724468
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -325,24 +325,20 @@ fn get_valid_types_with_udf<F: UDFCoercionExt>(
},
TypeSignature::OneOf(signatures) => {
let mut res = vec![];
- let mut errors = vec![];
for sig in signatures {
- match get_valid_types_with_udf(sig, current_types, func) {
- Ok(valid_types) => {
- res.extend(valid_types);
- }
- Err(e) => {
Review Comment:
Thanks @martin-g , updated this to defer non-`Plan` errors until all `OneOf` branches have been checked, so valid matches still succeed and non-`Plan` errors are only surfaced when every branch fails.
Would you mind taking another look?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2958136385
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1219,56 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ Signature::coercible(
+ vec![Coercion::new_exact(TypeSignatureClass::Decimal)],
+ Volatility::Immutable,
+ )
+ .type_signature
+ .clone(),
+ Signature::coercible(
+ vec![Coercion::new_exact(TypeSignatureClass::Duration)],
+ Volatility::Immutable,
+ )
+ .type_signature
+ .clone(),
Review Comment:
```suggestion
TypeSignature::Coercible(vec![Coercion::new_exact(TypeSignatureClass::Decimal)]),
TypeSignature::Coercible(vec![Coercion::new_exact(TypeSignatureClass::Duration)]),
```
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -325,24 +325,20 @@ fn get_valid_types_with_udf<F: UDFCoercionExt>(
},
TypeSignature::OneOf(signatures) => {
let mut res = vec![];
- let mut errors = vec![];
for sig in signatures {
- match get_valid_types_with_udf(sig, current_types, func) {
- Ok(valid_types) => {
- res.extend(valid_types);
- }
- Err(e) => {
Review Comment:
This assumes that all errors are mismatch errors.
IMO only planning errors could be ignored.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2986595893
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
Review Comment:
updated
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ assert!(!err.contains("TypeSignatureClass"));
+ }
+
+ #[test]
+ fn test_one_of_uses_generic_plan_error_for_arity_mismatch() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature = Signature::one_of(
+ vec![TypeSignature::Any(2), TypeSignature::Any(3)],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
Review Comment:
updated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "Jefffrey (via GitHub)" <gi...@apache.org>.
Jefffrey commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4257490235
I feel there is quite a lot going on with this PR now; is it possible to break down into smaller changes that are easier to review? Or are the changes too intertwined to make this possible?
Also thanks for providing the table comparison, however I find it a bit difficult to read in the table format. Maybe it's better to just lay it out via code blocks one after another since the error messages are quite long so the formatting inside tables makes it hard to read.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4714234674
Thank you for your contribution. Unfortunately, this pull request is stale because it has been open 60 days with no activity. Please remove the stale label or comment or this will be closed in 7 days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2986600430
##########
datafusion/sqllogictest/test_files/spark/math/hex.slt:
##########
@@ -56,7 +56,7 @@ SELECT hex(column1) FROM VALUES (arrow_cast('hello', 'LargeBinary')), (NULL), (a
NULL
776F726C64
-statement error Function 'hex' expects 1 arguments but received 2
+statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
Review Comment:
Hi @martin-g I agree the previous `hex(1, 2)` error was more actionable.
The current change intentionally fixes the `Internal error` path in `TypeSignature::OneOf`, but it does make some overload failures fall back to a more generic planning error. I avoided surfacing a branch-specific `OneOf` error directly because that turned out to be incorrect in other cases, for example mixed-arity overloads like `nth_value(c5, 2, 3)` or mixed-type overloads like `sum(bool_col)`.
Do you think we should try to preserve more actionable planner diagnostics for cases like this in the current PR, or handle that in a follow-up by choosing the best `OneOf` planning error?
Let me know if you have any preference. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2986662849
##########
datafusion/sqllogictest/test_files/spark/math/hex.slt:
##########
@@ -56,7 +56,7 @@ SELECT hex(column1) FROM VALUES (arrow_cast('hello', 'LargeBinary')), (NULL), (a
NULL
776F726C64
-statement error Function 'hex' expects 1 arguments but received 2
+statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
Review Comment:
I do understand the reason for this PR!
Let's see what others think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2986600430
##########
datafusion/sqllogictest/test_files/spark/math/hex.slt:
##########
@@ -56,7 +56,7 @@ SELECT hex(column1) FROM VALUES (arrow_cast('hello', 'LargeBinary')), (NULL), (a
NULL
776F726C64
-statement error Function 'hex' expects 1 arguments but received 2
+statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
Review Comment:
I agree the previous `hex(1, 2)` error was more actionable.
The current change intentionally fixes the `Internal error` path in `TypeSignature::OneOf`, but it does make some overload failures fall back to a more generic planning error. I avoided surfacing a branch-specific `OneOf` error directly because that turned out to be incorrect in other cases, for example mixed-arity overloads like `nth_value(c5, 2, 3)` or mixed-type overloads like `sum(bool_col)`.
Do you think we should try to preserve more actionable planner diagnostics for cases like this in the current PR, or handle that in a follow-up by choosing the best `OneOf` planning error?
Let me know if you have any preference.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2984123499
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ assert!(!err.contains("TypeSignatureClass"));
+ }
+
+ #[test]
+ fn test_one_of_uses_generic_plan_error_for_arity_mismatch() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature = Signature::one_of(
+ vec![TypeSignature::Any(2), TypeSignature::Any(3)],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ }
+
+ struct AlwaysExecErrUdf(Signature);
+
+ impl UDFCoercionExt for AlwaysExecErrUdf {
+ fn name(&self) -> &str {
+ "test"
+ }
+
+ fn signature(&self) -> &Signature {
+ &self.0
+ }
+
+ fn coerce_types(&self, _arg_types: &[DataType]) -> Result<Vec<DataType>> {
+ exec_err!("boom")
+ }
+ }
+
+ #[test]
+ fn test_one_of_propagates_non_plan_errors() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature =
+ Signature::one_of(vec![TypeSignature::UserDefined], Volatility::Immutable);
+
+ let err =
+ fields_with_udf(¤t_fields, &AlwaysExecErrUdf(signature)).unwrap_err();
+
Review Comment:
Here you can assert that `err` is `DataFusionError::Exec`
##########
datafusion/sqllogictest/test_files/spark/math/hex.slt:
##########
@@ -56,7 +56,7 @@ SELECT hex(column1) FROM VALUES (arrow_cast('hello', 'LargeBinary')), (NULL), (a
NULL
776F726C64
-statement error Function 'hex' expects 1 arguments but received 2
+statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
Review Comment:
IMO the previous error was more actionable by the user.
Now the user will need to consult with the documentation of `hex` and compare it against the way (s)he tries to use it.
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ assert!(!err.contains("TypeSignatureClass"));
+ }
+
+ #[test]
+ fn test_one_of_uses_generic_plan_error_for_arity_mismatch() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature = Signature::one_of(
+ vec![TypeSignature::Any(2), TypeSignature::Any(3)],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
Review Comment:
Same here.
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
Review Comment:
You can assert that the Err type is DataFusionError::Plan before calling `.to_string()` on it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2986597589
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1227,96 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Decimal,
+ )]),
+ TypeSignature::Coercible(vec![Coercion::new_exact(
+ TypeSignatureClass::Duration,
+ )]),
+ ],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ assert!(!err.contains("TypeSignatureClass"));
+ }
+
+ #[test]
+ fn test_one_of_uses_generic_plan_error_for_arity_mismatch() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature = Signature::one_of(
+ vec![TypeSignature::Any(2), TypeSignature::Any(3)],
+ Volatility::Immutable,
+ );
+
+ let err = fields_with_udf(¤t_fields, &MockUdf(signature)).unwrap_err();
+ let err = err.to_string();
+
+ assert!(err.starts_with(
+ "Error during planning: Function 'test' failed to match any signature"
+ ));
+ assert!(!err.contains("Internal error"));
+ }
+
+ struct AlwaysExecErrUdf(Signature);
+
+ impl UDFCoercionExt for AlwaysExecErrUdf {
+ fn name(&self) -> &str {
+ "test"
+ }
+
+ fn signature(&self) -> &Signature {
+ &self.0
+ }
+
+ fn coerce_types(&self, _arg_types: &[DataType]) -> Result<Vec<DataType>> {
+ exec_err!("boom")
+ }
+ }
+
+ #[test]
+ fn test_one_of_propagates_non_plan_errors() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Int32, true))];
+ let signature =
+ Signature::one_of(vec![TypeSignature::UserDefined], Volatility::Immutable);
+
+ let err =
+ fields_with_udf(¤t_fields, &AlwaysExecErrUdf(signature)).unwrap_err();
+
Review Comment:
updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #21032: fix: avoid internal errors for OneOf signature mismatches
URL: https://github.com/apache/datafusion/pull/21032
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2958714643
##########
datafusion/expr/src/type_coercion/functions.rs:
##########
@@ -1223,6 +1219,56 @@ mod tests {
Ok(())
}
+ #[test]
+ fn test_one_of_uses_generic_plan_error_instead_of_internal_error() {
+ let current_fields = vec![Arc::new(Field::new("t", DataType::Boolean, true))];
+ let signature = Signature::one_of(
+ vec![
+ Signature::coercible(
+ vec![Coercion::new_exact(TypeSignatureClass::Decimal)],
+ Volatility::Immutable,
+ )
+ .type_signature
+ .clone(),
+ Signature::coercible(
+ vec![Coercion::new_exact(TypeSignatureClass::Duration)],
+ Volatility::Immutable,
+ )
+ .type_signature
+ .clone(),
Review Comment:
updated to construct the `TypeSignature` directly in the test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4090052232
The current CI failures are due to an outdated `clickbench.slt` expectation rather than the `OneOf` changes in this PR.
The failing `EXPLAIN` for `COUNT(DISTINCT ...)` now gets optimized to `ProjectionExec + PlaceholderRowExec` because exact NDV can be derived from Parquet metadata.
That was fixed in [#21050](https://github.com/apache/datafusion/pull/21050).
I have rebased this branch onto the latest `main`, so the updated expectation is now included here as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4172741676
@alamb @Jefffrey sorry for the delayed reply. I spent some time digging into this further and also looked through the discussion in #20070.
I updated the implementation to keep the `OneOf` fix in the shared coercion path, while restoring more actionable diagnostics where they can be determined reliably.
The current approach is:
- keep the `OneOf` logic in the shared coercion path rather than special-casing individual functions there
- preserve concrete `arity` errors instead of falling back to a generic mismatch
- preserve concrete type errors for unique matching branches
- preserve hinted errors such as the `Binary -> String` cast hint
- combine same-arity `Coercible` mismatches into a concrete error instead of falling back to a generic one
- keep function-specific semantic diagnostics only where the generic `OneOf` logic cannot express the intended error well, such as `sum(Boolean)` / `avg(Boolean)`
- narrow the public `signature()` for `first_value` / `last_value` / `nth_value` so validation, candidate signatures, and introspection all reflect the real public contract
I also put together a few representative before/after examples from actual runs on `origin/main` vs this branch:
| Case | Why this case | SQL | Before (`origin/main`) | After (current branch) |
|---|---|---|---|---|
| hex_arity | arity mismatch on Spark function mentioned in review | `select hex(1, 2);` | DataFusion error: Error during planning: Internal error: Function 'hex' failed to match any signature, errors: Error during planning: Function 'hex' expects 1 arguments but received 2,Error during planning: Function 'hex' expects 1 arguments but received 2,Error during planning: Function 'hex' expects 1 arguments but received 2. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'hex(Int64, Int64)'. You might need to add explicit type casts. Candidate functions: hex(Int64) hex(String) hex(Binary) | DataFusion error: Error during planning: Function 'hex' expects 1 arguments but received 2. No function matches the given name and argument types 'hex(Int64, Int64)'. You might need to add explicit type casts. Candid
ate functions: hex(Int64) hex(String) hex(Binary) |
| substr_type | unique same-arity type mismatch | `SELECT substr(1, 3);` | Error: Error during planning: Internal error: Function 'substr' failed to match any signature, errors: Error during planning: Function 'substr' requires String, but received Int64 (DataType: Int64).,Error during planning: Function 'substr' expects 3 arguments but received 2. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'substr(Int64, Int64)'. You might need to add explicit type casts. Candidate functions: substr(str: String, start_pos: Int64) substr(str: String, start_pos: Int64, length: Int64) | Error: Error during planning: Function 'substr' requires String, but received Int64 (DataType: Int64).. No function matches the given name and argument types 'substr(Int64, Int64)'. You might need to add explicit type casts. Can
didate functions: substr(str: String, start_pos: Int64) substr(str: String, start_pos: Int64, length: Int64) |
| substr_binary_hint | hinted type mismatch | `SELECT substr(arrow_cast('foo', 'Binary'), 1);` | Error: Error during planning: Internal error: Function 'substr' failed to match any signature, errors: Error during planning: Function 'substr' requires String, but received Binary (DataType: Binary). Hint: Binary types are not automatically coerced to String. Use CAST(column AS VARCHAR) to convert Binary data to String.,Error during planning: Function 'substr' expects 3 arguments but received 2. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'substr(Binary, Int64)'. You might need to add explicit type casts. Candidate functions: substr(str: String, start_pos: Int64) substr(str: String, start_pos: Int64, length: Int64) | Error: Error during planning: Function 'substr' requires String, but received Bin
ary (DataType: Binary). Hint: Binary types are not automatically coerced to String. Use CAST(column AS VARCHAR) to convert Binary data to String.. No function matches the given name and argument types 'substr(Binary, Int64)'. You might need to add explicit type casts. Candidate functions: substr(str: String, start_pos: Int64) substr(str: String, start_pos: Int64, length: Int64) |
| sum_bool | aggregate semantic diagnostic | `SELECT sum(bool_col) FROM (VALUES (true), (false), (null)) AS t(bool_col);` | Error: Error during planning: Internal error: Function 'sum' failed to match any signature, errors: Error during planning: Function 'sum' requires Decimal, but received Boolean (DataType: Boolean).,Error during planning: Function 'sum' requires UInt64, but received Boolean (DataType: Boolean).,Error during planning: Function 'sum' requires Int64, but received Boolean (DataType: Boolean).,Error during planning: Function 'sum' requires Float64, but received Boolean (DataType: Boolean).,Error during planning: Function 'sum' requires Duration, but received Boolean (DataType: Boolean).. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'sum(Boolean)'. You might need to add explicit
type casts. Candidate functions: sum(Decimal) sum(UInt64) sum(Int64) sum(Float64) sum(Duration) | Error: Error during planning: Sum not supported for Boolean. No function matches the given name and argument types 'sum(Boolean)'. You might need to add explicit type casts. Candidate functions: sum(Decimal) sum(UInt64) sum(Int64) sum(Float64) sum(Duration) |
| nth_value_arity | window function arity contract | `SELECT nth_value(c1, 2, 3) OVER (ORDER BY c1) FROM (VALUES (1), (2), (3)) AS t(c1);` | Error: Error during planning: Internal error: Function 'nth_value' failed to match any signature, errors: Error during planning: The function 'nth_value' expected zero argument but received 3,Error during planning: The function 'nth_value' expected 1 arguments but received 3,Error during planning: The function 'nth_value' expected 2 arguments but received 3. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'nth_value(Int64, Int64, Int64)'. You might need to add explicit type casts. Candidate functions: nth_value(NullAry()) nth_value(Any) nth_value(Any, Any) | Error: Error during planning: The function 'nth_value' expected 2 arguments but received 3. No functio
n matches the given name and argument types 'nth_value(Int64, Int64, Int64)'. You might need to add explicit type casts. Candidate functions: nth_value(Any, Any) |
| log_same_arity | ambiguous same-arity overload | `SELECT log(1, '');` | Error: Error during planning: Internal error: Function 'log' failed to match any signature, errors: Error during planning: Function 'log' expects 1 arguments but received 2,Error during planning: Function 'log' expects 1 arguments but received 2,Error during planning: Function 'log' requires Decimal, but received String (DataType: Utf8).,Error during planning: Function 'log' requires Float, but received String (DataType: Utf8).. This issue was likely caused by a bug in DataFusion's code. Please help us to resolve this by filing a bug report in our issue tracker: https://github.com/apache/datafusion/issues. No function matches the given name and argument types 'log(Int64, Utf8)'. You might need to add explicit type casts. Candidate functions: log(Decimal) log(Float) log(Float, Decimal) log(Float, Float) | Error: Error during planning: Function 'log' requires one of Decimal, Float, but received String (DataTyp
e: Utf8).. No function matches the given name and argument types 'log(Int64, Utf8)'. You might need to add explicit type casts. Candidate functions: log(Decimal) log(Float) log(Float, Decimal) log(Float, Float) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4124927056
Hey @timsaucer @alamb, would appreciate another look at this PR when you have a chance. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4120275011
Hi @martin-g, just a gentle ping on this PR when you have a chance. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#discussion_r2990499878
##########
datafusion/sqllogictest/test_files/spark/math/hex.slt:
##########
@@ -56,7 +56,7 @@ SELECT hex(column1) FROM VALUES (arrow_cast('hello', 'LargeBinary')), (NULL), (a
NULL
776F726C64
-statement error Function 'hex' expects 1 arguments but received 2
+statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
Review Comment:
> Do you think we should try to preserve more actionable planner diagnostics for cases like this in the current PR, or handle that in a follow-up by choosing the best OneOf planning error?
It would be nice to avoid a regression in error messages, even if temporarily
How hard is it to get the old behavior back?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: avoid internal errors for OneOf signature mismatches [datafusion]
Posted by "myandpr (via GitHub)" <gi...@apache.org>.
myandpr commented on PR #21032:
URL: https://github.com/apache/datafusion/pull/21032#issuecomment-4089725514
fixed ci
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org