Add Cast operator #4024
Add Cast operator #4024
Conversation
Ten0
force-pushed
the
cast_operator
branch
4 times, most recently
from
074fb91
to
5cf4c23
Compare
There was a problem hiding this comment.
I'm generally in favor of adding this to diesel itself. The implementation also looks fine beside the noted minor changes.
Additionally I would appreciate that we add the new expression to the auto_type test module in diesel_derives just to ensure that it is compatible.
| @@ -37,6 +37,7 @@ pub(crate) mod nullable; | |||
| #[macro_use] | |||
| pub(crate) mod operators; | |||
| mod case_when; | |||
| pub mod cast; | |||
There was a problem hiding this comment.
I would like to keep the module private and just reexport the necessary items from the expression module.
| /// Marker trait: this SQL type (`Self`) can be casted to the target SQL type | ||
| /// (`ST`) using `CAST(expr AS target_sql_type)` | ||
| pub trait CastsTo<ST> {} | ||
| impl<ST1, ST2> CastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where ST1: CastsTo<ST2> |
There was a problem hiding this comment.
| impl<ST1, ST2> CastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where ST1: CastsTo<ST2> | |
| impl<ST1, ST2> CastsTo<sql_types::Nullable<ST2>> for sql_types::Nullable<ST1> where ST1: CastsTo<ST2> |
| /// `Self` | ||
| fn sql_type_name() -> &'static str; | ||
| } | ||
| impl<ST, DB> KnownCastSqlTypeName<DB> for sql_types::Nullable<ST> |
There was a problem hiding this comment.
| impl<ST, DB> KnownCastSqlTypeName<DB> for sql_types::Nullable<ST> | |
| impl<ST, DB> KnownCastSqlTypeName<DB> for sql_types::Nullable<ST> |
| #[diagnostic::on_unimplemented( | ||
| note = "In order to use `CAST`, it is necessary that Diesel knows how to express the name \ | ||
| of this type in the given backend.", | ||
| note = "This can be PRed into Diesel if the type is a standard SQL type." |
There was a problem hiding this comment.
We likely want to change this message to something more active like: If you run into this error message and believe that this cast should be supported open a PR that adds that trait implementation there (with link to the source or file path)
| { | ||
| type SqlType = ST; | ||
| } | ||
| impl<E, ST, DB> query_builder::QueryFragment<DB> for Cast<E, ST> |
There was a problem hiding this comment.
| impl<E, ST, DB> query_builder::QueryFragment<DB> for Cast<E, ST> | |
| impl<E, ST, DB> query_builder::QueryFragment<DB> for Cast<E, ST> |
| E: FieldAliasMapper<S>, | ||
| { | ||
| type Out = Cast<<E as FieldAliasMapper<S>>::Out, ST>; | ||
| fn map(self, alias: &query_source::Alias<S>) -> Self::Out { |
There was a problem hiding this comment.
| fn map(self, alias: &query_source::Alias<S>) -> Self::Out { | |
| fn map(self, alias: &query_source::Alias<S>) -> Self::Out { |
Implemented this, then realized I in fact didn't need it, figured that a draft PR would be a good place to put it nonetheless so that we don't lose the done work if it becomes needed in the future.
I'm open to not merging this and even closing the PR as "not needed".
All of this could also be implemented outside of Diesel.