-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Moveable binds and rows #3977
Moveable binds and rows #3977
Conversation
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 opening this PR and thanks for working on this. Overall, as already discussed the implementation looks good.
I left a round of feedback consisting of minor mostly stylistic things, otherwise this should be fine to merge.
cccd84a
to
5da1637
Compare
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.
Looks good, beside the minor stylistic changes requested.
3e20b51
to
db0eb9b
Compare
It will be a hard error in 2024 edition so implement it "properly" warning: creating a mutable reference to mutable static is discouraged --> diesel/src/sqlite/connection/mod.rs:229:18 | 229 | unsafe { &mut SQLITE_METADATA_LOOKUP } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable reference to mutable static | = note: for more information, see issue #114447 <rust-lang/rust#114447> = note: this will be a hard error in the 2024 edition = note: this mutable reference has lifetime `'static`, but if the static gets accessed (read or written) by any other means, or any other reference is created, then any further use of this mutable reference is Undefined Behavior = note: `#[warn(static_mut_refs)]` on by default
db0eb9b
to
e2c2c11
Compare
In the context of diesel_async, an async connection would also like to be used with sqlite. To enable creating an async connection wrapping a syn connection some types needs to be moveable and/or owned.
This PR adds support for moveable binds and owned row in core Diesel types as well as the sqlite connection and backend.
Support for postgres was tested at compile time but crashed due to invalid
Rc
constraints. That would need some deeper look to properly implement but doesn't seem to be blocking.