-
Notifications
You must be signed in to change notification settings - Fork 192
Bump rusqlite 0.33 -> 0.37; r2d2_sqlite 0.26 -> 0.31 #845
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
base: main
Are you sure you want to change the base?
Conversation
I tried building a crate depending on this branch and it stopped due to an error that may be unrelated evaluation panicked: Your generated code is out of date, please regenerate using planus version 1.2.0 reported from polars-arrow-format It looks like we need a |
Confirmed with a local setup that bumping these versions and pinning Planus compiles OK and rusqlite/connectorx integration tests pass 👍 I don't know if pinning a problematic dependency like Planus is good practice for crates so I've not done so in the PR, but it requires this to build: planus = { version = "=1.1.1" } I'm thinking perhaps I should contribute a test suite for sqlite though rather than having to do testing via my downstream project |
Thank you @lmmx for the PR!
That would be great. Please feel free to add tests to rust. This is definitely something missing. Currently we do have some simple tests for sqlite in python, which is also included in the ci for sqlite. The scripts for creating the test tables can be found here. Please let me know if you need any help from my side! |
Thank you for taking a look over it! 😁 🙏 I am just reviewing it more closely and spotted this was in the original connector-x/connectorx/Cargo.toml Line 50 in 0374786
So when I turn on the
Yes I have some in here which are essentially just integration tests in Rust for the rusqlite and arrow layer, this would be easy to "unwrap" back into pure connectorx.
Thank you for linking me, I'll take a look and try to keep the Rust scripts to the same style :-) |
rust_decimal
deprust_decimal
feature in thedst_arrow
feature grouprust_decimal
dep #844)I was going to continue with the other DB dependency bumps (outlined in #843) but I thought best to initially verify thisi s sound.
I can't actually see any tests for rusqlite in here, perhaps these ought be contributed too?