apache / apache/datafusion-sqlparser-rs
COPY TO/FROM is overly restrictive on options
- Ngôn ngữ chính
- Rust
- Star
- 3.5k
- Fork
- 772
- Merge trung bình
- 4 ngày 9 giờ
- Pull request đã merge (30 ngày)
- 17
Mô tả
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)
Hướng dẫn đóng góp
Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này
Hướng nghiên cứu
Bắt đầu với việc phân tích cú pháp các tùy chọn COPY trong src/parser/mod.rs tại các dòng được tham chiếu, cùng với các định nghĩa CopyOption và Statement::Copy trong src/ast/mod.rs. So sánh biểu diễn hiện tại với các ví dụ tùy chọn của PostgreSQL, DuckDB và DataFusion trong issue, bao gồm cả các khóa được đặt trong dấu ngoặc kép và các giá trị linh hoạt. Được xem là hoàn tất khi các tùy chọn COPY đủ tổng quát cho những phương ngữ đó mà không hạn chế không cần thiết các khóa hoặc giá trị, đồng thời các tùy chọn legacy vẫn được tính đến.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- rust
- Lĩnh vực
- compilers
- Loại issue
- Lỗi
- Độ khó
- 5/5
- Thời gian dự kiến
- Hơn một tuần
- Mức độ hoạt động
- Đình trệ
- Độ rõ ràng
- Khá rõ ràng
- Mức phù hợp với người mới
- 30/100