Skip to content

fix: avoid internal errors for OneOf signature mismatches#21032

Open
myandpr wants to merge 3 commits intoapache:mainfrom
myandpr:20501-oneof-error-message
Open

fix: avoid internal errors for OneOf signature mismatches#21032
myandpr wants to merge 3 commits intoapache:mainfrom
myandpr:20501-oneof-error-message

Conversation

@myandpr
Copy link

@myandpr myandpr commented Mar 18, 2026

Which issue does this PR close?

Rationale for this change

After #18769, unsupported calls such as SUM(Boolean) started producing misleading internal errors.

The immediate problem is that TypeSignature::OneOf aggregates branch failures into an Internal error, which then becomes visible during planning together with internal TypeSignatureClass::... details.

This PR is intentionally narrower than #20070 and only fixes that TypeSignature::OneOf internal-error path.

What changes are included in this PR?

In datafusion/expr/src/type_coercion/functions.rs:

  • keep the existing success path when any OneOf branch matches
  • change the all-failed OneOf case to return Function '...' failed to match any signature as a planning error rather than an internal error

This does not restore the older Sum not supported for Boolean wording.

That older message came from a more function-specific path, and this PR intentionally does not try to infer a single branch-specific error from OneOf, since that can be misleading for overloaded functions with different arities or multiple valid type families.

In those cases, returning a branch-specific error can report the wrong arity or an overly specific type requirement that only reflects one overload.

Are these changes tested?

Yes.

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(...)

Are there any user-facing changes?

Yes.

For failed TypeSignature::OneOf function calls, DataFusion now returns a normal planning error instead of an Internal error, while continuing to show the function call and candidate signatures.

Comment on lines +1227 to +1238
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(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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(),
TypeSignature::Coercible(vec![Coercion::new_exact(TypeSignatureClass::Decimal)]),
TypeSignature::Coercible(vec![Coercion::new_exact(TypeSignatureClass::Duration)]),

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated to construct the TypeSignature directly in the test.

@github-actions github-actions bot added logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt) labels Mar 19, 2026
@myandpr myandpr requested a review from martin-g March 19, 2026 09:23
@myandpr
Copy link
Author

myandpr commented Mar 19, 2026

fixed ci

myandpr added 3 commits March 19, 2026 21:13
Signed-off-by: yaommen <myanstu@163.com>
Signed-off-by: yaommen <myanstu@163.com>
Signed-off-by: yaommen <myanstu@163.com>
@myandpr myandpr force-pushed the 20501-oneof-error-message branch from f13570a to 8866d95 Compare March 19, 2026 13:14
@myandpr
Copy link
Author

myandpr commented Mar 19, 2026

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.

I have rebased this branch onto the latest main, so the updated expectation is now included here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Type signature matching errors are overly verbose and show internal details

2 participants