Implement Decode, Encode and Type for Box, Arc, Cow and Rc
#3674
Conversation
joeydewaal
changed the title
Decode, Encode and Type for Box, Arc, Cow and Rc
@joeydewaal technically it's still possible to use by getting the We can relax the sqlx/sqlx-core/src/query_as.rs Line 111 in 277dd36 |
joeydewaal
force-pushed
the
encode-decode
branch
from
676e11e to
f2930a6
Compare
|
Makes sense, I added the impl for |
|
CI should be fixed if you rebase. |
joeydewaal
force-pushed
the
encode-decode
branch
from
9d694c0 to
b6521ae
Compare
joeydewaal
force-pushed
the
encode-decode
branch
7 times, most recently
from
0480f29 to
ff041d6
Compare
joeydewaal
force-pushed
the
encode-decode
branch
from
ff041d6 to
b59de52
Compare
joeydewaal
force-pushed
the
encode-decode
branch
from
b59de52 to
3857c69
Compare
| impl<'r, DB> Decode<'r, DB> for Cow<'r, str> | ||
| where | ||
| DB: Database, | ||
| &'r str: Decode<'r, DB>, | ||
| { | ||
| fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> { | ||
| let borrowed = <&str as Decode<DB>>::decode(value)?; | ||
| Ok(Cow::Borrowed(borrowed)) | ||
| } | ||
| } | ||
|
|
||
| impl<'r, DB> Decode<'r, DB> for Cow<'r, [u8]> | ||
| where | ||
| DB: Database, | ||
| &'r [u8]: Decode<'r, DB>, | ||
| { | ||
| fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> { | ||
| let borrowed = <&[u8] as Decode<DB>>::decode(value)?; | ||
| Ok(Cow::Borrowed(borrowed)) | ||
| } | ||
| } |
There was a problem hiding this comment.
These impls aren't really going to be practical to use. Consider the most common usage; there is basically never a row retained that can be borrowed from. query_as(), query_scalar() and all the macro variants throw away the row value after deserializing. #[derive(FromRow)] doesn't support borrowed values.
Manual calls to Row::get() would work, but that's about it. I think that's gonna confuse people more than anything. Arguably, we could get rid of the lifetime on FromRow entirely and only support deserializing from an owned row. It's more of an "it's there if you really wanna use it" kind of thing.
I think we could support both borrowed and owned data in the future with specialization, but it's not really feasible in the language currently.
I think for similar reasons, Serde only supports deserializing Cow::Owned despite having better support for deserializing from borrowed data overall.
There was a problem hiding this comment.
At first I only implemented Decode for Cow::Owned but I switched to Cow::Borrowed because the current impls also use Cow::Borrowed and changing this would be a breaking change (I think?).
| T: Encode<'q, DB>, | ||
| T: ToOwned, |
There was a problem hiding this comment.
| T: Encode<'q, DB>, | |
| T: ToOwned, | |
| T: Encode<'q, DB> + ToOwned, |
There was a problem hiding this comment.
This could possibly use a comment about how ToOwned is only required to satisfy Cow.
| impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'_, T> | ||
| where | ||
| T: Encode<'q, DB>, |
There was a problem hiding this comment.
This might get weird with non-'static lifetimes. I would test that it's actually possible to bind with borrowed data.
| T: Type<DB>, | ||
| T: ToOwned, | ||
| T: ?Sized, |
There was a problem hiding this comment.
Weird way to style this. The requirements for a given type parameter should be grouped together.
| T: Type<DB>, | |
| T: ToOwned, | |
| T: ?Sized, | |
| T: Type<DB> + ToOwned + ?Sized, |
| T: Type<DB>, | ||
| T: ?Sized, |
There was a problem hiding this comment.
| T: Type<DB>, | |
| T: ?Sized, | |
| T: Type<DB> + ?Sized, |
fixes #3100
Implements
Decode,EncodeandTypeforBox<T>,Arc<T>,Cow<'_,T>andRc<T>. I left out theDecodeimpl forRc<T>because of theSendtrait bounds inQueryAsandQueryScalar(which makes it impossible to useRc).