geoffreyclaude opened a new pull request, #24758: URL: https://github.com/apache/datafusion/pull/24758
## Which issue does this PR close? - Closes #24754. ## 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: - [Paimon implements `VERSION AS OF` and `TIMESTAMP AS OF`](https://github.com/apache/paimon-rust/blob/8001f02d8cc33bd3d5d77a1e7de781ff3add28d6/crates/integrations/datafusion/src/relation_planner.rs#L86-L104). - [VGI implements `TABLESAMPLE`](https://github.com/Query-farm/vgi-datafusion/blob/c6e3a6cfb05202a61233aa535912d8e2faae3e6e/src/sampling.rs#L65-L76). - [Coral already rejects unsupported modifiers exhaustively in its own planner](https://github.com/withcoral/coral/blob/8678de77aba0faed532e9b43995702a835dacca4/crates/coral-engine/src/runtime/scoped_table_functions.rs#L180-L223), so a sqlparser upgrade cannot quietly introduce another ignored field. 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. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
