apache / apache/datafusion-sqlparser-rs
make `Parser` generic around dialect
- Dominant language
- Rust
- Stars
- 3.5k
- Forks
- 772
- Avg merge
- 4d 9h
- Merged PRs (30d)
- 17
Description
if we care about performance, we should stop using dynamic dispatch and make the parser generic around the dialect, with that we could make lots of these methods `const` (or drop the `Precedence` enum and just have const values on the trait) and probably improve performance significantly in general.
That would of course be a big change to the public API.
This is definitely how would implement `Parser` if I was starting now, but I think we should see some evidence that parsing SQL is a meaningful chunk of time for anyone before making a change like this.
My guess is that:
* even for quick queries, SQL parsing is <1% of query time
* making `Parser` generic and therefore dropping the restriction on `Dialect` that it has to be 'object safe' would actually only save us ~20%
If both those assumptions are right, this doesn't seem worth it unless it makes the code generally easier to reason with and work on.
_Originally posted by @samuelcolvin in https://github.com/sqlparser-rs/sqlparser-rs/issues/1379#issuecomment-2289164242_
(separate issue seems worth it for this discussion)
Contributor guide
No contributing guide indexed for this repository
Research direction
Start by examining Parser, Dialect, and the Precedence enum, then establish whether SQL parsing is a meaningful part of query time. Benchmark the current dynamic-dispatch approach and compare it with a generic Parser design. Done means having evidence about the performance benefit and a settled direction for the public API change.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- compilers, databases
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 25/100