Skip to content

Add support for deriving FromSql and ToSql implementations for structs with references #574

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexwl
Copy link

@alexwl alexwl commented Feb 24, 2020

For example, this code:

#[derive(FromSql, ToSql)]
struct Item <'a,'b:'a> {
    name: &'a str,
    data: &'b [u8]
}

now generates

FromSql:

impl <'a> postgres_types::FromSql<'a> for Item<'a,'a> {
    fn from_sql(_type: &postgres_types::Type, buf: &'a [u8])
                -> std::result::Result<Item<'a,'a>,
                                       std::boxed::Box<dyn std::error::Error +
                                                       std::marker::Sync +
                                                       std::marker::Send>> {
        //...
    }
    //...
}

ToSql:

impl <'a, 'b: 'a> postgres_types::ToSql for Item<'a, 'b> {
    fn to_sql(&self,
              _type: &postgres_types::Type,
              buf: &mut postgres_types::private::BytesMut)
              -> std::result::Result<postgres_types::IsNull,
                                     std::boxed::Box<std::error::Error +
                                                     std::marker::Sync +
                                                     std::marker::Send>> {
        //...
    }
    //...
}

@sfackler
Copy link
Owner

Thanks for doing this! I'd kind of prefer to follow serde's lead with how it handles lifetime parameters though - should just require a new field annotation: https://serde.rs/field-attrs.html#borrow.

@alexwl
Copy link
Author

alexwl commented Feb 25, 2020

serde adds bounds on the 'de lifetime of the generated Deserialize impl:

impl<'de: 'a, 'a> Deserialize<'de> for Q<'a>

I updated the derive logic for FromSql. The impl for Item now looks like this:

impl <'de: 'a + 'b, 'a, 'b : 'a> postgres_types::FromSql<'de> for Item<'a,'b> {
    fn from_sql(_type: &postgres_types::Type, buf: &'de [u8])
                -> std::result::Result<Item<'a,'b>,
                                       std::boxed::Box<dyn std::error::Error +
                                                       std::marker::Sync +
                                                       std::marker::Send>> {
        //...
    }
    //...
}

(What is the appropriate lifetime name: 'de, 'buf, 'row, ... ?)

As for the borrow field attribute, does postgres-derive need this?

In serde, fields of type &str and &[u8] are implicitly borrowed from the input data, other types must be borrowed explicitly using the #[serde(borrow)] attribute. FromSql impl can borrow all fields from the buf implicitly if there's no need to support types like Cow<'b, str> or PhantomData<&'c str> (currently there's no FromSql impl for Cow or PhantomData in postgres-types).

@sfackler
Copy link
Owner

I think it's best to be conservative for now and require the annnotation in the same contexts that serde does.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants