apache / apache/datafusion-sqlparser-rs
Consistent AST encoding of parentheses to enable correct spans for function arguments, subqueries etc.
- 主要语言
- Rust
- 星标
- 3.5k
- 派生
- 772
- 平均合并
- 4 天 9 小时
- 30 天内合并 PR
- 17
描述
First off, I'm relatively new to both this codebase and Rust in general. But I thought it might be useful to start this topic anyway.
Epic: #1548
There are a bunch of places where **parentheses** can appear in SQL. Currently these are not handled consistently in the AST. This makes it difficult or impossible to attach Spans to all of them. @Nyrox who did much of the work introducing Spanned [mentioned](https://github.com/apache/datafusion-sqlparser-rs/issues/1563#issuecomment-2586942739) using AttachedToken to encode opening and closing parenthesis tokens in the AST. I think it would be good to come up with a standard way that can be applied to all of those nodes. Also it seems in many instances this might be a breaking change.
Attaching parentheses tokens to the AST is important for accurate spans: The span for the Function node `CALL(something)` should go all the way from `C` to `)`, not just `CALL` or even worse, `CALL(something`. Currently there's no way to encode this information.
On the other hand, parentheses are not semantically meaningful for the AST, so I don't want to needlessly complicate the structure.
One thing that's a bit vague to me is if the parentheses should be attached to the 'thing' itself or its container? E.g. there is Function -> FunctionArguments -> FunctionArgumentsList, to which of those do the parens belong? Similarly, Expr::InSubquery has a subquery field; do the parens belong to the subquery or to the InSubquery? (Reading Fmt::Display for Expr::InSubquery would imply the parens belong to it since it renders them and not the subquery which is just an Expr – but the subquery field could be a new Subquery type, which then would be able to render its own parens... 🤔)
To make matters even more complicated, some of these parentheses are optional (see `FunctionArguments` and also the somewhat related `OneOrManyWithParens`).
Here are some nodes that need to know about their parentheses (I might have missed some; to be systematic I'd probably go through every mention of LParen/RParen in the parser).
in ast/mod.rs
- Expr::Subquery
- Expr::InSubquery
- Expr::InUnnest
- Expr::Exists
- Expr::AllOp.right
- FunctionArguments
- FunctionArgumentList
- Statement::Execute.has_parentheses
in ast/query.rs
- Cte.closing_paren_token
Here's an initial idea: instead of adding `opening_paren_token` and `closing_paren_token` everywhere, how about creating a new struct `ParensPair { opening: AttachedToken, closing: AttachedToken }`? Or might we augment `OneOrManyWithParens` for this?
贡献指南
这个仓库没有索引到贡献指南
调研方向
首先检查 ast/mod.rs 和 ast/query.rs 中列出的节点,然后按照 issue 中的建议,追踪 parser 对 LParen 和 RParen 的每一处使用。比较当前 span 的形成方式,并记录一种一致的表示方式及其兼容性影响;该 issue 目前还没有确定的设计或验收标准。
由索引模型根据 Issue 内容生成。
评估
- 技术栈
- rust, sql
- 领域
- compilers
- Issue 类型
- 功能
- 难度
- 5/5
- 预计耗时
- 一周以上
- 活跃度
- 停滞
- 描述清晰度
- 需要澄清
- 新手友好度
- 25/100