21135: fix: propagate errors for unsupported table function arguments instead of silently dropping them

[martin-augment]

21135: To review by AI

[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 enhances the robustness of table function argument parsing by ensuring that invalid or unsupported argument types are no longer silently ignored or lead to ambiguous error messages. The changes allow for immediate and precise error reporting, improving the developer and user experience when interacting with DataFusion's SQL capabilities.

Highlights

• Error Propagation: Modified the SQL to relational planner to correctly propagate errors when encountering unsupported table function arguments, instead of silently dropping them or producing misleading errors.
• Improved Error Handling: Changed the argument processing for table functions from flat_map to map and introduced collect::<Result<Vec<_>>>()? to ensure that any errors during argument conversion are immediately returned.
• New Integration Test: Added a new integration test to specifically verify that unsupported tuple arguments for table functions now correctly yield a 'Struct not supported' error.
• New SQL Logictests: Introduced several new SQL logictests to confirm that various unsupported argument types (e.g., named arguments, wildcard expressions) for table functions now produce appropriate error messages.

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. ↩

[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: 69065146-2a58-4bc8-9147-a1d0984140dd

📥 Commits

Reviewing files that changed from the base of the PR and between 132bf44 and e616e83.

📒 Files selected for processing (3)

• datafusion/sql/src/relation/mod.rs
• datafusion/sql/tests/sql_integration.rs
• datafusion/sqllogictest/test_files/table_functions.slt

---

Walkthrough

The changes modify error handling in table function argument processing. The SQL relation module switches from deferred error handling via flat_map to immediate error propagation via map, changing the collection pattern to collect::<Result<Vec<_>>>()?. This ensures conversion errors from unsupported argument types are raised earlier with specific error details rather than generic fallback messages. The integration test suite verifies this behavior with new test cases confirming that unsupported argument expressions (tuples, structs, named parameters, wildcards) produce appropriately specific error messages during query planning.

✨ 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-21135-2026-03-25-08-42-52

---

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.}

[augmentcode]

🤖 Augment PR Summary

Summary: Ensure unsupported table-function arguments surface as planning errors rather than being silently dropped.
Changes: Convert argument iteration to map + collect::<Result<Vec<_>>>()? so Err short-circuits.
Tests: Add SQL integration test for tuple argument error propagation.
Tests: Add sqllogictest cases for named args and wildcard args (, t.).
Why: Avoid misleading downstream errors such as “requires 1 to 3 arguments” / “Table Functions are not supported”.
Notes: Behavior is unchanged for supported positional expression arguments.

_{🤖 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.

[claude]

Test comment - please ignore

[claude]

placeholder

[gemini-code-assist]

Code Review

This pull request refactors the processing of function arguments in SQL to relational plan conversion. It changes flat_map to map and introduces early error propagation using collect::<Result<Vec<_>>>()? when converting SQL expressions to logical expressions for function arguments. This ensures that unsupported function argument types correctly propagate specific error messages instead of generic or misleading ones. New integration tests and sqllogictests have been added to validate this improved error handling. There are no review comments to address.

[claude]

Code Review - PR 21135: propagate errors for unsupported table function arguments

This is a clean and correct bug fix. Here is my analysis:

WHAT THE BUG WAS

The original code used flat_map to process table function arguments. In Rust, Result implements IntoIterator: Ok(v) yields one item, but Err(e) yields zero items. This caused flat_map to silently discard the unsupported-argument error, leading to confusing downstream errors like 'requires 1 to 3 arguments' instead of the actual cause.

THE FIX

Changing to .map(...).collect() with Result propagation via the ? operator is the idiomatic Rust pattern. The first error short-circuits and is propagated immediately.

POSITIVE ASPECTS

• Minimal, focused change: only touches what is necessary.
• Three test vectors covering named args, unqualified wildcard, and qualified wildcard.
• SQL logictest error messages use correct regex anchoring for special characters.
• Trailing whitespace cleanup in table_functions.slt is a nice touch.

MINOR OBSERVATIONS (non-blocking)

1. Test fragility: The unit test asserts assert_contains!(msg, Struct not supported). This message comes from sql_expr_to_logical_expr processing the tuple, not from the new error path itself. If struct-handling error messages change, this test will break confusingly. A brief comment noting this dependency would help future maintainers.

2. Direct error path coverage (optional): A test with a named argument like start => 1 would more directly exercise the else branch and verify the plan_err! message. The sqllogictest already covers this, so it is optional.

3. No behavior change for valid arguments confirmed.

SUMMARY

The fix is correct, idiomatic, and well-tested. Ready to merge.