-
Notifications
You must be signed in to change notification settings - Fork 126
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
Support duckdb #304
base: main
Are you sure you want to change the base?
Support duckdb #304
Conversation
Hi, and thanks a lot for your interest! As I replied on #295 (comment) I'd prefer to have these drivers on external repos as to facilitate maintenance cause my bandwidth is limited. I would love to add this to the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi! Just noticed this PR. Thanks for implementing this! I would be interested in this feature. I am not familiar with refinery's internals so I can't give a very in-depth review.
Are you considering moving this to a separate repo as @jxs suggested?
@@ -16,4 +16,7 @@ pub mod mysql; | |||
#[cfg(feature = "tiberius")] | |||
pub mod tiberius; | |||
|
|||
#[cfg(feature = "duckdb")] | |||
pub mod ducdb; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean? 😄
pub mod ducdb; | |
pub mod duckdb; |
fn execute(&mut self, queries: &[&str]) -> Result<usize, Self::Error> { | ||
let transaction = self.transaction()?; | ||
let mut count = 0; | ||
for query in queries.iter() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this is not needed AFAIK
for query in queries.iter() { | |
for query in queries { |
while let Some(row) = rows.next()? { | ||
let version = row.get(0)?; | ||
let applied_on: String = row.get(2)?; | ||
// Safe to call unwrap, as we stored it in RFC3339 format on the database |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the useful comment! Perhaps we could use expect()
to add a bit of extra context? Could be useful in the odd situation where this actually panics (bitflips, data corruption, etc)
This pull request introduces support for DuckDB. The majority of the implementation is adapted from the existing rusqlite support, with necessary modifications for duckdb. Note that one or two test cases are slightly different as barrel currently does not support DuckDB.