Skip to content

Commit e2608a4

Browse files
sgrifsgrif
committed
Turn to_sql into a standard AST pass
We're not quite to the point where we have a single uniform function, as we still need to eliminate some of the branches that come from the existence of `Changeset` and `InsertValues`. I've been wanting to introduce a `context` parameter that gets passed around to enable things like expression index support in PG upsert, and I suspect that this will fix the problem. I suspect this will alter our performance characteristics in a few cases (particularly when the query is boxed). I don't expect any major regressions from this, but I do want to see if it's worth sticking `#[inline]` in a few places (or even `#[inline(always)]` in some particularly hot paths like `SelectStatement` and tuples)
1 parent 2599ea7 commit e2608a4

45 files changed

Lines changed: 250 additions & 643 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎diesel/src/connection/statement_cache.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,10 +225,8 @@ impl<DB> StatementCacheKey<DB> where
225225
}
226226

227227
fn construct_sql<T: QueryFragment<DB>>(source: &T) -> QueryResult<String> {
228-
use result::Error::QueryBuilderError;
229-
230228
let mut query_builder = DB::QueryBuilder::default();
231-
try!(source.to_sql(&mut query_builder).map_err(QueryBuilderError));
229+
try!(source.to_sql(&mut query_builder));
232230
Ok(query_builder.finish())
233231
}
234232
}

‎diesel/src/expression/array_comparison.rs‎

Lines changed: 15 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -67,47 +67,35 @@ impl<T, U, DB> QueryFragment<DB> for In<T, U> where
6767
T: QueryFragment<DB>,
6868
U: QueryFragment<DB> + MaybeEmpty,
6969
{
70-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
70+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
7171
if self.values.is_empty() {
7272
out.push_sql("1=0");
7373
} else {
74-
try!(self.left.to_sql(out));
74+
self.left.walk_ast(out.reborrow())?;
7575
out.push_sql(" IN (");
76-
try!(self.values.to_sql(out));
76+
self.values.walk_ast(out.reborrow())?;
7777
out.push_sql(")");
7878
}
7979
Ok(())
8080
}
81-
82-
fn walk_ast(&self, mut pass: AstPass<DB>) -> QueryResult<()> {
83-
self.left.walk_ast(pass.reborrow())?;
84-
self.values.walk_ast(pass.reborrow())?;
85-
Ok(())
86-
}
8781
}
8882

8983
impl<T, U, DB> QueryFragment<DB> for NotIn<T, U> where
9084
DB: Backend,
9185
T: QueryFragment<DB>,
9286
U: QueryFragment<DB> + MaybeEmpty,
9387
{
94-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
88+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
9589
if self.values.is_empty() {
9690
out.push_sql("1=1");
9791
} else {
98-
try!(self.left.to_sql(out));
92+
self.left.walk_ast(out.reborrow())?;
9993
out.push_sql(" NOT IN (");
100-
try!(self.values.to_sql(out));
94+
self.values.walk_ast(out.reborrow())?;
10195
out.push_sql(")");
10296
}
10397
Ok(())
10498
}
105-
106-
fn walk_ast(&self, mut pass: AstPass<DB>) -> QueryResult<()> {
107-
self.left.walk_ast(pass.reborrow())?;
108-
self.values.walk_ast(pass.reborrow())?;
109-
Ok(())
110-
}
11199
}
112100

113101
impl_query_id!(In<T, U>);
@@ -193,19 +181,16 @@ impl<T, DB> QueryFragment<DB> for Many<T> where
193181
DB: Backend,
194182
T: QueryFragment<DB>,
195183
{
196-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
197-
try!(self.0[0].to_sql(out));
198-
for value in self.0[1..].iter() {
199-
out.push_sql(", ");
200-
try!(value.to_sql(out));
201-
}
202-
Ok(())
203-
}
204-
205-
fn walk_ast(&self, mut pass: AstPass<DB>) -> QueryResult<()> {
206-
pass.unsafe_to_cache_prepared();
184+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
185+
out.unsafe_to_cache_prepared();
186+
let mut first = true;
207187
for value in &self.0 {
208-
value.walk_ast(pass.reborrow())?;
188+
if first {
189+
first = false;
190+
} else {
191+
out.push_sql(", ");
192+
}
193+
value.walk_ast(out.reborrow())?;
209194
}
210195
Ok(())
211196
}
@@ -245,10 +230,6 @@ impl<T, ST, DB> QueryFragment<DB> for Subselect<T, ST> where
245230
DB: Backend,
246231
T: QueryFragment<DB>,
247232
{
248-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
249-
self.values.to_sql(out)
250-
}
251-
252233
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
253234
self.values.walk_ast(pass)
254235
}

‎diesel/src/expression/bound.rs‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,6 @@ impl<T, U, DB> QueryFragment<DB> for Bound<T, U> where
2929
DB: Backend + HasSqlType<T>,
3030
U: ToSql<T, DB>,
3131
{
32-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
33-
out.push_bind_param();
34-
Ok(())
35-
}
36-
3732
fn walk_ast(&self, mut pass: AstPass<DB>) -> QueryResult<()> {
3833
pass.push_bind_param(&self.item)?;
3934
Ok(())

‎diesel/src/expression/coerce.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,6 @@ impl<T, ST, DB> QueryFragment<DB> for Coerce<T, ST> where
5252
T: QueryFragment<DB>,
5353
DB: Backend,
5454
{
55-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
56-
self.expr.to_sql(out)
57-
}
58-
5955
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
6056
self.expr.walk_ast(pass)
6157
}

‎diesel/src/expression/count.rs‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -81,16 +81,12 @@ impl<T: Expression> Expression for Count<T> {
8181
}
8282

8383
impl<T: QueryFragment<DB>, DB: Backend> QueryFragment<DB> for Count<T> {
84-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
84+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
8585
out.push_sql("COUNT(");
86-
try!(self.target.to_sql(out));
86+
self.target.walk_ast(out.reborrow())?;
8787
out.push_sql(")");
8888
Ok(())
8989
}
90-
91-
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
92-
self.target.walk_ast(pass)
93-
}
9490
}
9591

9692
impl_query_id!(Count<T>);
@@ -105,14 +101,10 @@ impl Expression for CountStar {
105101
}
106102

107103
impl<DB: Backend> QueryFragment<DB> for CountStar {
108-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
104+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
109105
out.push_sql("COUNT(*)");
110106
Ok(())
111107
}
112-
113-
fn walk_ast(&self, _: AstPass<DB>) -> QueryResult<()> {
114-
Ok(())
115-
}
116108
}
117109

118110
impl_query_id!(CountStar);

‎diesel/src/expression/exists.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,12 @@ impl<T, DB> QueryFragment<DB> for Exists<T> where
5656
DB: Backend,
5757
T: QueryFragment<DB>,
5858
{
59-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
59+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
6060
out.push_sql("EXISTS (");
61-
try!(self.0.to_sql(out));
61+
self.0.walk_ast(out.reborrow())?;
6262
out.push_sql(")");
6363
Ok(())
6464
}
65-
66-
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
67-
self.0.walk_ast(pass)?;
68-
Ok(())
69-
}
7065
}
7166

7267
impl_query_id!(Exists<T>);

‎diesel/src/expression/functions/aggregate_folding.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,12 @@ macro_rules! fold_function {
3232
T: Expression + QueryFragment<DB>,
3333
DB: Backend + HasSqlType<T::SqlType>,
3434
{
35-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
35+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
3636
out.push_sql(concat!($operator, "("));
37-
try!(self.target.to_sql(out));
37+
self.target.walk_ast(out.reborrow())?;
3838
out.push_sql(")");
3939
Ok(())
4040
}
41-
42-
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
43-
self.target.walk_ast(pass)?;
44-
Ok(())
45-
}
4641
}
4742

4843
impl_query_id!($type_name<T>);

‎diesel/src/expression/functions/aggregate_ordering.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,12 @@ macro_rules! ord_function {
3131
T: Expression + QueryFragment<DB>,
3232
DB: Backend + HasSqlType<T::SqlType>,
3333
{
34-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
34+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
3535
out.push_sql(concat!($operator, "("));
36-
try!(self.target.to_sql(out));
36+
self.target.walk_ast(out.reborrow())?;
3737
out.push_sql(")");
3838
Ok(())
3939
}
40-
41-
fn walk_ast(&self, pass: AstPass<DB>) -> QueryResult<()> {
42-
self.target.walk_ast(pass)?;
43-
Ok(())
44-
}
4540
}
4641

4742
impl_query_id!($type_name<T>);

‎diesel/src/expression/functions/date_and_time.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,10 @@ impl NonAggregate for now {
1818
}
1919

2020
impl<DB: Backend> QueryFragment<DB> for now {
21-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> BuildQueryResult {
21+
fn walk_ast(&self, mut out: AstPass<DB>) -> QueryResult<()> {
2222
out.push_sql("CURRENT_TIMESTAMP");
2323
Ok(())
2424
}
25-
26-
fn walk_ast(&self, _: AstPass<DB>) -> QueryResult<()> {
27-
Ok(())
28-
}
2925
}
3026

3127
impl_query_id!(now);

‎diesel/src/expression/functions/mod.rs‎

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,13 @@ macro_rules! sql_function_body {
3838
DB: $crate::backend::Backend,
3939
for <'a> ($(&'a $arg_name),*): $crate::query_builder::QueryFragment<DB>,
4040
{
41-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> $crate::query_builder::BuildQueryResult {
42-
use $crate::query_builder::QueryBuilder;
41+
fn walk_ast(&self, mut out: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
4342
out.push_sql(concat!(stringify!($fn_name), "("));
44-
try!($crate::query_builder::QueryFragment::to_sql(
45-
&($(&self.$arg_name),*), out));
43+
$crate::query_builder::QueryFragment::walk_ast(
44+
&($(&self.$arg_name),*), out.reborrow())?;
4645
out.push_sql(")");
4746
Ok(())
4847
}
49-
50-
fn walk_ast(&self, pass: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
51-
try!($crate::query_builder::QueryFragment::walk_ast(
52-
&($(&self.$arg_name),*), pass));
53-
Ok(())
54-
}
5548
}
5649

5750
impl_query_id!($struct_name<$($arg_name),+>);
@@ -156,15 +149,10 @@ macro_rules! no_arg_sql_function_body {
156149
impl<DB> $crate::query_builder::QueryFragment<DB> for $type_name where
157150
DB: $crate::backend::Backend + $($constraint)::+,
158151
{
159-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> $crate::query_builder::BuildQueryResult {
160-
use $crate::query_builder::QueryBuilder;
152+
fn walk_ast(&self, mut out: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
161153
out.push_sql(concat!(stringify!($type_name), "()"));
162154
Ok(())
163155
}
164-
165-
fn walk_ast(&self, _: &mut $crate::query_builder::AstPass<DB>) -> QueryResult<()> {
166-
Ok(())
167-
}
168156
}
169157
};
170158

@@ -174,15 +162,10 @@ macro_rules! no_arg_sql_function_body {
174162
impl<DB> $crate::query_builder::QueryFragment<DB> for $type_name where
175163
DB: $crate::backend::Backend,
176164
{
177-
fn to_sql(&self, out: &mut DB::QueryBuilder) -> $crate::query_builder::BuildQueryResult {
178-
use $crate::query_builder::QueryBuilder;
165+
fn walk_ast(&self, mut out: $crate::query_builder::AstPass<DB>) -> $crate::result::QueryResult<()> {
179166
out.push_sql(concat!(stringify!($type_name), "()"));
180167
Ok(())
181168
}
182-
183-
fn walk_ast(&self, _: $crate::query_builder::AstPass<DB>) -> QueryResult<()> {
184-
Ok(())
185-
}
186169
}
187170
};
188171
}

0 commit comments

Comments
 (0)