Skip to content
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.

Sign up for GitHub

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

Implement Decode, Encode and Type for Box, Arc, Cow and Rc #3674

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Implement Decode, Encode and Type for Box, Arc, Cow and Rc #3674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9d1447a
feat: implement Decode,Encode,Type for Box,Arc,Cow
joeydewaal Jan 9, 2025
f6122de
feat implement Encode,Type for Rc
joeydewaal Jan 9, 2025
14f7013
feat: implement Decode for Rc
joeydewaal Mar 2, 2025
a66d0eb
chore: make tests more concise
joeydewaal Mar 2, 2025
82f2ccd
chore: use macro's
joeydewaal Mar 2, 2025
da34017
chore: remove conflicting impls
joeydewaal Mar 2, 2025
d0e4f39
chore: more macro's
joeydewaal Mar 3, 2025
3857c69
Relax Sized bound for Decode, Encode
joeydewaal Mar 12, 2025
308a03e
update unit tests
joeydewaal Mar 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions sqlx-core/src/decode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
//! Provides [`Decode`] for decoding values from the database.

use std::borrow::Cow;
use std::rc::Rc;
use std::sync::Arc;

use crate::database::Database;
use crate::error::BoxDynError;

Expand Down Expand Up @@ -77,3 +81,78 @@ where
}
}
}

macro_rules! impl_decode_for_smartpointer {
($smart_pointer:tt) => {
impl<'r, DB, T> Decode<'r, DB> for $smart_pointer<T>
where
DB: Database,
T: Decode<'r, DB>,
{
fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> {
Ok(Self::new(T::decode(value)?))
}
}

impl<'r, DB> Decode<'r, DB> for $smart_pointer<str>
where
DB: Database,
&'r str: Decode<'r, DB>,
{
fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> {
let ref_str = <&str as Decode<DB>>::decode(value)?;
Ok(ref_str.into())
}
}

impl<'r, DB> Decode<'r, DB> for $smart_pointer<[u8]>
where
DB: Database,
&'r [u8]: Decode<'r, DB>,
{
fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> {
let ref_str = <&[u8] as Decode<DB>>::decode(value)?;
Ok(ref_str.into())
}
}
};
}

impl_decode_for_smartpointer!(Arc);
impl_decode_for_smartpointer!(Box);
impl_decode_for_smartpointer!(Rc);

// implement `Decode` for Cow<T> for all SQL types
impl<'r, DB, T> Decode<'r, DB> for Cow<'_, T>
where
DB: Database,
T: ToOwned,
<T as ToOwned>::Owned: Decode<'r, DB>,
{
fn decode(value: <DB as Database>::ValueRef<'r>) -> Result<Self, BoxDynError> {
let owned = <<T as ToOwned>::Owned as Decode<DB>>::decode(value)?;
Ok(Cow::Owned(owned))
}
}

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))
}
}
Comment on lines +138 to +158
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?).

71 changes: 71 additions & 0 deletions sqlx-core/src/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
//! Provides [`Encode`] for encoding values for the database.

use std::borrow::Cow;
use std::mem;
use std::rc::Rc;
use std::sync::Arc;

use crate::database::Database;
use crate::error::BoxDynError;
Expand Down Expand Up @@ -129,3 +132,71 @@ macro_rules! impl_encode_for_option {
}
};
}

macro_rules! impl_encode_for_smartpointer {
($smart_pointer:ty) => {
impl<'q, T, DB: Database> Encode<'q, DB> for $smart_pointer
where
T: Encode<'q, DB>,
{
#[inline]
fn encode(
self,
buf: &mut <DB as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
<T as Encode<DB>>::encode_by_ref(self.as_ref(), buf)
}

#[inline]
fn encode_by_ref(
&self,
buf: &mut <DB as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
<&T as Encode<DB>>::encode(self, buf)
}

#[inline]
fn produces(&self) -> Option<DB::TypeInfo> {
(**self).produces()
}

#[inline]
fn size_hint(&self) -> usize {
(**self).size_hint()
}
}
};
}

impl_encode_for_smartpointer!(Arc<T>);
impl_encode_for_smartpointer!(Box<T>);
impl_encode_for_smartpointer!(Rc<T>);

impl<'q, T, DB: Database> Encode<'q, DB> for Cow<'_, T>
where
T: Encode<'q, DB>,
Comment on lines +175 to +177
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might get weird with non-'static lifetimes. I would test that it's actually possible to bind with borrowed data.

T: ToOwned,
Comment on lines +177 to +178
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
T: Encode<'q, DB>,
T: ToOwned,
T: Encode<'q, DB> + ToOwned,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could possibly use a comment about how ToOwned is only required to satisfy Cow.

{
#[inline]
fn encode(self, buf: &mut <DB as Database>::ArgumentBuffer<'q>) -> Result<IsNull, BoxDynError> {
<&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
}

#[inline]
fn encode_by_ref(
&self,
buf: &mut <DB as Database>::ArgumentBuffer<'q>,
) -> Result<IsNull, BoxDynError> {
<&T as Encode<DB>>::encode_by_ref(&self.as_ref(), buf)
}

#[inline]
fn produces(&self) -> Option<DB::TypeInfo> {
<&T as Encode<DB>>::produces(&self.as_ref())
}

#[inline]
fn size_hint(&self) -> usize {
<&T as Encode<DB>>::size_hint(&self.as_ref())
}
}
39 changes: 39 additions & 0 deletions sqlx-core/src/types/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
//! To represent nullable SQL types, `Option<T>` is supported where `T` implements `Type`.
//! An `Option<T>` represents a potentially `NULL` value from SQL.

use std::{borrow::Cow, rc::Rc, sync::Arc};

use crate::database::Database;
use crate::type_info::TypeInfo;

Expand Down Expand Up @@ -241,3 +243,40 @@ impl<T: Type<DB>, DB: Database> Type<DB> for Option<T> {
ty.is_null() || <T as Type<DB>>::compatible(ty)
}
}

macro_rules! impl_type_for_smartpointer {
($smart_pointer:ty) => {
impl<T, DB: Database> Type<DB> for $smart_pointer
where
T: Type<DB>,
T: ?Sized,
Comment on lines +251 to +252
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
T: Type<DB>,
T: ?Sized,
T: Type<DB> + ?Sized,

{
fn type_info() -> DB::TypeInfo {
<T as Type<DB>>::type_info()
}

fn compatible(ty: &DB::TypeInfo) -> bool {
<T as Type<DB>>::compatible(ty)
}
}
};
}

impl_type_for_smartpointer!(Arc<T>);
impl_type_for_smartpointer!(Box<T>);
impl_type_for_smartpointer!(Rc<T>);

impl<T, DB: Database> Type<DB> for Cow<'_, T>
where
T: Type<DB>,
T: ToOwned,
T: ?Sized,
Comment on lines +271 to +273
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird way to style this. The requirements for a given type parameter should be grouped together.

Suggested change
T: Type<DB>,
T: ToOwned,
T: ?Sized,
T: Type<DB> + ToOwned + ?Sized,

{
fn type_info() -> DB::TypeInfo {
<T as Type<DB>>::type_info()
}

fn compatible(ty: &DB::TypeInfo) -> bool {
<T as Type<DB>>::compatible(ty)
}
}
16 changes: 0 additions & 16 deletions sqlx-mysql/src/types/bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,28 +40,12 @@ impl<'r> Decode<'r, MySql> for &'r [u8] {
}
}

impl Type<MySql> for Box<[u8]> {
fn type_info() -> MySqlTypeInfo {
<&[u8] as Type<MySql>>::type_info()
}

fn compatible(ty: &MySqlTypeInfo) -> bool {
<&[u8] as Type<MySql>>::compatible(ty)
}
}

impl Encode<'_, MySql> for Box<[u8]> {
fn encode_by_ref(&self, buf: &mut Vec<u8>) -> Result<IsNull, BoxDynError> {
<&[u8] as Encode<MySql>>::encode(self.as_ref(), buf)
}
}

impl<'r> Decode<'r, MySql> for Box<[u8]> {
fn decode(value: MySqlValueRef<'r>) -> Result<Self, BoxDynError> {
<&[u8] as Decode<MySql>>::decode(value).map(Box::from)
}
}

impl Type<MySql> for Vec<u8> {
fn type_info() -> MySqlTypeInfo {
<[u8] as Type<MySql>>::type_info()
Expand Down
35 changes: 5 additions & 30 deletions sqlx-mysql/src/types/str.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use std::borrow::Cow;

use crate::decode::Decode;
use crate::encode::{Encode, IsNull};
use crate::error::BoxDynError;
use crate::io::MySqlBufMutExt;
use crate::protocol::text::{ColumnFlags, ColumnType};
use crate::types::Type;
use crate::{MySql, MySqlTypeInfo, MySqlValueRef};
use std::borrow::Cow;

impl Type<MySql> for str {
fn type_info() -> MySqlTypeInfo {
Expand Down Expand Up @@ -46,28 +47,12 @@ impl<'r> Decode<'r, MySql> for &'r str {
}
}

impl Type<MySql> for Box<str> {
fn type_info() -> MySqlTypeInfo {
<&str as Type<MySql>>::type_info()
}

fn compatible(ty: &MySqlTypeInfo) -> bool {
<&str as Type<MySql>>::compatible(ty)
}
}

impl Encode<'_, MySql> for Box<str> {
fn encode_by_ref(&self, buf: &mut Vec<u8>) -> Result<IsNull, BoxDynError> {
<&str as Encode<MySql>>::encode(&**self, buf)
}
}

impl<'r> Decode<'r, MySql> for Box<str> {
fn decode(value: MySqlValueRef<'r>) -> Result<Self, BoxDynError> {
<&str as Decode<MySql>>::decode(value).map(Box::from)
}
}

impl Type<MySql> for String {
fn type_info() -> MySqlTypeInfo {
<str as Type<MySql>>::type_info()
Expand All @@ -90,16 +75,6 @@ impl Decode<'_, MySql> for String {
}
}

impl Type<MySql> for Cow<'_, str> {
fn type_info() -> MySqlTypeInfo {
<&str as Type<MySql>>::type_info()
}

fn compatible(ty: &MySqlTypeInfo) -> bool {
<&str as Type<MySql>>::compatible(ty)
}
}

impl Encode<'_, MySql> for Cow<'_, str> {
fn encode_by_ref(&self, buf: &mut Vec<u8>) -> Result<IsNull, BoxDynError> {
match self {
Expand All @@ -109,8 +84,8 @@ impl Encode<'_, MySql> for Cow<'_, str> {
}
}

impl<'r> Decode<'r, MySql> for Cow<'r, str> {
fn decode(value: MySqlValueRef<'r>) -> Result<Self, BoxDynError> {
value.as_str().map(Cow::Borrowed)
impl Encode<'_, MySql> for Cow<'_, [u8]> {
fn encode_by_ref(&self, buf: &mut Vec<u8>) -> Result<IsNull, BoxDynError> {
<&[u8] as Encode<MySql>>::encode(self.as_ref(), buf)
}
}
9 changes: 0 additions & 9 deletions sqlx-postgres/src/types/bytes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,15 +80,6 @@ fn text_hex_decode_input(value: PgValueRef<'_>) -> Result<&[u8], BoxDynError> {
.map_err(Into::into)
}

impl Decode<'_, Postgres> for Box<[u8]> {
fn decode(value: PgValueRef<'_>) -> Result<Self, BoxDynError> {
Ok(match value.format() {
PgValueFormat::Binary => Box::from(value.as_bytes()?),
PgValueFormat::Text => Box::from(hex::decode(text_hex_decode_input(value)?)?),
})
}
}

impl Decode<'_, Postgres> for Vec<u8> {
fn decode(value: PgValueRef<'_>) -> Result<Self, BoxDynError> {
Ok(match value.format() {
Expand Down
Loading
Loading