Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discuss of sqlparser-rs project status / maintenance October 2022 #643

Closed
alamb opened this issue Oct 1, 2022 · 16 comments
Closed

Discuss of sqlparser-rs project status / maintenance October 2022 #643

alamb opened this issue Oct 1, 2022 · 16 comments

Comments

@alamb
Copy link
Collaborator

alamb commented Oct 1, 2022

I hope this ticket will serve as a place to discuss the current status of the sqlparser-rs project and where the maintainers plan to take it

Background / People with Write Access

This project was initially started by @andygrove

At various times, @benesch @maxcountryman @Dandandan @nickolay and myself have helped maintain the code. This is also the list of people who have write access to the github repo and crates.io https://crates.io/crates/sqlparser

Current Status

The project is structurally complete -- the maintainers accept straightforward additions to support other sql statements and dialects as described in https://github.com/sqlparser-rs/sqlparser-rs#contributing but basically we haven't made any significant changes to the crate in several years.

Issues with Current Status

However, there are proposals to make more more significant improvements to sqlparser, and this is where we run into issues.

Some examples:

These are all great feature ideas, but they also require downstream code churn as well as additional maintenance. There are different opinions on how the benefit vs cost tradeoff

However, all such ideas basically die in the "the maintainers aren't willing or have time to review / maintain the changes" phase. This mismatch in expectation is not fair to contributors

Possible ways forward

We can add new maintainers to this crate who have more energy (as @lovasoa has offered here: #601 (comment)) that can take the project in a new direction

We work on an "ecosystem" of crates so that additional add on crates can add functionality that is not in the core sqlparser crate

Users who want new features can fork the project (it is all apache 2 licensed and there are no restrictions on doing so)

@alamb
Copy link
Collaborator Author

alamb commented Oct 1, 2022

I wanted to be explicit that my motivation for maintaining this crate is that we use it as part of https://github.com/apache/arrow-datafusion and https://github.com/influxdata/influxdb_iox. While we periodically want to extend with some additional sql syntax, the basic feature set is good for us. I don't (yet) need any of the other proposed features and thus don't have the time to devote to driving them forward (nor the downstream implications of major API changes to this crate)

Thus my preference is to keep on with the status quo (small sql syntax additions but no structural changes)

@AugustoFKL
Copy link
Contributor

@alamb I'd love to be part of the maintainer team if possible :)

I work a lot with SQL and Rust, so this project is a really meaningful way to learn and develop my skills, and coding it is really fun

@alamb
Copy link
Collaborator Author

alamb commented Oct 1, 2022

@alamb I'd love to be part of the maintainer team if possible :)

Thank you -- I think the best way to help initially would be to help review the PRs and get them ready for merge -- and @alamb me when they are ready. 🙏

@lovasoa
Copy link
Contributor

lovasoa commented Oct 2, 2022

Thank you very much @alamb for opening this !
I reiterate my proposition to help maintain the AST visitor in the long term if it's merged. And speaking for my particular PR; it doesn't require any downstream code churn: it is just a new trait implementation, behind a feature flag that is disabled by default.

@andygrove
Copy link
Collaborator

Thanks for raising this @alamb. I would love to spend more time helping here, but I just don't have the time right now, and this crate is "good enough" for the projects where I use it (DataFusion/Ballista & Dask SQL).

It would be great to get some new maintainers here.

@Dandandan
Copy link
Collaborator

Similar to @andygrove the focus on sqlparser-rs is limited from my side - you van expect some occasional help with reviewing and merging open MRs whenever I have some spare time. New active maintainers would be great 👍

@SuperBo
Copy link
Contributor

SuperBo commented Oct 9, 2022

Hi everybody, about the future of this project. I want to use sqlparser-rs as a base parser for a project which can hint most popular SQL dialects.

Is fully support BigQuery, Snowflake, and Postgres one of the achievement of this project?

@AugustoFKL
Copy link
Contributor

Hi everybody, about the future of this project. I want to use sqlparser-rs as a base parser for a project which can hint most popular SQL dialects.

Is fully support BigQuery, Snowflake, and Postgres one of the achievement of this project?

@SuperBo I think @alamb can answer it better but, from my POV, theoretically, the project would handle all syntactical elements of each supported dialect. However, as the number of maintainers is reduced, there's no way to expand the project in a really fast manner.

I think supporting generic features (e.g., common data types and statements) is more feasible, but still, full support for each dialect seems impossible, as the AST would need to be dialect-specific, which isn't possible considering the current project contributing system.

@46bit
Copy link

46bit commented Oct 10, 2022

I agree that large features that affect the whole codebase do need more maintainer time. My changes in #501 do affect the parser code quite a bit and I'm happy to join as a maintainer. We've just hit open beta with our API that uses this library so I should have more time soon.

@alamb
Copy link
Collaborator Author

alamb commented Oct 10, 2022

Thank you everyone who is offering to help maintain this library. The first thing I would recommend for would be maintainers and those who would like to see larger changes would be to help maintain the current crate. This means reviewing PRs and ensuring they are tested, documented, and conform to the current style of the project and helping new contributors.

To be open and transparent, I am worried about people who are interested in maintaining the code for some short period of time, making major changes, and then getting busy with other projects. Of course there is turnover over time, but I want make sure this library can remain an easy to use, small, sql parsing library. As it is open source / apache licensed, forks are always possible for those who want a different direction (and several more advanced users have done just that)

@SuperBo I look at this library as incrementally adding support towards all SQL dialects. As @AugustoFKL points out, full support for all dialects is unlikely to happen without much more significant investment of time and some technical changes, but nothing fundamental prevents it in my mind

@alamb
Copy link
Collaborator Author

alamb commented Oct 10, 2022

My changes in #501 do affect the parser code quite a bit and I'm happy to join as a maintainer.

Thanks @46bit -- Issue #501 is one I do see as really needing an in-crate change to support, rather than it simply being a matter of "easier to maintain"

@SuperBo
Copy link
Contributor

SuperBo commented Oct 11, 2022

Hi, I agree with you guys that to fully support all dialects, restructure AST is inevitable. This will requires a major change in code and maintenance effort.

@omer-shtivi
Copy link
Contributor

I want to help the project.
how do I know which issues are accepted, and I can start work on?

@AugustoFKL
Copy link
Contributor

@omer-shtivi usually I just open the PR for a feature that I know is feasible and simple to be reviewed.

Huge changes aren't prone to be accepted, as mentioned by a lot of discussions and in the readme.

I'd focus on issues that don't require huge refactors, as @alamb looks like to be the only stable maintainer for now, with not completely dedicated time. Huge PRs are a pain anyway.

If it's something really controversial or you're not sure whether it's a reasonable change, create and issue and please link me. We can discuss this before needing the hand of @alamb. I have some spare time, mainly on weekends, to help here and review PRs/issues.

@alamb
Copy link
Collaborator Author

alamb commented Oct 31, 2022

Thanks @AugustoFKL -- I agree with #643 (comment) 👍

Basically I try to do a release about once a month by collecting contributions (as @AugustoFKL says mostly focused on extending the sql syntax that is supported by following the existing patterns).

I would definitely suggest discussing (feel free to tag me) if there is a larger change you have in mind if you want to know thoughts on our ability to accept it.

Given the limited maintenance time we have, I would be most interested in extending sqlparser's capabilities so that new features could be added in other crates rather than here where we have yet more to maintain.

For example, I would love it if someone created a sqlparser visitor / rewriter crate rather than add one in this repo as that adds features and doesn't require changes (at least any large changes) to this crate.

@alamb
Copy link
Collaborator Author

alamb commented Mar 1, 2023

Update here: #818

Closing this issue

@alamb alamb closed this as completed Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants