Skip to content

Commit 4ef8c77

Browse files
committed
Improve our compile error messages for some invalid queries
Instead of producing an overflowing trait bound error show a concrete error message what's wrong for certain query combination. This requires to workaround rust-lang/rust#34260 in our single method query dsl implementations: * `GroupByDsl` * `BoxedDsl` * `DistinctDsl` * `DistinctOnDsl` * `LockingDsl`
1 parent a2e21be commit 4ef8c77

File tree

7 files changed

+85
-45
lines changed

7 files changed

+85
-45
lines changed

diesel/src/query_dsl/boxed_dsl.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1+
use crate::dsl;
2+
use crate::expression::TypedExpressionType;
3+
use crate::expression::ValidGrouping;
14
use crate::query_builder::AsQuery;
5+
use crate::query_builder::SelectStatement;
26
use crate::query_source::Table;
7+
use crate::Expression;
38

49
/// The `into_boxed` method
510
///
@@ -13,15 +18,17 @@ pub trait BoxedDsl<'a, DB> {
1318
type Output;
1419

1520
/// See the trait documentation.
16-
fn internal_into_boxed(self) -> Self::Output;
21+
fn internal_into_boxed(self) -> dsl::IntoBoxed<'a, Self, DB>;
1722
}
1823

1924
impl<'a, T, DB> BoxedDsl<'a, DB> for T
2025
where
21-
T: Table + AsQuery,
22-
T::Query: BoxedDsl<'a, DB>,
26+
T: Table + AsQuery<Query = SelectStatement<T>>,
27+
SelectStatement<T>: BoxedDsl<'a, DB>,
28+
T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
29+
T::SqlType: TypedExpressionType,
2330
{
24-
type Output = <T::Query as BoxedDsl<'a, DB>>::Output;
31+
type Output = dsl::IntoBoxed<'a, SelectStatement<T>, DB>;
2532

2633
fn internal_into_boxed(self) -> Self::Output {
2734
self.as_query().internal_into_boxed()

diesel/src/query_dsl/distinct_dsl.rs

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
1+
use crate::dsl;
12
#[cfg(feature = "postgres")]
23
use crate::expression::SelectableExpression;
4+
use crate::expression::TypedExpressionType;
5+
use crate::expression::ValidGrouping;
6+
use crate::query_builder::AsQuery;
7+
use crate::query_builder::SelectStatement;
38
use crate::query_source::Table;
9+
use crate::Expression;
410

511
/// The `distinct` method
612
///
@@ -14,17 +20,18 @@ pub trait DistinctDsl {
1420
type Output;
1521

1622
/// See the trait documentation.
17-
fn distinct(self) -> Self::Output;
23+
fn distinct(self) -> dsl::Distinct<Self>;
1824
}
1925

2026
impl<T> DistinctDsl for T
2127
where
22-
T: Table,
23-
T::Query: DistinctDsl,
28+
T: Table + AsQuery<Query = SelectStatement<T>>,
29+
T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
30+
T::SqlType: TypedExpressionType,
2431
{
25-
type Output = <T::Query as DistinctDsl>::Output;
32+
type Output = dsl::Distinct<SelectStatement<T>>;
2633

27-
fn distinct(self) -> Self::Output {
34+
fn distinct(self) -> dsl::Distinct<SelectStatement<T>> {
2835
self.as_query().distinct()
2936
}
3037
}
@@ -42,19 +49,20 @@ pub trait DistinctOnDsl<Selection> {
4249
type Output;
4350

4451
/// See the trait documentation
45-
fn distinct_on(self, selection: Selection) -> Self::Output;
52+
fn distinct_on(self, selection: Selection) -> dsl::DistinctOn<Self, Selection>;
4653
}
4754

4855
#[cfg(feature = "postgres")]
4956
impl<T, Selection> DistinctOnDsl<Selection> for T
5057
where
5158
Selection: SelectableExpression<T>,
52-
T: Table,
53-
T::Query: DistinctOnDsl<Selection>,
59+
T: Table + AsQuery<Query = SelectStatement<T>>,
60+
T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
61+
T::SqlType: TypedExpressionType,
5462
{
55-
type Output = <T::Query as DistinctOnDsl<Selection>>::Output;
63+
type Output = dsl::DistinctOn<SelectStatement<T>, Selection>;
5664

57-
fn distinct_on(self, selection: Selection) -> Self::Output {
65+
fn distinct_on(self, selection: Selection) -> dsl::DistinctOn<Self, Selection> {
5866
self.as_query().distinct_on(selection)
5967
}
6068
}

diesel/src/query_dsl/group_by_dsl.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
use crate::dsl;
12
use crate::expression::Expression;
2-
use crate::query_builder::AsQuery;
3+
use crate::expression::TypedExpressionType;
4+
use crate::expression::ValidGrouping;
5+
use crate::query_builder::{AsQuery, SelectStatement};
36
use crate::query_source::Table;
47

58
/// The `group_by` method
@@ -14,18 +17,19 @@ pub trait GroupByDsl<Expr: Expression> {
1417
type Output;
1518

1619
/// See the trait documentation.
17-
fn group_by(self, expr: Expr) -> Self::Output;
20+
fn group_by(self, expr: Expr) -> dsl::GroupBy<Self, Expr>;
1821
}
1922

2023
impl<T, Expr> GroupByDsl<Expr> for T
2124
where
2225
Expr: Expression,
23-
T: Table + AsQuery,
24-
T::Query: GroupByDsl<Expr>,
26+
T: Table + AsQuery<Query = SelectStatement<T>>,
27+
T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
28+
T::SqlType: TypedExpressionType,
2529
{
26-
type Output = <T::Query as GroupByDsl<Expr>>::Output;
30+
type Output = dsl::GroupBy<SelectStatement<T>, Expr>;
2731

28-
fn group_by(self, expr: Expr) -> Self::Output {
32+
fn group_by(self, expr: Expr) -> dsl::GroupBy<Self, Expr> {
2933
self.as_query().group_by(expr)
3034
}
3135
}

diesel/src/query_dsl/locking_dsl.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
use crate::expression::TypedExpressionType;
2+
use crate::expression::ValidGrouping;
13
use crate::query_builder::AsQuery;
4+
use crate::query_builder::SelectStatement;
25
use crate::query_source::Table;
6+
use crate::Expression;
37

48
/// Methods related to locking select statements
59
///
@@ -21,10 +25,11 @@ pub trait LockingDsl<Lock> {
2125

2226
impl<T, Lock> LockingDsl<Lock> for T
2327
where
24-
T: Table + AsQuery,
25-
T::Query: LockingDsl<Lock>,
28+
T: Table + AsQuery<Query = SelectStatement<T>>,
29+
T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
30+
T::SqlType: TypedExpressionType,
2631
{
27-
type Output = <T::Query as LockingDsl<Lock>>::Output;
32+
type Output = <SelectStatement<T> as LockingDsl<Lock>>::Output;
2833

2934
fn with_lock(self, lock: Lock) -> Self::Output {
3035
self.as_query().with_lock(lock)

diesel/src/query_dsl/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,7 @@ pub trait QueryDsl: Sized {
803803
/// let data = users::table.inner_join(posts::table)
804804
/// .group_by(users::id)
805805
/// .select((users::name, count(posts::id)))
806+
/// # .order_by(users::id.asc())
806807
/// .load::<(String, i64)>(&connection)?;
807808
///
808809
/// assert_eq!(vec![(String::from("Sean"), 2), (String::from("Tess"), 1)], data);

diesel_compile_tests/tests/compile-fail/boxed_queries_and_group_by.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,6 @@ fn main() {
6565
.into_boxed();
6666
//~^ ERROR BoxedDsl
6767

68-
// you cannot call group by after boxing
69-
users::table
70-
.into_boxed()
71-
.group_by(users::id)
72-
//~^ ERROR no method named `group_by` found
73-
.select(users::name)
74-
.load::<String>(&conn);
75-
7668
users::table
7769
.group_by(users::name)
7870
.select(users::name)
@@ -95,4 +87,14 @@ fn main() {
9587
// this is a different type now
9688
a = users::table.group_by(users::id).into_boxed();
9789
//~^ ERROR mismatched types
90+
91+
// you cannot call group by after boxing
92+
users::table
93+
.into_boxed()
94+
.group_by(users::id)
95+
//~^ ERROR type mismatch
96+
//~| ERROR Table
97+
//~| ERROR GroupByDsl
98+
.select(users::name)
99+
.load::<String>(&conn);
98100
}

diesel_compile_tests/tests/compile-fail/select_for_update_cannot_be_mixed_with_some_clauses.rs

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,35 @@ table! {
1212
fn main() {
1313
use self::users::dsl::*;
1414

15-
// FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
16-
// should be E0277
17-
//users.for_update().distinct();
18-
// FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
19-
// should be E0277
20-
// users.distinct().for_update();
15+
users.for_update().distinct();
16+
//~^ ERROR: E0271
17+
//~| ERROR: E0277
18+
//~| ERROR: E0277
19+
users.distinct().for_update();
20+
//~^ ERROR: E0271
21+
//~| ERROR: E0277
22+
users.for_update().distinct_on(id);
23+
//~^ ERROR: E0271
24+
//~| ERROR: E0277
25+
//~| ERROR: E0277
26+
//~| ERROR: E0277
27+
users.distinct_on(id).for_update();
28+
//~^ ERROR: E0271
29+
//~| ERROR: E0277
2130

2231
users.for_update().group_by(id);
23-
//~^ ERROR: E0599
24-
// FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
25-
// should be E0277
26-
// users.group_by(id).for_update();
32+
//~^ ERROR: E0271
33+
//~| ERROR: E0277
34+
//~| ERROR: E0277
35+
users.group_by(id).for_update();
36+
//~^ ERROR: E0271
37+
//~| ERROR: E0277
2738

28-
// FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
29-
// should be E0277
30-
// users.into_boxed().for_update();
39+
users.into_boxed().for_update();
40+
//~^ ERROR: E0271
41+
//~| ERROR: E0277
3142
users.for_update().into_boxed();
32-
//~^ ERROR: E0275
43+
//~^ ERROR: E0271
44+
//~| ERROR: E0277
45+
//~| ERROR: E0277
3346
}

0 commit comments

Comments
 (0)