Skip to content

Commit fdefafa

Browse files
committed
Remove the associated type from SelectableExpression
This removes the flawed "SQL type varies based on the query source" design in favor of "things become nullable if they're on the wrong side of an outer join". This means that columns from the right side of a left join can no longer directly be selected. `.nullable` must be manually called. This sounds like a huge deal, but that's already the case today with tuples (which is a more common case), and it hasn't caused a huge ergonomics issue. Ultimately most queries just use the default select clause. This also causes the nullability to bubble up. If you're building a compound expression that involves a column from the right side of a left join, you don't have to worry about that in the compound expression. You just have to make the whole thing nullable at the top level. This mirrors SQL's semantics quite nicely. One downside of this is that `.nullable` can no longer be used in arbitrary select clauses. Doing so is questionably useful at best, but I'd still like to bring back that support in the future. Ultimately doing so more requires rust-lang/rust#40097 or similar.
1 parent 93e8b96 commit fdefafa

File tree

25 files changed

+88
-130
lines changed

25 files changed

+88
-130
lines changed

diesel/src/expression/array_comparison.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ impl<T, QS> SelectableExpression<QS> for Many<T> where
191191
Many<T>: AppearsOnTable<QS>,
192192
T: SelectableExpression<QS>,
193193
{
194-
type SqlTypeForSelect = T::SqlTypeForSelect;
195194
}
196195

197196
impl<T, QS> AppearsOnTable<QS> for Many<T> where
@@ -247,7 +246,6 @@ impl<T, ST, QS> SelectableExpression<QS> for Subselect<T, ST> where
247246
Subselect<T, ST>: AppearsOnTable<QS>,
248247
T: Query,
249248
{
250-
type SqlTypeForSelect = ST;
251249
}
252250

253251
impl<T, ST, QS> AppearsOnTable<QS> for Subselect<T, ST> where

diesel/src/expression/bound.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ impl<T: QueryId, U> QueryId for Bound<T, U> {
6666
impl<T, U, QS> SelectableExpression<QS> for Bound<T, U> where
6767
Bound<T, U>: AppearsOnTable<QS>,
6868
{
69-
type SqlTypeForSelect = T;
7069
}
7170

7271
impl<T, U, QS> AppearsOnTable<QS> for Bound<T, U> where

diesel/src/expression/coerce.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ impl<T, ST> Expression for Coerce<T, ST> where
4141
impl<T, ST, QS> SelectableExpression<QS> for Coerce<T, ST> where
4242
T: SelectableExpression<QS>,
4343
{
44-
type SqlTypeForSelect = Self::SqlType;
4544
}
4645

4746
impl<T, ST, QS> AppearsOnTable<QS> for Coerce<T, ST> where

diesel/src/expression/expression_methods/global_expression_methods.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ pub trait ExpressionMethods: Expression + Sized {
286286
/// # fn main() {
287287
/// # use self::users::dsl::*;
288288
/// # let order = "name";
289-
/// let ordering: Box<BoxableExpression<users, DB, SqlType=(), SqlTypeForSelect=()>> =
289+
/// let ordering: Box<BoxableExpression<users, DB, SqlType=()>> =
290290
/// if order == "name" {
291291
/// Box::new(name.desc())
292292
/// } else {

diesel/src/expression/functions/mod.rs

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,13 +62,9 @@ macro_rules! sql_function_body {
6262

6363
#[allow(non_camel_case_types)]
6464
impl<$($arg_name),*, QS> $crate::expression::SelectableExpression<QS> for $struct_name<$($arg_name),*> where
65-
$($arg_name: $crate::expression::SelectableExpression<
66-
QS,
67-
SqlTypeForSelect = <$arg_name as $crate::expression::Expression>::SqlType,
68-
>,)*
65+
$($arg_name: $crate::expression::SelectableExpression<QS>,)*
6966
$struct_name<$($arg_name),*>: $crate::expression::AppearsOnTable<QS>,
7067
{
71-
type SqlTypeForSelect = Self::SqlType;
7268
}
7369

7470
#[allow(non_camel_case_types)]
@@ -143,7 +139,6 @@ macro_rules! no_arg_sql_function_body_except_to_sql {
143139
}
144140

145141
impl<QS> $crate::expression::SelectableExpression<QS> for $type_name {
146-
type SqlTypeForSelect = $return_type;
147142
}
148143

149144
impl<QS> $crate::expression::AppearsOnTable<QS> for $type_name {

diesel/src/expression/mod.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,30 +123,27 @@ impl<'a, T: ?Sized, QS> AppearsOnTable<QS> for &'a T where
123123
{
124124
}
125125

126-
/// Indicates that an expression can be selected from a source. The associated
127-
/// type is usually the same as `Expression::SqlType`, but is used to indicate
128-
/// that a column is always nullable when it appears on the right side of a left
129-
/// outer join, even if it wasn't nullable to begin with.
126+
/// Indicates that an expression can be selected from a source. Columns will
127+
/// implement this for their table. Certain special types, like `CountStar` and
128+
/// `Bound` will implement this for all sources. Most compound expressions will
129+
/// implement this if each of their parts implement it.
130130
///
131-
/// Columns will implement this for their table. Certain special types, like
132-
/// `CountStar` and `Bound` will implement this for all sources. All other
133-
/// expressions will inherit this from their children.
131+
/// Notably, columns will not implement this trait for the right side of a left
132+
/// join. To select a column or expression using a column from the right side of
133+
/// a left join, you must call `.nullable()` on it.
134134
pub trait SelectableExpression<QS: ?Sized>: AppearsOnTable<QS> {
135-
type SqlTypeForSelect;
136135
}
137136

138137
impl<T: ?Sized, QS> SelectableExpression<QS> for Box<T> where
139138
T: SelectableExpression<QS>,
140139
Box<T>: AppearsOnTable<QS>,
141140
{
142-
type SqlTypeForSelect = T::SqlTypeForSelect;
143141
}
144142

145143
impl<'a, T: ?Sized, QS> SelectableExpression<QS> for &'a T where
146144
T: SelectableExpression<QS>,
147145
&'a T: AppearsOnTable<QS>,
148146
{
149-
type SqlTypeForSelect = T::SqlTypeForSelect;
150147
}
151148

152149
/// Marker trait to indicate that an expression does not include any aggregate
@@ -184,7 +181,7 @@ impl<QS, T, DB> BoxableExpression<QS, DB> for T where
184181
{
185182
}
186183

187-
impl<QS, ST, STS, DB> QueryId for BoxableExpression<QS, DB, SqlType=ST, SqlTypeForSelect=STS> {
184+
impl<QS, ST, DB> QueryId for BoxableExpression<QS, DB, SqlType=ST> {
188185
type QueryId = ();
189186

190187
fn has_static_query_id() -> bool {

diesel/src/expression/nullable.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,16 +37,8 @@ impl<T, DB> QueryFragment<DB> for Nullable<T> where
3737
}
3838
}
3939

40-
/// This impl relies on the fact that the only time `T::SqlType` will differ
41-
/// from `T::SqlTypeForSelect` is to make the right side of a left join become
42-
/// nullable.
43-
impl<T, QS> SelectableExpression<QS> for Nullable<T> where
44-
T: SelectableExpression<QS>,
45-
Nullable<T>: AppearsOnTable<QS>,
46-
{
47-
type SqlTypeForSelect = Self::SqlType;
48-
}
49-
40+
/// Nullable can be used in where clauses everywhere, but can only be used in
41+
/// select clauses for outer joins.
5042
impl<T, QS> AppearsOnTable<QS> for Nullable<T> where
5143
T: AppearsOnTable<QS>,
5244
Nullable<T>: Expression,

diesel/src/expression/sql_literal.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ impl<ST> Query for SqlLiteral<ST> {
5252
}
5353

5454
impl<QS, ST> SelectableExpression<QS> for SqlLiteral<ST> {
55-
type SqlTypeForSelect = ST;
5655
}
5756

5857
impl<QS, ST> AppearsOnTable<QS> for SqlLiteral<ST> {

diesel/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ pub mod helper_types {
7676
use super::expression::helper_types::Eq;
7777

7878
/// Represents the return type of `.select(selection)`
79-
pub type Select<Source, Selection, Type = <Selection as super::Expression>::SqlType> =
80-
<Source as SelectDsl<Selection, Type>>::Output;
79+
pub type Select<Source, Selection> =
80+
<Source as SelectDsl<Selection>>::Output;
8181

8282
/// Represents the return type of `.filter(predicate)`
8383
pub type Filter<Source, Predicate> =

diesel/src/macros/internal.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,8 @@ macro_rules! impl_selectable_expression {
2222
impl<$($ty_params,)* QS> $crate::expression::SelectableExpression<QS>
2323
for $struct_ty where
2424
$struct_ty: $crate::expression::AppearsOnTable<QS>,
25-
$($ty_params: $crate::expression::SelectableExpression<
26-
QS,
27-
SqlTypeForSelect = <$ty_params as $crate::expression::Expression>::SqlType,
28-
>,)*
25+
$($ty_params: $crate::expression::SelectableExpression<QS>,)*
2926
{
30-
type SqlTypeForSelect = <$struct_ty as $crate::expression::Expression>::SqlType;
3127
}
3228

3329
impl<$($ty_params,)* QS> $crate::expression::AppearsOnTable<QS>

0 commit comments

Comments
 (0)