Implement for sqlx #329
Implement for sqlx #329
Conversation
|
Mysql and Postgres are untested. |
|
I am using namespaced features in this PR now, which would mean that this MSRV is stuck to 1.60. Do you think that's okay? |
Yeah I think bumping the MSRV to 1.60 is a good idea! It will also help with #328 |
There was a problem hiding this comment.
Thanks for putting up this PR @Kijewski, I really appreciate it!
The implementation looks good, just one comment on the macro. Otherwise it looks like we'll need to update the x-plat CI workflow and exclude at least the sqlx/sqlite feature since we're failing to build sqlite via cross.
|
Yeah, I used a bit too many macros in here. :) Could you have a look how to update the CI workflow? I don't know exactly how to exclude the test when cross compiling. |
| #[cfg_attr(docsrs, doc(cfg(feature = "sqlx")))] | ||
| impl<'r, DB> Decode<'r, DB> for CompactString | ||
| where | ||
| DB: Database, | ||
| for<'x> &'x str: Decode<'x, DB> + Type<DB>, | ||
| { | ||
| fn decode(value: <DB as HasValueRef<'r>>::ValueRef) -> Result<Self, BoxDynError> { | ||
| let value = value.to_owned(); | ||
| let value: &str = value.try_decode()?; | ||
| Ok(value.try_to_compact_string()?) | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason to not specifically implement Decode for each of the Database variants? They each look like they have faster code paths for decoding &str that can be used to create a CompactStr.
There was a problem hiding this comment.
Those functions are only available in their respective crates, and let value: &str = value.try_decode()?; boils down to the same code.
|
Thanks for the PR @Kijewski! The CI issue looks to be |
No description provided.