Skip to content

Commit a055d05

Browse files
committed
Remove Expression#to_sql
This method was entirely redundant with `QueryFragment#to_sql`, and effectively only exists because `Expression` predates `QueryFragment`, and I needed to be able to sometimes call `to_insert_sql`. Any cases that couldn't be covered previously have been removed. With this change, our heirarchy is much more clear. `QueryFragment` represents a thing that can be converted to SQL. `Expression` represents something that represents a SQL type. `Query` represents a typed `QueryFragment` that can be meaningfully executed. `Expression` does not have `QueryFragment` as a constraint, but it is implied that implementing one means you'd implement the other. The reason I did not add this constraint was that I ultimately intend to make `QueryFragment` generic over the backend, and I do not want to deal with that when talking purely about an `Expression`. I've tried to loosely audit some of the where clauses to avoid putting too much "type safety" bits in the where clause for `QueryFragment` impls, and focus on "can be converted to SQL". It's certainly not done, but I've tried to at the very least not make it any worse. A lot of this can get cleaned up when I do a pass to attempt to improve specific error messages. I need to explitly document this somewhere, but generally a structure should have no constraints on types in its definition, type safety should be enforced in `Query` or `Expression` impls, and/or DSL methods, and memory/serialization safety should be enforced in `QueryFragment` impls.
1 parent 9321fd8 commit a055d05

35 files changed

Lines changed: 154 additions & 77 deletions

diesel/src/expression/aliased.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,15 @@ impl<'a, Expr> Aliased<'a, Expr> {
1919

2020
pub struct FromEverywhere;
2121

22-
impl<'a, T> Expression for Aliased<'a, T> where T: Expression {
22+
impl<'a, T> Expression for Aliased<'a, T> where
23+
T: Expression,
24+
{
2325
type SqlType = T::SqlType;
26+
}
2427

28+
impl<'a, T> QueryFragment for Aliased<'a, T> where
29+
T: QueryFragment,
30+
{
2531
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
2632
out.push_identifier(&self.alias)
2733
}
@@ -33,7 +39,7 @@ impl<'a, T, QS> SelectableExpression<QS> for Aliased<'a, T> where
3339
{
3440
}
3541

36-
impl<'a, T: Expression> QuerySource for Aliased<'a, T> {
42+
impl<'a, T: Expression + QueryFragment> QuerySource for Aliased<'a, T> {
3743
fn from_clause(&self, out: &mut QueryBuilder) -> BuildQueryResult {
3844
try!(self.expr.to_sql(out));
3945
out.push_sql(" ");

diesel/src/expression/array_comparison.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::marker::PhantomData;
22

3-
use query_builder::{QueryBuilder, BuildQueryResult};
3+
use query_builder::*;
44
use super::{AsExpression, Expression, SelectableExpression, NonAggregate};
55
use types::{Array, NativeSqlType};
66

@@ -61,7 +61,11 @@ impl<Expr, ST> Expression for Any<Expr, ST> where
6161
Expr: Expression<SqlType=Array<ST>>,
6262
{
6363
type SqlType = ST;
64+
}
6465

66+
impl<Expr, ST> QueryFragment for Any<Expr, ST> where
67+
Expr: QueryFragment,
68+
{
6569
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
6670
out.push_sql("ANY(");
6771
try!(self.expr.to_sql(out));

diesel/src/expression/bound.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@ impl<T: NativeSqlType, U> Bound<T, U> {
1616

1717
impl<T, U> Expression for Bound<T, U> where
1818
T: NativeSqlType,
19-
U: ToSql<T>,
2019
{
2120
type SqlType = T;
21+
}
2222

23+
impl<T, U> QueryFragment for Bound<T, U> where
24+
T: NativeSqlType,
25+
U: ToSql<T>,
26+
{
2327
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
2428
let mut bytes = Vec::new();
2529
match try!(self.item.to_sql(&mut bytes)) {

diesel/src/expression/count.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use query_builder::{QueryBuilder, BuildQueryResult};
1+
use query_builder::*;
22
use super::{Expression, SelectableExpression};
33
use types::BigInt;
44

@@ -46,13 +46,15 @@ pub fn count_star() -> CountStar {
4646

4747
#[derive(Debug, Clone, Copy)]
4848
#[doc(hidden)]
49-
pub struct Count<T: Expression> {
49+
pub struct Count<T> {
5050
target: T,
5151
}
5252

5353
impl<T: Expression> Expression for Count<T> {
5454
type SqlType = BigInt;
55+
}
5556

57+
impl<T: QueryFragment> QueryFragment for Count<T> {
5658
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
5759
out.push_sql("COUNT(");
5860
try!(self.target.to_sql(out));
@@ -70,7 +72,9 @@ pub struct CountStar;
7072

7173
impl Expression for CountStar {
7274
type SqlType = BigInt;
75+
}
7376

77+
impl QueryFragment for CountStar {
7478
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
7579
out.push_sql("COUNT(*)");
7680
Ok(())

diesel/src/expression/date_and_time.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use expression::{Expression, SelectableExpression};
2-
use query_builder::{QueryBuilder, BuildQueryResult};
2+
use query_builder::*;
33
use types::{Timestamp, VarChar};
44

55
pub struct AtTimeZone<Ts, Tz> {
@@ -22,7 +22,12 @@ impl<Ts, Tz> Expression for AtTimeZone<Ts, Tz> where
2222
{
2323
// FIXME: This should be Timestamptz when we support that type
2424
type SqlType = Timestamp;
25+
}
2526

27+
impl<Ts, Tz> QueryFragment for AtTimeZone<Ts, Tz> where
28+
Ts: QueryFragment,
29+
Tz: QueryFragment,
30+
{
2631
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
2732
try!(self.timestamp.to_sql(out));
2833
out.push_sql(" AT TIME ZONE ");
@@ -32,4 +37,6 @@ impl<Ts, Tz> Expression for AtTimeZone<Ts, Tz> where
3237

3338
impl<Ts, Tz, Qs> SelectableExpression<Qs> for AtTimeZone<Ts, Tz> where
3439
AtTimeZone<Ts, Tz>: Expression,
40+
Ts: SelectableExpression<Qs>,
41+
Tz: SelectableExpression<Tz>,
3542
{}

diesel/src/expression/functions/aggregate_ordering.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use expression::{Expression, SelectableExpression};
2-
use query_builder::{QueryBuilder, BuildQueryResult};
2+
use query_builder::*;
33
use types::{SqlOrd, NativeSqlType};
44

55
macro_rules! ord_function {
@@ -15,13 +15,15 @@ macro_rules! ord_function {
1515
}
1616

1717
#[derive(Debug, Clone, Copy)]
18-
pub struct $type_name<T: Expression> {
18+
pub struct $type_name<T> {
1919
target: T,
2020
}
2121

2222
impl<T: Expression> Expression for $type_name<T> {
2323
type SqlType = T::SqlType;
24+
}
2425

26+
impl<T: QueryFragment> QueryFragment for $type_name<T> {
2527
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
2628
out.push_sql(concat!($operator, "("));
2729
try!(self.target.to_sql(out));

diesel/src/expression/functions/mod.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,16 @@ macro_rules! sql_function_body {
3131
for <'a> ($(&'a $arg_name),*): $crate::expression::Expression,
3232
{
3333
type SqlType = $return_type;
34+
}
3435

36+
#[allow(non_camel_case_types)]
37+
impl<$($arg_name),*> $crate::query_builder::QueryFragment for $struct_name<$($arg_name),*> where
38+
for <'a> ($(&'a $arg_name),*): $crate::query_builder::QueryFragment,
39+
{
3540
fn to_sql(&self, out: &mut $crate::query_builder::QueryBuilder)
3641
-> $crate::query_builder::BuildQueryResult {
3742
out.push_sql(concat!(stringify!($fn_name), "("));
38-
try!($crate::expression::Expression::to_sql(
43+
try!($crate::query_builder::QueryFragment::to_sql(
3944
&($(&self.$arg_name),*), out));
4045
out.push_sql(")");
4146
Ok(())
@@ -107,7 +112,9 @@ macro_rules! no_arg_sql_function_body {
107112

108113
impl $crate::expression::Expression for $type_name {
109114
type SqlType = $return_type;
115+
}
110116

117+
impl $crate::query_builder::QueryFragment for $type_name {
111118
fn to_sql(&self, out: &mut $crate::query_builder::QueryBuilder)
112119
-> $crate::query_builder::BuildQueryResult {
113120
out.push_sql(concat!(stringify!($type_name), "()"));

diesel/src/expression/grouped.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use expression::{Expression, SelectableExpression, NonAggregate};
2-
use query_builder::{QueryBuilder, BuildQueryResult};
2+
use query_builder::*;
33

44
pub struct Grouped<T>(pub T);
55

66
impl<T: Expression> Expression for Grouped<T> {
77
type SqlType = T::SqlType;
8+
}
89

10+
impl<T: QueryFragment> QueryFragment for Grouped<T> {
911
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
1012
out.push_sql("(");
1113
try!(self.0.to_sql(out));

diesel/src/expression/mod.rs

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ pub mod dsl {
5858
pub use self::dsl::*;
5959
pub use self::sql_literal::SqlLiteral;
6060

61-
use query_builder::{QueryBuilder, BuildQueryResult};
6261
use types::NativeSqlType;
6362

6463
/// Represents a typed fragment of SQL. Apps should not need to implement this
@@ -69,24 +68,14 @@ use types::NativeSqlType;
6968
/// implementing this directly.
7069
pub trait Expression {
7170
type SqlType: NativeSqlType;
72-
73-
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult;
7471
}
7572

7673
impl<T: Expression + ?Sized> Expression for Box<T> {
7774
type SqlType = T::SqlType;
78-
79-
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
80-
Expression::to_sql(&**self, out)
81-
}
8275
}
8376

8477
impl<'a, T: Expression + ?Sized> Expression for &'a T {
8578
type SqlType = T::SqlType;
86-
87-
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
88-
Expression::to_sql(&**self, out)
89-
}
9079
}
9180

9281
/// Describes how a type can be represented as an expression for a given type.
@@ -152,14 +141,16 @@ impl<T: NonAggregate + ?Sized> NonAggregate for Box<T> {
152141
impl<'a, T: NonAggregate + ?Sized> NonAggregate for &'a T {
153142
}
154143

144+
use query_builder::QueryFragment;
145+
155146
/// Helper trait used when boxing expressions. This exists to work around the
156147
/// fact that Rust will not let us use non-core types as bounds on a trait
157148
/// object (you could not return `Box<Expression+NonAggregate>`)
158-
pub trait BoxableExpression<QS, ST: NativeSqlType>: Expression + SelectableExpression<QS, ST> + NonAggregate {
149+
pub trait BoxableExpression<QS, ST: NativeSqlType>: Expression + SelectableExpression<QS, ST> + NonAggregate + QueryFragment {
159150
}
160151

161152
impl<QS, T, ST> BoxableExpression<QS, ST> for T where
162153
ST: NativeSqlType,
163-
T: Expression + SelectableExpression<QS, ST> + NonAggregate,
154+
T: Expression + SelectableExpression<QS, ST> + NonAggregate + QueryFragment,
164155
{
165156
}

diesel/src/expression/nullable.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use expression::{Expression, SelectableExpression, NonAggregate};
2-
use query_builder::{QueryBuilder, BuildQueryResult};
2+
use query_builder::*;
33
use types::IntoNullable;
44

55
pub struct Nullable<T>(T);
@@ -15,7 +15,11 @@ impl<T> Expression for Nullable<T> where
1515
<T as Expression>::SqlType: IntoNullable,
1616
{
1717
type SqlType = <<T as Expression>::SqlType as IntoNullable>::Nullable;
18+
}
1819

20+
impl<T> QueryFragment for Nullable<T> where
21+
T: QueryFragment,
22+
{
1923
fn to_sql(&self, out: &mut QueryBuilder) -> BuildQueryResult {
2024
self.0.to_sql(out)
2125
}

0 commit comments

Comments
 (0)