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

Make crate::parse_result::ParseResult public #43

Merged
merged 4 commits into from
Aug 2, 2024

Conversation

levkk
Copy link
Contributor

@levkk levkk commented Apr 22, 2024

parse_result::ParseResult is returned from crate::query::parse which is a public method but the result struct is private. This compiles and runs fine typically, but it's impossible to pass the output of parse to other functions since we can't import the private struct and declare it as an input variable.

Copy link
Member

@seanlinsley seanlinsley left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I guess this wasn't caught before because our code doesn't refer to ParseResult explicitly even though its fields are accessed

src/query.rs Outdated
@@ -5,7 +5,7 @@ use prost::Message;

use crate::bindings::*;
use crate::error::*;
use crate::parse_result::ParseResult;
pub use crate::parse_result::ParseResult;
Copy link
Member

Choose a reason for hiding this comment

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

To keep all re-exports in the same place, this should probably be moved to lib.rs as pub use parse_result::*, and then this file can replace all imports with a single use crate::*

Cargo.toml Outdated
@@ -1,7 +1,7 @@
[package]
name = "pg_query"
description = "PostgreSQL parser that uses the actual PostgreSQL server source to parse SQL queries and return the internal PostgreSQL parse tree."
version = "5.1.0"
version = "5.1.0-1"
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious, what's the motivation here? Is that version format standard in the Rust open source world?

So far we've only bumped the version number when publishing a new release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The version bump would allow to release this crate without necessarily releasing a new version of pg_query the C library (I believe pg_query.rs was synced with the C library version at some point recently). The versioning is technically semver, but I'm not sure it's strictly how Rust crates do it; typically, only major, minor and patch numbers are used.

@seanlinsley seanlinsley merged commit 37e6690 into pganalyze:main Aug 2, 2024
@seanlinsley seanlinsley mentioned this pull request Oct 17, 2024
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.

2 participants