Merge pull request #2571 from weiznich/stabilize/group_by

Make `group_by` part of our public API

CHANGELOG.md

@@ -50,6 +50,8 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 * Added `#[diesel(serialize_as)]` analogous to `#[diesel(deserialize_as)]`. This allows
   customization of the serialization behaviour of `Insertable` structs.
 
+* Added support for `GROUP BY` clauses
+
 ### Removed
 
 * All previously deprecated items have been removed.

diesel/src/lib.rs

@@ -303,6 +303,9 @@ pub mod helper_types {
 
     /// Represents the return type of `.nullable()`
     pub type NullableSelect<Source> = <Source as SelectNullableDsl>::Output;
+
+    /// Represents the return type of `.group_by(expr)`
+    pub type GroupBy<Source, Expr> = <Source as GroupByDsl<Expr>>::Output;
 }
 
 pub mod prelude {
@@ -332,8 +335,6 @@ pub mod prelude {
     pub use crate::query_builder::AsChangeset;
     #[doc(inline)]
     pub use crate::query_builder::DecoratableTarget;
-    #[doc(hidden)]
-    pub use crate::query_dsl::GroupByDsl;
     #[doc(inline)]
     pub use crate::query_dsl::{BelongingToDsl, JoinOnDsl, QueryDsl, RunQueryDsl, SaveChangesDsl};
     #[doc(inline)]

diesel/src/query_dsl/boxed_dsl.rs

@@ -1,5 +1,10 @@
+use crate::dsl;
+use crate::expression::TypedExpressionType;
+use crate::expression::ValidGrouping;
 use crate::query_builder::AsQuery;
+use crate::query_builder::SelectStatement;
 use crate::query_source::Table;
+use crate::Expression;
 
 /// The `into_boxed` method
 ///
@@ -13,15 +18,17 @@ pub trait BoxedDsl<'a, DB> {
     type Output;
 
     /// See the trait documentation.
-    fn internal_into_boxed(self) -> Self::Output;
+    fn internal_into_boxed(self) -> dsl::IntoBoxed<'a, Self, DB>;
 }
 
 impl<'a, T, DB> BoxedDsl<'a, DB> for T
 where
-    T: Table + AsQuery,
-    T::Query: BoxedDsl<'a, DB>,
+    T: Table + AsQuery<Query = SelectStatement<T>>,
+    SelectStatement<T>: BoxedDsl<'a, DB>,
+    T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
+    T::SqlType: TypedExpressionType,
 {
-    type Output = <T::Query as BoxedDsl<'a, DB>>::Output;
+    type Output = dsl::IntoBoxed<'a, SelectStatement<T>, DB>;
 
     fn internal_into_boxed(self) -> Self::Output {
         self.as_query().internal_into_boxed()

diesel/src/query_dsl/distinct_dsl.rs

@@ -1,6 +1,12 @@
+use crate::dsl;
 #[cfg(feature = "postgres")]
 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_source::Table;
+use crate::Expression;
 
 /// The `distinct` method
 ///
@@ -14,17 +20,18 @@ pub trait DistinctDsl {
     type Output;
 
     /// See the trait documentation.
-    fn distinct(self) -> Self::Output;
+    fn distinct(self) -> dsl::Distinct<Self>;
 }
 
 impl<T> DistinctDsl for T
 where
-    T: Table,
-    T::Query: DistinctDsl,
+    T: Table + AsQuery<Query = SelectStatement<T>>,
+    T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
+    T::SqlType: TypedExpressionType,
 {
-    type Output = <T::Query as DistinctDsl>::Output;
+    type Output = dsl::Distinct<SelectStatement<T>>;
 
-    fn distinct(self) -> Self::Output {
+    fn distinct(self) -> dsl::Distinct<SelectStatement<T>> {
         self.as_query().distinct()
     }
 }
@@ -42,19 +49,20 @@ pub trait DistinctOnDsl<Selection> {
     type Output;
 
     /// See the trait documentation
-    fn distinct_on(self, selection: Selection) -> Self::Output;
+    fn distinct_on(self, selection: Selection) -> dsl::DistinctOn<Self, Selection>;
 }
 
 #[cfg(feature = "postgres")]
 impl<T, Selection> DistinctOnDsl<Selection> for T
 where
     Selection: SelectableExpression<T>,
-    T: Table,
-    T::Query: DistinctOnDsl<Selection>,
+    T: Table + AsQuery<Query = SelectStatement<T>>,
+    T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
+    T::SqlType: TypedExpressionType,
 {
-    type Output = <T::Query as DistinctOnDsl<Selection>>::Output;
+    type Output = dsl::DistinctOn<SelectStatement<T>, Selection>;
 
-    fn distinct_on(self, selection: Selection) -> Self::Output {
+    fn distinct_on(self, selection: Selection) -> dsl::DistinctOn<Self, Selection> {
         self.as_query().distinct_on(selection)
     }
 }

diesel/src/query_dsl/group_by_dsl.rs

@@ -1,35 +1,35 @@
+use crate::dsl;
 use crate::expression::Expression;
-use crate::query_builder::AsQuery;
+use crate::expression::TypedExpressionType;
+use crate::expression::ValidGrouping;
+use crate::query_builder::{AsQuery, SelectStatement};
 use crate::query_source::Table;
 
-/// This trait is not yet part of Diesel's public API. It may change in the
-/// future without a major version bump.
+/// The `group_by` method
 ///
-/// This trait exists as a stop-gap for users who need to use `GROUP BY` in
-/// their queries, so that they are not forced to drop entirely to raw SQL. The
-/// arguments to `group_by` are not checked, nor is the select statement
-/// forced to be valid.
+/// This trait should not be relied on directly by most apps. Its behavior is
+/// provided by [`QueryDsl`]. However, you may need a where clause on this trait
+/// to call `group_by` from generic code.
 ///
-/// Since Diesel otherwise assumes that you have no `GROUP BY` clause (which
-/// would mean that mixing an aggregate and non aggregate expression in the same
-/// query is an error), you may need to use `sql` for your select clause.
+/// [`QueryDsl`]: ../trait.QueryDsl.html
 pub trait GroupByDsl<Expr: Expression> {
     /// The type returned by `.group_by`
     type Output;
 
     /// See the trait documentation.
-    fn group_by(self, expr: Expr) -> Self::Output;
+    fn group_by(self, expr: Expr) -> dsl::GroupBy<Self, Expr>;
 }
 
 impl<T, Expr> GroupByDsl<Expr> for T
 where
     Expr: Expression,
-    T: Table + AsQuery,
-    T::Query: GroupByDsl<Expr>,
+    T: Table + AsQuery<Query = SelectStatement<T>>,
+    T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
+    T::SqlType: TypedExpressionType,
 {
-    type Output = <T::Query as GroupByDsl<Expr>>::Output;
+    type Output = dsl::GroupBy<SelectStatement<T>, Expr>;
 
-    fn group_by(self, expr: Expr) -> Self::Output {
+    fn group_by(self, expr: Expr) -> dsl::GroupBy<Self, Expr> {
         self.as_query().group_by(expr)
     }
 }

diesel/src/query_dsl/locking_dsl.rs

@@ -1,5 +1,9 @@
+use crate::expression::TypedExpressionType;
+use crate::expression::ValidGrouping;
 use crate::query_builder::AsQuery;
+use crate::query_builder::SelectStatement;
 use crate::query_source::Table;
+use crate::Expression;
 
 /// Methods related to locking select statements
 ///
@@ -21,10 +25,11 @@ pub trait LockingDsl<Lock> {
 
 impl<T, Lock> LockingDsl<Lock> for T
 where
-    T: Table + AsQuery,
-    T::Query: LockingDsl<Lock>,
+    T: Table + AsQuery<Query = SelectStatement<T>>,
+    T::DefaultSelection: Expression<SqlType = T::SqlType> + ValidGrouping<()>,
+    T::SqlType: TypedExpressionType,
 {
-    type Output = <T::Query as LockingDsl<Lock>>::Output;
+    type Output = <SelectStatement<T> as LockingDsl<Lock>>::Output;
 
     fn with_lock(self, lock: Lock) -> Self::Output {
         self.as_query().with_lock(lock)

diesel/src/query_dsl/mod.rs

@@ -44,8 +44,6 @@ pub mod select_dsl;
 mod single_value_dsl;
 
 pub use self::belonging_to_dsl::BelongingToDsl;
-#[doc(hidden)]
-pub use self::group_by_dsl::GroupByDsl;
 pub use self::join_dsl::{InternalJoinDsl, JoinOnDsl, JoinWithImplicitOnClause};
 #[doc(hidden)]
 pub use self::load_dsl::LoadQuery;
@@ -62,6 +60,7 @@ pub mod methods {
     pub use super::distinct_dsl::*;
     #[doc(inline)]
     pub use super::filter_dsl::*;
+    pub use super::group_by_dsl::GroupByDsl;
     pub use super::limit_dsl::LimitDsl;
     pub use super::load_dsl::{ExecuteDsl, LoadQuery};
     pub use super::locking_dsl::{LockingDsl, ModifyLockDsl};
@@ -417,7 +416,7 @@ pub trait QueryDsl: Sized {
     /// assert_eq!(Ok(expected_data), data);
     /// # }
     /// ```
-    fn inner_join<Rhs>(self, rhs: Rhs) -> Self::Output
+    fn inner_join<Rhs>(self, rhs: Rhs) -> InnerJoin<Self, Rhs>
     where
         Self: JoinWithImplicitOnClause<Rhs, joins::Inner>,
     {
@@ -430,7 +429,7 @@ pub trait QueryDsl: Sized {
     /// instead. See [`inner_join`] for usage examples.
     ///
     /// [`inner_join`]: #method.inner_join
-    fn left_outer_join<Rhs>(self, rhs: Rhs) -> Self::Output
+    fn left_outer_join<Rhs>(self, rhs: Rhs) -> LeftJoin<Self, Rhs>
     where
         Self: JoinWithImplicitOnClause<Rhs, joins::LeftOuter>,
     {
@@ -440,7 +439,7 @@ pub trait QueryDsl: Sized {
     /// Alias for [`left_outer_join`].
     ///
     /// [`left_outer_join`]: #method.left_outer_join
-    fn left_join<Rhs>(self, rhs: Rhs) -> Self::Output
+    fn left_join<Rhs>(self, rhs: Rhs) -> LeftJoin<Self, Rhs>
     where
         Self: JoinWithImplicitOnClause<Rhs, joins::LeftOuter>,
     {
@@ -549,7 +548,7 @@ pub trait QueryDsl: Sized {
 
     /// Sets the order clause of a query.
     ///
-    /// If there was already a order clause, it will be overridden. See
+    /// If there was already an order clause, it will be overridden. See
     /// also:
     /// [`.desc()`](../expression_methods/trait.ExpressionMethods.html#method.desc)
     /// and
@@ -770,6 +769,55 @@ pub trait QueryDsl: Sized {
         methods::OffsetDsl::offset(self, offset)
     }
 
+    /// Sets the `group by` clause of a query.
+    ///
+    /// **Note:** Queries having a `group by` clause require a custom select clause.
+    /// Use `QueryDsl::select()` to specify one
+    ///
+    /// If there was already a group by clause, it will be overridden.
+    /// Ordering by multiple columns can be achieved by passing a tuple of those
+    /// columns.
+    ///
+    /// Diesel follows postgresql's group by semantic, this means any column
+    /// appearing in a group by clause is considered to be aggregated. If a
+    /// primary key is part of the group by clause every column from the
+    /// corresponding table is considerd to be aggregated. Select clauses
+    /// cannot mix aggregated and non aggregated expressions.
+    ///
+    /// For group by clauses containing columns from more than one table it
+    /// is required to call [`allow_columns_to_appear_in_same_group_by_clause!`]
+    ///
+    /// [`allow_columns_to_appear_in_same_group_by_clause!`]: ../macro.allow_columns_to_appear_in_same_group_by_clause.html
+    ///
+    /// # Examples
+    /// ```rust
+    /// # include!("../doctest_setup.rs");
+    /// # fn main() {
+    /// #     run_test();
+    /// # }
+    /// #
+    /// # fn run_test() -> QueryResult<()> {
+    /// #     use crate::schema::{users, posts};
+    /// #     use diesel::dsl::count;
+    /// #     let connection = establish_connection();
+    /// let data = users::table.inner_join(posts::table)
+    ///     .group_by(users::id)
+    ///     .select((users::name, count(posts::id)))
+    /// #   .order_by(users::id.asc())
+    ///     .load::<(String, i64)>(&connection)?;
+    ///
+    /// assert_eq!(vec![(String::from("Sean"), 2), (String::from("Tess"), 1)], data);
+    /// # Ok(())
+    /// # }
+    /// ```
+    fn group_by<GB>(self, group_by: GB) -> GroupBy<Self, GB>
+    where
+        GB: Expression,
+        Self: methods::GroupByDsl<GB>,
+    {
+        methods::GroupByDsl::group_by(self, group_by)
+    }
+
     /// Adds `FOR UPDATE` to the end of the select statement.
     ///
     /// This method is only available for MySQL and PostgreSQL. SQLite does not

diesel_compile_tests/tests/compile-fail/boxed_queries_and_group_by.rs

@@ -65,14 +65,6 @@ fn main() {
         .into_boxed();
     //~^ ERROR BoxedDsl
 
-    // you cannot call group by after boxing
-    users::table
-        .into_boxed()
-        .group_by(users::id)
-        //~^ ERROR no method named `group_by` found
-        .select(users::name)
-        .load::<String>(&conn);
-
     users::table
         .group_by(users::name)
         .select(users::name)
@@ -95,4 +87,14 @@ fn main() {
     // this is a different type now
     a = users::table.group_by(users::id).into_boxed();
     //~^ ERROR mismatched types
+
+    // you cannot call group by after boxing
+    users::table
+        .into_boxed()
+        .group_by(users::id)
+        //~^ ERROR type mismatch
+        //~| ERROR Table
+        //~| ERROR GroupByDsl
+        .select(users::name)
+        .load::<String>(&conn);
 }

diesel_compile_tests/tests/compile-fail/select_for_update_cannot_be_mixed_with_some_clauses.rs

@@ -12,22 +12,35 @@ table! {
 fn main() {
     use self::users::dsl::*;
 
-    // FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
-    // should be E0277
-    //users.for_update().distinct();
-    // FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
-    // should be E0277
-    // users.distinct().for_update();
+    users.for_update().distinct();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
+    //~| ERROR: E0277
+    users.distinct().for_update();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
+    users.for_update().distinct_on(id);
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
+    //~| ERROR: E0277
+    //~| ERROR: E0277
+    users.distinct_on(id).for_update();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
 
     users.for_update().group_by(id);
-    //~^ ERROR: E0599
-    // FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
-    // should be E0277
-    // users.group_by(id).for_update();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
+    //~| ERROR: E0277
+    users.group_by(id).for_update();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
 
-    // FIXME: Overflows because of https://github.com/rust-lang/rust/issues/34260
-    // should be E0277
-    // users.into_boxed().for_update();
+    users.into_boxed().for_update();
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
     users.for_update().into_boxed();
-    //~^ ERROR: E0275
+    //~^ ERROR: E0271
+    //~| ERROR: E0277
+    //~| ERROR: E0277
 }