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.

Sign up for GitHub

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

Jump to bottom

Update sqlparser-derive to use syn 2.0 #1040

Merged
merged 5 commits into from
Nov 27, 2023
Merged

Update sqlparser-derive to use syn 2.0 #1040

merged 5 commits into from
Nov 27, 2023

Conversation

serprex
Copy link
Contributor

@serprex serprex commented Nov 6, 2023 •
edited
Loading

@tobyhede
Copy link
Contributor

@serprex Some more context on this change would be useful ... I assume it is updating syn?

@serprex
Copy link
Contributor Author

serprex commented Nov 14, 2023

Yes

alamb
alamb reviewed Nov 18, 2023
View reviewed changes
derive/Cargo.toml Outdated Show resolved Hide resolved
@alamb
Copy link
Contributor

alamb commented Nov 18, 2023

Thank you for this contribution @serprex

@coveralls
Copy link

coveralls commented Nov 18, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 7009097570

  • 0 of 17 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.004%) to 87.706%
Changes Missing Coverage Covered Lines Changed/Added Lines %
derive/src/lib.rs 0 17 0.0%
Totals Coverage Status
Change from base Build 7008917646: -0.004%
Covered Lines: 17992
Relevant Lines: 20514
💛 - Coveralls

@serprex
Copy link
Contributor Author

serprex commented Nov 18, 2023

Won't have time this weekend to look into, but will look at how to run tests & fix next week (I only ran cargo test in derive crate)

@alamb alamb marked this pull request as draft November 20, 2023 19:45
@alamb
Copy link
Contributor

alamb commented Nov 20, 2023

Thanks @serprex . Converting to a draft while review feedback is being addressed. Please mark it ready for review when it is ready for another look

@serprex serprex marked this pull request as ready for review November 21, 2023 00:43
@alamb alamb changed the title derive: syn 2 Update sqlaprser-derive to use syn 2.0 Nov 21, 2023
@alamb alamb mentioned this pull request Nov 21, 2023
Closed
7 tasks
@serprex serprex changed the title Update sqlaprser-derive to use syn 2.0 Update sqlparser-derive to use syn 2.0 Nov 22, 2023
@alamb alamb mentioned this pull request Nov 27, 2023
Closed
8 tasks
@alamb
Copy link
Contributor

alamb commented Nov 27, 2023

Given I think this may be a non trivial change, I plan to merge this immediately after #1030

Then this PR will have maximum "bake time" for early adopters and others to try it out before I release it on crates.io

@alamb alamb mentioned this pull request Nov 27, 2023
Closed
5 tasks
alamb
alamb reviewed Nov 27, 2023
View reviewed changes
derive/Cargo.toml
@@ -18,6 +18,6 @@ edition = "2021"
proc-macro = true

[dependencies]
syn = "1.0"
syn = { version = "2.0", default-features = false, features = ["printing", "parsing", "derive", "proc-macro"] }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems according to
https://docs.rs/syn/latest/syn/index.html#optional-features

Listing these 4 will at least avoid

clone-impls (enabled by default) — Clone impls for all syntax tree types.

alamb
alamb approved these changes Nov 27, 2023
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me -- thank you @serprex

I took the liberty of merging up from main and reducing the feature set used

@alamb alamb merged commit 8d97330 into apache:main Nov 27, 2023
10 checks passed
@alamb
Copy link
Contributor

alamb commented Nov 27, 2023

Thanks again @serprex and @tobyhede

serprex added a commit to PeerDB-io/sqlparser-rs that referenced this pull request Dec 3, 2023
@serprex
Merge 0.40 + apache/datafusion-sqlparser-rs#1040
ebeab80
@serprex serprex mentioned this pull request Dec 3, 2023
Merged
serprex added a commit to PeerDB-io/sqlparser-rs that referenced this pull request Dec 11, 2023
@serprex
Merge pull request #28 from PeerDB-io/merge-upstream
e2a651a 
Merge 0.40 + apache/datafusion-sqlparser-rs#1040

Upstream implemented END parsing as COMMIT on postgresql: apache/datafusion-sqlparser-rs#1035 allowing some convergence
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alamb alamb alamb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@serprex @tobyhede @alamb @coveralls