Skip to content

export Bytes type through DataRowBody::storage_bytes method. #1000

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.

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

Merged
merged 1 commit into from
Feb 26, 2023

Conversation

fakeshadow
Copy link
Contributor

@fakeshadow fakeshadow commented Feb 24, 2023

Exposing Bytes type can be beneficial as #996 suggested.

Regarding high level usage it can be introduced as following pesudo api:

impl Row {
    fn get2<I, T>(&self, idx: &I) -> Result<T, Error>
    where
        I: RowIndex + fmt::Display,
        T: FromSqlOwned,
    {
        ...
    }
}

trait FromSqlOwned {
  fn from_sql(ty: &Type, raw: &Bytes) -> Result<Self, Box<dyn Error + Sync + Send>>;
}

// new type of Bytes with utf8 encoding check for cheap clone.
struct ByteString(Bytes);

impl FromSqlOwned for ByteString {
  fn from_sql(ty: &Type, raw: &Bytes) -> Result<Self, Box<dyn Error + Sync + Send>> {
    std::str::from_utf8(raw.as_ref())?
    ByteString(raw.clone())  
  }
}

Closes #996

@fakeshadow
Copy link
Contributor Author

Regarding changelog should I add an unreleased session on top?

@sfackler sfackler merged commit 3007dd8 into sfackler:master Feb 26, 2023
@sfackler
Copy link
Owner

Nope, no need.

@fakeshadow fakeshadow deleted the protocol/export_bytes branch February 27, 2023 00:59
@ParkMyCar
Copy link

I see the fn buffer_bytes(...) method now exists on DataRowBody, has anything like the high level interface been implemented yet? Given a Row I don't believe you can access the DataRowBody given it's a private field and looking through the docs there don't appear to be any accessors.

My goal is to get zero-copy accessing of a bytea column, similar to what's mentioned in #996. More than happy to submit a PR if you'd be open to something like the high level usage proposed here @sfackler?

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

Successfully merging this pull request may close these issues.

Consider expose Bytes type trhough postgres-protocol?
3 participants