Merge pull request diesel-rs#2636 from weiznich/fix/1641

Fix diesel-rs#1641

Author: weiznich

CHANGELOG.md

@@ -187,6 +187,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 
 * Parenthesis are now inserted around all infix operations provided by diesel's `ExpressionMethods` traits
 
+* Queries containing a `distinct on` clause check now on compile time that a compatible order clause was set.
+
 ### Deprecated
 
 * `diesel_(prefix|postfix|infix)_operator!` have been deprecated. These macros

diesel/src/pg/query_builder/distinct_on.rs

@@ -1,13 +1,26 @@
 use crate::expression::SelectableExpression;
 use crate::pg::Pg;
+use crate::query_builder::order_clause::{NoOrderClause, OrderClause};
 use crate::query_builder::{AstPass, QueryFragment, QueryId, SelectQuery, SelectStatement};
 use crate::query_dsl::methods::DistinctOnDsl;
+use crate::query_dsl::order_dsl::ValidOrderingForDistinct;
 use crate::result::QueryResult;
+use crate::sql_types::SingleValue;
+use crate::Expression;
 
 /// Represents `DISTINCT ON (...)`
 #[derive(Debug, Clone, Copy, QueryId)]
 pub struct DistinctOnClause<T>(pub(crate) T);
 
+impl<T> ValidOrderingForDistinct<DistinctOnClause<T>> for NoOrderClause {}
+impl<T> ValidOrderingForDistinct<DistinctOnClause<T>> for OrderClause<(T,)> {}
+impl<T> ValidOrderingForDistinct<DistinctOnClause<T>> for OrderClause<T>
+where
+    T: Expression,
+    T::SqlType: SingleValue,
+{
+}
+
 impl<T> QueryFragment<Pg> for DistinctOnClause<T>
 where
     T: QueryFragment<Pg>,
@@ -24,7 +37,9 @@ impl<ST, F, S, D, W, O, LOf, G, Selection> DistinctOnDsl<Selection>
     for SelectStatement<F, S, D, W, O, LOf, G>
 where
     Selection: SelectableExpression<F>,
+    Selection::SqlType: SingleValue,
     Self: SelectQuery<SqlType = ST>,
+    O: ValidOrderingForDistinct<DistinctOnClause<Selection>>,
     SelectStatement<F, S, DistinctOnClause<Selection>, W, O, LOf, G>: SelectQuery<SqlType = ST>,
 {
     type Output = SelectStatement<F, S, DistinctOnClause<Selection>, W, O, LOf, G>;

diesel/src/query_builder/distinct_clause.rs

@@ -1,5 +1,6 @@
 use crate::backend::Backend;
 use crate::query_builder::*;
+use crate::query_dsl::order_dsl::ValidOrderingForDistinct;
 use crate::result::QueryResult;
 
 #[derive(Debug, Clone, Copy, QueryId)]
@@ -20,5 +21,8 @@ impl<DB: Backend> QueryFragment<DB> for DistinctClause {
     }
 }
 
+impl<O> ValidOrderingForDistinct<NoDistinctClause> for O {}
+impl<O> ValidOrderingForDistinct<DistinctClause> for O {}
+
 #[cfg(feature = "postgres")]
 pub use crate::pg::DistinctOnClause;

diesel/src/query_builder/mod.rs

@@ -25,7 +25,7 @@ pub(crate) mod locking_clause;
 #[doc(hidden)]
 pub mod nodes;
 pub(crate) mod offset_clause;
-mod order_clause;
+pub(crate) mod order_clause;
 mod returning_clause;
 pub(crate) mod select_clause;
 mod select_statement;

diesel/src/query_builder/select_statement/dsl_impls.rs

@@ -22,6 +22,7 @@ use crate::query_builder::{
 };
 use crate::query_dsl::boxed_dsl::BoxedDsl;
 use crate::query_dsl::methods::*;
+use crate::query_dsl::order_dsl::ValidOrderingForDistinct;
 use crate::query_dsl::*;
 use crate::query_source::joins::{Join, JoinOn, JoinTo};
 use crate::query_source::QuerySource;
@@ -159,7 +160,7 @@ where
 impl<ST, F, S, D, W, O, LOf, G, LC, Expr> OrderDsl<Expr>
     for SelectStatement<F, S, D, W, O, LOf, G, LC>
 where
-    Expr: AppearsOnTable<F>,
+    Expr: AppearsOnTable<F> + ValidOrderingForDistinct<D>,
     Self: SelectQuery<SqlType = ST>,
     SelectStatement<F, S, D, W, OrderClause<Expr>, LOf, G, LC>: SelectQuery<SqlType = ST>,
 {

diesel/src/query_builder/select_statement/mod.rs

@@ -30,6 +30,7 @@ use crate::expression::subselect::ValidSubselect;
 use crate::expression::*;
 use crate::query_builder::limit_offset_clause::LimitOffsetClause;
 use crate::query_builder::{QueryId, SelectQuery};
+use crate::query_dsl::order_dsl::ValidOrderingForDistinct;
 use crate::query_source::joins::{AppendSelection, Inner, Join};
 use crate::query_source::*;
 use crate::result::QueryResult;
@@ -113,6 +114,7 @@ where
 impl<F, S, D, W, O, LOf, G, LC> SelectQuery for SelectStatement<F, S, D, W, O, LOf, G, LC>
 where
     S: SelectClauseExpression<F>,
+    O: ValidOrderingForDistinct<D>,
 {
     type SqlType = S::SelectClauseSqlType;
 }

diesel/src/query_dsl/distinct_dsl.rs

@@ -3,8 +3,7 @@ use crate::dsl;
 use crate::expression::SelectableExpression;
 use crate::expression::TypedExpressionType;
 use crate::expression::ValidGrouping;
-use crate::query_builder::AsQuery;
-use crate::query_builder::SelectStatement;
+use crate::query_builder::{AsQuery, SelectStatement};
 use crate::query_source::Table;
 use crate::Expression;
 
@@ -57,6 +56,7 @@ impl<T, Selection> DistinctOnDsl<Selection> for T
 where
     Selection: SelectableExpression<T>,
     T: Table + AsQuery<Query = SelectStatement<T>>,
+    SelectStatement<T>: DistinctOnDsl<Selection>,
     T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
     T::SqlType: TypedExpressionType,
 {

diesel/src/query_dsl/mod.rs

@@ -38,7 +38,7 @@ pub mod load_dsl;
 mod locking_dsl;
 mod nullable_select_dsl;
 mod offset_dsl;
-mod order_dsl;
+pub(crate) mod order_dsl;
 #[doc(hidden)]
 pub mod positional_order_dsl;
 mod save_changes_dsl;
@@ -152,7 +152,10 @@ pub trait QueryDsl: Sized {
     ///     .execute(&connection)
     ///     .unwrap();
     /// let all_animals = animals.select((species, name, legs)).load(&connection);
-    /// let distinct_animals = animals.select((species, name, legs)).distinct_on(species).load(&connection);
+    /// let distinct_animals = animals
+    ///     .select((species, name, legs))
+    ///     .order_by((species, legs))
+    ///     .distinct_on(species).load(&connection);
     ///
     /// assert_eq!(Ok(vec![Animal::new("dog", Some("Jack"), 4),
     ///                    Animal::new("dog", None, 4),

diesel/src/query_dsl/order_dsl.rs

@@ -56,3 +56,5 @@ where
         self.as_query().then_order_by(expr)
     }
 }
+
+pub trait ValidOrderingForDistinct<D> {}

diesel/src/type_impls/tuples.rs

@@ -316,6 +316,10 @@ macro_rules! tuple_impls {
                 }
             }
 
+            #[cfg(feature = "postgres")]
+            impl<__D, $($T,)*> crate::query_dsl::order_dsl::ValidOrderingForDistinct<crate::pg::DistinctOnClause<__D>>
+                for crate::query_builder::order_clause::OrderClause<(__D, $($T,)*)> {}
+
         )+
     }
 }