apache / apache/datafusion-sqlparser-rs

Always wrap statement in a named struct

Open
#1,204 5 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.5k
Forks
772
Avg merge
4d 9h
Merged PRs (30d)
17

Description

In [GreptimeDB](https://github.com/GreptimeTeam/greptimedb/blob/v0.7.1/src/sql/src/statements/delete.rs), we write code like:

```rust
#[derive(Debug, Clone, PartialEq, Eq, Visit, VisitMut)]
pub struct Delete {
// Can only be sqlparser::ast::Statement::Delete variant
pub inner: Statement,
}
```

It's because in sqlparser-rs, these statement variants are unnamed struct:

```rust
Delete {
/// Multi tables delete are supported in mysql
tables: Vec,
/// FROM
from: Vec,
/// USING (Snowflake, Postgres, MySQL)
using: Option>,
/// WHERE
selection: Option,
/// RETURNING
returning: Option>,
},
```

I wonder if we can always wrap statement in a named struct so that downstream software can reuse the AST and impls (like for Display) more smoothly.

I don't know if it's a breaking change or we're generally OK with this.

Ref - https://github.com/GreptimeTeam/greptimedb/issues/3646

Contributor guide

No contributing guide indexed for this repository

Research direction

Start by reviewing the statement definitions in the sqlparser AST and the downstream wrapper shown in GreptimeDB's src/sql/src/statements/delete.rs. Check how downstream code implements or consumes Display and other AST traits, then determine the compatibility impact of wrapping each statement variant. Done means a documented design or accepted implementation plan that addresses reuse and breaking-change concerns.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, sql
Domain
compilers
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.