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

feat(rust/driver/snowflake): add adbc_snowflake crate with Go driver wrapper #2207

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

mbrobbel
Copy link
Contributor

@mbrobbel mbrobbel commented Oct 1, 2024

This PR adds the adbc_snowflake crate to the Rust workspace. This crate provides a Snowflake ADBC driver for Rust by wrapping the Go driver, loaded by the Rust driver manager implementation. The build.rs script builds the Go driver and links it statically.

  • Add methods to the wrapper structs to handle Snowflake specific configurations
  • Add a feature to support loading the Go driver dynamically
  • Docs
  • Tests

rust/Cargo.lock Outdated Show resolved Hide resolved
Comment on lines +35 to +56
type Option = OptionConnection;

fn set_option(&mut self, key: Self::Option, value: OptionValue) -> Result<()> {
self.0.set_option(key, value)
}

fn get_option_string(&self, key: Self::Option) -> Result<String> {
self.0.get_option_string(key)
}

fn get_option_bytes(&self, key: Self::Option) -> Result<Vec<u8>> {
self.0.get_option_bytes(key)
}

fn get_option_int(&self, key: Self::Option) -> Result<i64> {
self.0.get_option_int(key)
}

fn get_option_double(&self, key: Self::Option) -> Result<f64> {
self.0.get_option_double(key)
}
}
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 not familiar enough with Rust, but can't these just be inherited somehow from a base type instead of having to implement them here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we should be using traits for this. Discussed before here: #1725 (comment).

I'll try to put up a PR to change these option structs to traits following the proposal in #1725 (comment).

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