apache / apache/datafusion-sqlparser-rs

make `Parser` generic around dialect

Open
#1,381 4 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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.