Skip to content

sql-parser: absorb separate sql parser repository #1384

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

Merged
merged 2 commits into from
Dec 24, 2019

Conversation

benesch
Copy link
Contributor

@benesch benesch commented Dec 21, 2019

Absorb MaterializeInc/sqlparser as a new crate named sql-parser.
Having a separate repository actively hinders Materialize development,
as every change requires at least three steps: making the change in the
sqlparser repository, bumping the version of sqlparser in the
materialize repository, and then actually using the new feature in the
materialize repository.

For a while, the upstream library was seeing active development, so a
separate repository made sense, as it eased the process of incorporating
upstream patches. But at this point we've far surpassed upstream in our
SQL parsing capabilities, and development upstream has stalled entirely.
Plus, upstream has the dubious goal of supporting all popular SQL
dialects, while we have the simpler task of just supporting the
Materialize SQL dialect; ripping out support for the other dialects will
make the code much simpler.

This is a direct import of the sqlparser repository, modulo changes to
the license header and Cargo.toml. Cleanups will follow in future
commits.

@benesch benesch force-pushed the absorb-sqlparser branch 2 times, most recently from 213d393 to 1942eac Compare December 21, 2019 12:12
@benesch benesch force-pushed the absorb-sqlparser branch 3 times, most recently from 4b2d62f to ca37d82 Compare December 24, 2019 17:13
Absorb MaterializeInc/sqlparser as a new crate named sql-parser.
Having a separate repository actively hinders Materialize development,
as every change requires at least three steps: making the change int he
sqlparser repository, bumping the version of sqlparser in the
materialize repository, and then actually using the new feature in the
materialize repository.

For a while, the upstream library was seeing active development, so a
separate repository made sense, as it eased the process of incorporating
upstream patches. But at this point we've far surpassed upstream in our
SQL parsing capabilities, and development upstream has stalled entirely.
Plus, upstream has the dubious goal of supporting all popular SQL
dialects, while we have the simpler task of just supporting the
Materialize SQL dialect; ripping out support for the other dialects will
make the code much simpler.

This is a direct import of the sqlparser repository, modulo changes to
the license header and Cargo.toml. Cleanups will follow in future
commits.
We don't use this feature, so it's not worth the extra complexity.
@benesch
Copy link
Contributor Author

benesch commented Dec 24, 2019

As discussed in Slack, this is now still licensed under Apache 2. Merging because this is blocking a bunch of other work (see MaterializeInc/database-issues#492) and the import was very mechanical.

@benesch benesch merged commit 526eba0 into MaterializeInc:master Dec 24, 2019
@benesch benesch deleted the absorb-sqlparser branch December 24, 2019 17:33
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

Successfully merging this pull request may close these issues.

1 participant