21032: fix: avoid internal errors for OneOf signature mismatches

[martin-augment]

21032: To review by AI

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 616e08e9-8c91-4377-becd-e2705817eb31

📥 Commits

Reviewing files that changed from the base of the PR and between 291bcca and 8866d95.

📒 Files selected for processing (7)

• datafusion/expr/src/type_coercion/functions.rs
• datafusion/sqllogictest/test_files/array.slt
• datafusion/sqllogictest/test_files/errors.slt
• datafusion/sqllogictest/test_files/functions.slt
• datafusion/sqllogictest/test_files/named_arguments.slt
• datafusion/sqllogictest/test_files/spark/math/hex.slt
• datafusion/sqllogictest/test_files/string/string_literal.slt

---

Walkthrough

This pull request modifies error handling for function signature matching in DataFusion. The core change in datafusion/expr/src/type_coercion/functions.rs updates the get_valid_types_with_udf function to handle TypeSignature::OneOf failures differently: instead of accumulating and returning all branch errors, it now collects only the first non-Plan error and returns a generic plan error message if no branches match. Corresponding test files are updated to reflect the new error message formats, replacing detailed error messages with standardized planning error prefixes.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pr-21032-2026-03-24-20-32-28

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the error handling mechanism for functions utilizing TypeSignature::OneOf in DataFusion. The primary goal is to improve the clarity and specificity of error messages when function signatures fail to match. By differentiating between planning errors and other error types, the system now provides more actionable feedback to users, moving away from vague "Internal error" messages towards more descriptive "Planning error" messages, thereby enhancing the overall robustness and user-friendliness of error reporting.

Highlights

• Improved Error Messages: Replaced generic "Internal error" messages with more specific "Planning error" messages for TypeSignature::OneOf mismatches, providing clearer feedback on function signature failures.
• Enhanced Error Propagation: Modified the TypeSignature::OneOf error handling to correctly defer and propagate non-planning errors, ensuring that critical execution errors are not masked.
• Comprehensive Testing: Added new unit tests to thoroughly validate the refined OneOf error handling logic across various scenarios, including arity mismatches and mixed error types.
• Updated SQL Logic Tests: Adjusted sqllogictest files to reflect the new, more precise error messages for functions like generate_series, nth_value, substr, and hex.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

Code Review: fix: avoid internal errors for OneOf signature mismatches

The PR correctly solves the stated problem — TypeSignature::OneOf failures no longer surface as Internal error. The logic is sound and the primary test cases are covered. Here are some observations:

---

Medium Priority

Arity-mismatch error quality regression (hex.slt)

The hex.slt test change replaces a specific, actionable error message with a generic one:

- statement error Function 'hex' expects 1 arguments but received 2
+ statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
SELECT hex(1, 2);

The old message "Function 'hex' expects 1 arguments but received 2" directly told users what went wrong. The new message "Function 'hex' failed to match any signature" gives no actionable hint. This happens because the arity-mismatch plan_err! from get_valid_types (for TypeSignature::Any(n)) is now silently discarded and replaced with the generic fallback.

A more surgical approach could preserve the "best" planning error (e.g., the arity-mismatch message) rather than discarding all Plan errors uniformly. For functions with a OneOf containing a single fixed-arity overload, the arity error is unambiguously the right message to surface.

---

Low Priority

Loss of per-branch error detail

The old code collected all branch error strings and joined them. While it incorrectly used internal_err!, users got all failure reasons. The new code returns only "Function '...' failed to match any signature" when all branches return Plan errors. The comment in the code acknowledges this tradeoff, but for functions with many overloads, this could make debugging harder. Worth documenting or tracking as a follow-up.

Missing test: NotImplemented variant inside OneOf

The default ScalarUDFImpl::coerce_types returns not_impl_err! (DataFusionError::NotImplemented). There is no test for a OneOf that includes a TypeSignature::UserDefined branch where coerce_types is not implemented. In that case the NotImplemented error would be captured as deferred_err and propagated, which could be surprising alongside valid other branches. A test verifying the expected behavior would be useful.

errors.slt test is a good addition — the sum(bool) regression test is valuable, even if the resulting error message is still relatively opaque (pre-existing limitation).

---

Overall

The fix is correct and the core test coverage (type mismatch, arity mismatch, non-plan error propagation, success with later-branch errors) is good. The main concern is the arity-mismatch error regression in hex.slt. The rest of the comments are minor and could be addressed as follow-ups if preferred.

[augmentcode]

🤖 Augment PR Summary

Summary: This PR adjusts overload resolution for Signature::OneOf so signature mismatches don’t surface as internal errors during planning.

Changes:

• In get_valid_types_with_udf, treat per-branch Plan errors as non-fatal while searching overloads
• If no overload matches, return a generic planning error (failed to match any signature) and avoid leaking branch-specific details
• Add unit tests for the new behavior and update SQLLogicTest expectations to match the new error strings

Technical Notes: Non-planning errors (e.g. execution errors from user-defined coercion) are still propagated when no overload succeeds.

_{🤖 Was this summary useful? React with 👍 or 👎}

[augmentcode]

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

[gemini-code-assist]

Code Review

This pull request refactors the error handling for function signature mismatches, specifically within the TypeSignature::OneOf branch of the get_valid_types_with_udf function. The change aims to provide more user-friendly error messages by returning a generic planning error instead of an internal error with detailed signature failures. New unit tests have been added to validate this updated error propagation logic, and existing SQL logic tests have been updated to reflect the new, more concise error messages for signature mismatches.

[martin-augment]

Arity-mismatch error quality regression (hex.slt)

The hex.slt test change replaces a specific, actionable error message with a generic one:

- statement error Function 'hex' expects 1 arguments but received 2
+ statement error DataFusion error: Error during planning: Function 'hex' failed to match any signature
SELECT hex(1, 2);

The old message "Function 'hex' expects 1 arguments but received 2" directly told users what went wrong. The new message "Function 'hex' failed to match any signature" gives no actionable hint. This happens because the arity-mismatch plan_err! from get_valid_types (for TypeSignature::Any(n)) is now silently discarded and replaced with the generic fallback.

A more surgical approach could preserve the "best" planning error (e.g., the arity-mismatch message) rather than discarding all Plan errors uniformly. For functions with a OneOf containing a single fixed-arity overload, the arity error is unambiguously the right message to surface.

value:useful; category:bug; feedback: The Claude AI reviewer is correct! The old error was concrete and the user could fix the issue immediately. The new error is generic and does not tell the user what is wrong. (s)he will have to consult with the documentation and compare it against the way (s)he tries to use the hex function to realize where is the problem