Skip to content

Delegate encode_format to T in &T and Option<T> #928

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 3 commits into from
Aug 4, 2022

Conversation

hmac
Copy link
Contributor

@hmac hmac commented Aug 4, 2022

#836 has been really useful for my use case, and this PR makes some small improvements to it.

I have a custom type (call it T) that impls ToSql, and I override encode_format. However in some situations I'm working with &T or Option<T> and I want to_sql to use the format that I've specified in encode_format. This change ensures that by delegating to T::encode_format in these cases.

As well as being useful, I believe this fixes a correctness issue, as these types already delegate to their inner type to implement to_sql, so they should do the same for encode_format to avoid emitting, e.g., a text encoding that is tagged as binary.

I've also added [derive(Clone, Copy, Debug)] to Format, as it's a simple enum and this makes working with it a bit easier.

hmac added 3 commits August 3, 2022 23:32
It's useful to be able to inspect Format values when debugging.
In the ToSql impls for &T and Option<T>, override encode_format to
delegate to the impl for T. This ensures that if T overrides this
method, it also overrides it for &T and Option<T>.
@sfackler sfackler merged commit d650adb into sfackler:master Aug 4, 2022
@sfackler
Copy link
Owner

sfackler commented Aug 4, 2022

Thanks!

@hmac hmac deleted the hmac/encode-format branch August 4, 2022 23:56
@hmac hmac restored the hmac/encode-format branch August 4, 2022 23:56
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.

2 participants