deps: upgrade to sqlx v0.8
#408
Conversation
| fn encode( | ||
| self, | ||
| buf: &mut <sqlx::Sqlite as Database>::ArgumentBuffer<'q>, | ||
| ) -> Result<IsNull, BoxDynError> { | ||
| Encode::<'_, sqlx::Sqlite>::encode(self.into_string(), buf) |
There was a problem hiding this comment.
| Encode::<'_, sqlx::Sqlite>::encode(self.into_string(), buf) | |
| Encode::<'_, sqlx::Sqlite>::encode(self.as_str(), buf) |
Needless allocation?
There was a problem hiding this comment.
I tried this change but it doesn't work. AFAICT what's happening is buf here is a SqliteArgumentValue which contains a Cow<'q, str>. Because we have self as an owned value, trying to use self.as_str() doesn't work because self gets dropped at the end of this scope, but needs to live for 'q.
There was a problem hiding this comment.
Yes, the 'q lifetime complicates life when Postgres or MySQL backend is used.
| buf: &mut <sqlx::Sqlite as HasArguments<'q>>::ArgumentBuffer, | ||
| ) -> IsNull { | ||
| buf: &mut <sqlx::Sqlite as Database>::ArgumentBuffer<'q>, | ||
| ) -> Result<IsNull, BoxDynError> { | ||
| Encode::<'_, sqlx::Sqlite>::encode(alloc::string::String::from(self.as_str()), buf) |
There was a problem hiding this comment.
| Encode::<'_, sqlx::Sqlite>::encode(alloc::string::String::from(self.as_str()), buf) | |
| Encode::<'_, sqlx::Sqlite>::encode(self.as_str(), buf) |
There was a problem hiding this comment.
Similar to the above, here we have a SqliteArgumentValue. Trying to use self.as_str() doesn't work because then the lifetime of &self would need to be longer than 'q, but there's no way to enforce that because then our impl would contain more lifetimes than the definition of the Encode trait
compact_str/src/features/sqlx.rs
Outdated
| @@ -41,7 +39,7 @@ where | |||
| DB: Database, | |||
| for<'x> &'x str: Decode<'x, DB> + Type<DB>, | |||
| { | |||
| fn decode(value: <DB as HasValueRef<'r>>::ValueRef) -> Result<Self, BoxDynError> { | |||
| fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> { | |||
| let value = value.to_owned(); | |||
There was a problem hiding this comment.
Good call! I tried removing the to_owned() but that didn't work because all we have here is a ValueRef. Instead I refactored this to use the Decode impl for &str which presumably would do the most efficient thing
|
@ParkMyCar , I tried to integrate this branch and tested |
ParkMyCar
force-pushed
the
deps/upgrade-sqlx
branch
from
52ec8af to
9f498c2
Compare
* skip using to_owned in impl of Decode, defer to Decode impl for &str * fix clippy in sqlx example
ParkMyCar
force-pushed
the
deps/upgrade-sqlx
branch
from
e58186c to
ffd9aba
Compare
|
Hey @rshigapov-bhft, thanks for the comments and extra testing here! Going to merge the PR but please let me know if there is anything I missed :) |
As it says on the tin! Upgrades the
sqlxdependency tov0.8, this picks up the fix for RUSTSEC-2024-0363.Fixes #407