apache / apache/datafusion-sqlparser-rs

Consistent AST encoding of parentheses to enable correct spans for function arguments, subqueries etc.

未關閉
#1,676 0 則留言 4 個 reaction 已指派 0 人 在 GitHub 檢視
主要語言
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

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。