Skip to content

fix(sql): reject unsupported relation modifiers - #24758

Draft
geoffreyclaude wants to merge 1 commit into
apache:mainfrom
geoffreyclaude:codex/relation-modifier-rejection
Draft

fix(sql): reject unsupported relation modifiers#24758
geoffreyclaude wants to merge 1 commit into
apache:mainfrom
geoffreyclaude:codex/relation-modifier-rejection

Conversation

@geoffreyclaude

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

SQL should either do what the user wrote or explain that it cannot. Today, if no RelationPlanner claims a relation, the default path can silently drop modifiers such as TABLESAMPLE or VERSION AS OF and produce an ordinary table scan. That is especially surprising after an extension planner deliberately passes the relation through: a successful query may have different semantics from the SQL on the screen.

Real integrations show why this boundary matters:

This PR puts the same safety net at DataFusion's default fallback boundary.

What changes are included in this PR?

  • Keep the existing extension-first flow: a custom planner can still consume any modifier it supports.
  • Make the default planner reject unsupported named-table modifiers, derived-table sampling, table-function WITH ORDINALITY, and table-function SETTINGS with clear errors.
  • Destructure the affected TableFactor and TableFunctionArgs shapes exhaustively. A future sqlparser field now requires an explicit decision at compile time.

Are these changes tested?

Yes. The regression test exercises both fallback paths—without an extension planner and after a pass-through planner—and covers five representative forms across named tables, derived tables, and table functions.

I also ran:

  • cargo fmt --all
  • cargo clippy --all-targets --all-features -- -D warnings
  • the contributor guide's extended workspace test command with the pinned test-data submodules initialized

Are there any user-facing changes?

Yes, intentionally: syntax that was previously accepted but ignored now returns a clear “not supported” planning error. Queries without these modifiers are unchanged, and extensions that consume a modifier before fallback continue to work as before.

@github-actions github-actions Bot added sql SQL Planner core Core DataFusion crate labels Aug 28, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.79592% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.48%. Comparing base (4d3e79e) to head (10587f4).

Files with missing lines Patch % Lines
datafusion/sql/src/relation/mod.rs 89.79% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24758      +/-   ##
==========================================
- Coverage   81.48%   81.48%   -0.01%     
==========================================
  Files        1122     1122              
  Lines      404248   404292      +44     
  Branches   404248   404292      +44     
==========================================
+ Hits       329390   329420      +30     
- Misses      55547    55558      +11     
- Partials    19311    19314       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

core Core DataFusion crate sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Relation planning silently ignores unsupported table modifiers

2 participants