You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/10/14 14:17:09 UTC
[I] Move large Statement arms out of SqlToRel::sql_statement_to_plan_with_context_impl [datafusion]
andygrove opened a new issue, #26301:
URL: https://github.com/apache/datafusion/issues/26301
### Is your feature request related to a problem or challenge?
[`SqlToRel::sql_statement_to_plan_with_context_impl` (sql/src/statement.rs#L267-L1649)](https://github.com/apache/datafusion/blob/0d79252f61bd8777990b2749c216ffd07079c30d/datafusion/sql/src/statement.rs#L267-L1649) is a 1,307-line match over `ast::Statement`, with cognitive complexity 357 and cyclomatic complexity 217. `statement.rs` has had 39 commits in the last 12 months.
Many arms are already thin dispatchers (`explain_to_plan`, `set_statement_to_plan`, `describe_table_to_plan`, ...), but about a dozen inline large bodies:
- `CreateTable` (about L312-L652, ~340 lines), mostly destructuring ~70 sqlparser fields followed by a long run of `if x { return not_impl_err!(..) }`.
- `CreateView`, `Drop`, `Prepare`/`Execute`, `ShowTables`/`ShowColumns`, `Insert`, `StartTransaction` (L1268-L1344), `CreateFunction` (L1372-L1524) and `Truncate`.
Every sqlparser upgrade that adds a field to `CreateTable` has to touch this destructuring pattern, which makes the function a recurring merge-conflict and review burden.
### Describe the solution you'd like
- Make the match a pure dispatcher where each large arm calls a method on `SqlToRel`, following the existing `describe_table_to_plan` pattern: `create_table_to_plan`, `create_view_to_plan`, `drop_to_plan`, `prepare_to_plan`, `execute_to_plan`, `show_tables_to_plan`, `start_transaction_to_plan`, `create_function_to_plan`, `truncate_to_plan`, ...
- For `CreateTable`, bind the struct by name (`Statement::CreateTable(create_table)`) and move the unsupported-option checks into a helper such as `reject_unsupported_create_table_options(&create_table)`, so new sqlparser fields don't require editing the match pattern.
- Optionally split `statement.rs` into submodules by statement family (DDL, DML, utility).
### Describe alternatives you've considered
Leave it as is.
### Additional context
Line references are pinned to commit 0d79252f61bd.
--
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: github-unsubscribe@datafusion.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [I] Move large Statement arms out of SqlToRel::sql_statement_to_plan_with_context_impl [datafusion]
Posted by "Theodus (via GitHub)" <gi...@apache.org>.
Theodus commented on issue #26301:
URL: https://github.com/apache/datafusion/issues/26301#issuecomment-6200738448
take
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org