apache / apache/datafusion-sqlparser-rs

COPY TO/FROM is overly restrictive on options

未關閉
#1,085 4 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視

還沒有人認領這個 Issue。

主要語言
Rust
星號
3.5k
分支
774
平均合併
4 天 9 小時
30 天內合併 PR
17

描述

Current parsing of `COPY TO/FROM` options is too restrictive on the how the keys & values can be provided.

https://github.com/sqlparser-rs/sqlparser-rs/blob/ce498864dc705f72e9f85d4dc5f7eba3d17b9ef6/src/parser/mod.rs#L5157-L5180

Given this parsing code, the expected format is:

```sql
COPY (SELECT 1 AS a, 2 AS b) TO 'file'
WITH (
FORMAT 'text',
FREEZE true,
DELIMITER 'd',
NULL 'null',
HEADER true,
QUOTE 'q',
ESCAPE 'e',
FORCE_QUOTE (a, b),
FORCE_NOT_NULL (a, b),
FORCE_NULL (a, b),
ENCODING 'encoding'
)
```

The problems:

- Key must be a keyword and cannot be a quoted string, even though postgres allows this, i.e. `WITH ("format" 'text', ...)`
- It strictly limits the key set at parsing time, leading to further problems:
- Need to update this parser whenever postgres supports a new key, e.g. DEFAULT (supported in postgres 16: https://www.postgresql.org/docs/16/sql-copy.html)
- We can't use this code for other databases/engines with a similar copy statement as they may have different supported keys (e.g. [duckdb](https://duckdb.org/docs/sql/statements/copy.html) and datafusion, see #1080)
- Some values are too strict in their format:
- FORMAT expects an identifier, which disallows escaped strings (`format e'csv'`), but this is allowed by postgres
- FREEZE expects keywords `true` or `false` but postgres allows these to be passed as string literals (single quoted, double quoted, escaped), e.g. `freeze 'true'`
- Same for HEADER

**Solutions**

We could fix these problems for Postgres specifically to make it more permissive, but I was hoping to also fix it to allow usage by other engine dialects (e.g. duckdb and datafusion). This might involve removing the `CopyOption` enum entirely:

https://github.com/sqlparser-rs/sqlparser-rs/blob/ce498864dc705f72e9f85d4dc5f7eba3d17b9ef6/src/ast/mod.rs#L4707-L4736

And replacing its usage in `Statement::Copy` with a more generic version that allows any string key with a new value enum to represent different values (e.g. string value, parenthesized column list value, number value):

https://github.com/sqlparser-rs/sqlparser-rs/blob/ce498864dc705f72e9f85d4dc5f7eba3d17b9ef6/src/ast/mod.rs#L1466-L1478

I'm not sure how the legacy options will fit into this, it could just be left as is and probably tackled as part of another ticket (as the original reason here was for interop with datafusion copy statement)

貢獻指南

這個儲存庫沒有索引到貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

研究方向

從參考行中的 src/parser/mod.rs 內的 COPY 選項解析,以及 src/ast/mod.rs 中的 CopyOption 和 Statement::Copy 定義開始。將目前的表示方式與 issue 中 PostgreSQL、DuckDB 和 DataFusion 的選項範例進行比較,包括加上引號的鍵與寬鬆的值。完成的標準是:COPY 選項對這些方言足夠通用,同時不會不必要地限制鍵或值,並且仍然將 legacy 選項納入考量。

由索引模型根據 Issue 內容生成。

評估

技術堆疊
rust
領域
compilers
Issue 類型
缺陷
難度
5/5
預估耗時
一週以上
活躍度
停滯
描述清晰度
基本清楚
新手友好度
30/100

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

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