Implement ValidGrouping<GB> for more cases

This commit adds support for more kinds of `GROUP BY` clauses.

* We implement now `ValidGrouping<table::primary_key, IsAggregate = Yes>` for each column
of that table, as according to the postgres documentation it is
sufficient to have only the primary key in the group by clause
* We provide a basic infrastructure to handle `GROUP BY` clauses
consisting of more than one column by introducing a
`IsContainedInGroupBy` helper trait. It is currently used to check if a
column is part of a composite `GROUP BY` clause by checking if the
corresponding tuple contains this column. As basic requirement an  `impl
IsContainedInGroupBy<column_a> for column_b { type Output = No; }` is
required for each pair `column_a` and `column_b` that should be allowed
to appear together in a `GROUP BY` clause. This is automatically
implemented for all columns belonging to one table. For cross table
cases the `allow_columns_to_appear_in_same_group_by_clause!` macro is
provided. (This macros just skips the default per table generated
impls). As future extension we may want to generate a call to that macro
for all columns in a generated schema, after checking the impact on
compile times of having that much impls.
* Additionally this commit adds a bunch of test cases that cover the old
and new allowed usecases of `GROUP BY` clauses

Fixes diesel-rs#2370, CC diesel-rs#210

diesel/src/deserialize.rs

@@ -434,9 +434,9 @@ where
 
 impl<T, ST, DB> StaticallySizedRow<ST, DB> for T
 where
-    ST: SqlType + crate::type_impls::tuples::TupleSize,
+    ST: SqlType + crate::util::TupleSize,
     T: Queryable<ST, DB>,
     DB: Backend,
 {
-    const FIELD_COUNT: usize = <ST as crate::type_impls::tuples::TupleSize>::SIZE;
+    const FIELD_COUNT: usize = <ST as crate::util::TupleSize>::SIZE;
 }

diesel/src/expression/mod.rs

@@ -444,6 +444,34 @@ impl<'a, T: ValidGrouping<GB> + ?Sized, GB> ValidGrouping<GB> for &'a T {
 #[doc(inline)]
 pub use diesel_derives::ValidGrouping;
 
+#[doc(hidden)]
+pub trait IsContainedInGroupBy<T> {
+    type Output;
+}
+
+#[doc(hidden)]
+#[allow(missing_debug_implementations, missing_copy_implementations)]
+pub mod is_contained_in_group_by {
+    pub struct Yes;
+    pub struct No;
+
+    pub trait IsAny<O> {
+        type Output;
+    }
+
+    impl<T> IsAny<T> for Yes {
+        type Output = Yes;
+    }
+
+    impl IsAny<Yes> for No {
+        type Output = Yes;
+    }
+
+    impl IsAny<No> for No {
+        type Output = No;
+    }
+}
+
 /// Can two `IsAggregate` types appear in the same expression?
 ///
 /// You should never implement this trait. It will eventually become a trait

diesel/src/macros/mod.rs

@@ -4,6 +4,7 @@
 pub(crate) mod prelude {
     #[doc(inline)]
     pub use crate::{
+        allow_columns_to_appear_in_same_group_by_clause,
         allow_tables_to_appear_in_same_query,
         joinable,
         table,
@@ -86,12 +87,18 @@ macro_rules! __diesel_column {
         {
         }
 
+        impl<__GB> $crate::expression::ValidGrouping<__GB> for $column_name
+        where __GB: $crate::expression::IsContainedInGroupBy<$column_name, Output = $crate::expression::is_contained_in_group_by::Yes>,
+        {
+            type IsAggregate = $crate::expression::is_aggregate::Yes;
+        }
+
         impl $crate::expression::ValidGrouping<()> for $column_name {
             type IsAggregate = $crate::expression::is_aggregate::No;
         }
 
-        impl $crate::expression::ValidGrouping<$column_name> for $column_name {
-            type IsAggregate = $crate::expression::is_aggregate::Yes;
+        impl $crate::expression::IsContainedInGroupBy<$column_name> for $column_name {
+            type Output = $crate::expression::is_contained_in_group_by::Yes;
         }
 
         impl $crate::query_source::Column for $column_name {
@@ -838,11 +845,59 @@ macro_rules! __diesel_table_impl {
                     ty = ($($column_ty)*),
                     $($column)*
                 })+
+
+                $crate::__diesel_valid_grouping_for_table_columns! {
+                    primary_key = $primary_key, $($column_name,)*
+                }
             }
         }
     }
 }
 
+#[macro_export]
+#[doc(hidden)]
+macro_rules! __diesel_valid_grouping_for_table_columns {
+    (primary_key = ($primary_key: tt), $($cols: ident,)* ) => {
+        $crate::__diesel_valid_grouping_for_table_columns! {
+            primary_key = $primary_key,
+            $($cols,)*
+        }
+    };
+    (primary_key = $primary_key: tt, $left_col: ident, $($right_col: ident,)+) => {
+        $(
+            $crate::static_cond! {
+                if $left_col == $primary_key {
+                    impl $crate::expression::IsContainedInGroupBy<$right_col> for $left_col {
+                        type Output = $crate::expression::is_contained_in_group_by::Yes;
+                    }
+                } else {
+                    impl $crate::expression::IsContainedInGroupBy<$right_col> for $left_col {
+                        type Output = $crate::expression::is_contained_in_group_by::No;
+                    }
+                }
+            }
+
+            $crate::static_cond! {
+                if $right_col == $primary_key {
+                    impl $crate::expression::IsContainedInGroupBy<$left_col> for $right_col {
+                        type Output = $crate::expression::is_contained_in_group_by::Yes;
+                    }
+                } else {
+                    impl $crate::expression::IsContainedInGroupBy<$left_col> for $right_col {
+                        type Output = $crate::expression::is_contained_in_group_by::No;
+                    }
+                }
+            }
+       )*
+        $crate::__diesel_valid_grouping_for_table_columns! {
+            primary_key = $primary_key,
+            $($right_col,)*
+        }
+    };
+    (primary_key = $primary_key: tt, $left_col: ident,) => {};
+    (primary_key = $primary_key: tt) => {};
+}
+
 #[macro_export]
 #[doc(hidden)]
 macro_rules! __diesel_table_query_source_impl {
@@ -1008,7 +1063,7 @@ macro_rules! joinable_inner {
 /// in a subselect. When this macro is invoked with more than 2 tables, every
 /// combination of those tables will be allowed to appear together.
 ///
-/// If you are using `infer_schema!` or `diesel print-schema`, an invocation of
+/// If you are using `diesel print-schema`, an invocation of
 /// this macro will be generated for you for all tables in your schema.
 ///
 /// # Example
@@ -1051,6 +1106,181 @@ macro_rules! allow_tables_to_appear_in_same_query {
     () => {};
 }
 
+#[doc(hidden)]
+#[macro_export(local_inner_macros)]
+macro_rules! __diesel_impl_allow_in_same_group_by_clause {
+    (
+        left = [$($left_path:tt)::+],
+    ) => {};
+    (
+        left = [$($left_path:tt)::+],
+        $($right_path:tt)::+
+    ) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)+],
+            right = [$($right_path)+],
+            left_tbl = [],
+            left_path = [],
+        }
+    };
+    (
+        left = [$($left_path:tt)::+],
+        $($right_path:tt)::+,
+        $($other:tt)*
+    ) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)+],
+            right = [$($right_path)+],
+            left_tbl = [],
+            left_path = [],
+        }
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)::+],
+            $($other)*
+        }
+    };
+    (
+        left = [$left_path_p1: tt  $($left_path: tt)+],
+        right = [$($right_path: tt)*],
+        left_tbl = [$($left_tbl:tt)?],
+        left_path = [$($left_out_path:tt)*],
+    ) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)+],
+            right = [$($right_path)*],
+            left_tbl = [$left_path_p1],
+            left_path = [$($left_out_path)* $($left_tbl)?],
+        }
+    };
+    (
+        left = [$left_col: tt],
+        right = [$($right_path: tt)*],
+        left_tbl = [$($left_tbl:tt)?],
+        left_path = [$($left_out_path:tt)*],
+    ) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$left_col],
+            right = [$($right_path)*],
+            left_tbl = [$($left_tbl)?],
+            left_path = [$($left_out_path)*],
+            right_tbl = [],
+            right_path = [],
+        }
+    };
+    (
+        left = [$left_col: tt ],
+        right = [$right_path_p1: tt  $($right_path: tt)+],
+        left_tbl = [$($left_tbl:tt)?],
+        left_path = [$($left_out_path:tt)*],
+        right_tbl = [$($right_tbl:tt)?],
+        right_path = [$($right_out_path:tt)*],
+    ) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$left_col],
+            right = [$($right_path)+],
+            left_tbl = [$($left_tbl)?],
+            left_path = [$($left_out_path)*],
+            right_tbl = [$right_path_p1],
+            right_path = [$($right_out_path)* $($right_tbl)?],
+        }
+    };
+    (
+        left = [$left_col: tt],
+        right = [$right_col: tt],
+        left_tbl = [$left_tbl:tt],
+        left_path = [$($left_begin:tt)*],
+        right_tbl = [$right_tbl:tt],
+        right_path = [$($right_begin:tt)*],
+    ) => {
+        $crate::static_cond! {
+            if $left_tbl != $right_tbl {
+                impl $crate::expression::IsContainedInGroupBy<$($left_begin ::)* $left_tbl :: $left_col> for $($right_begin ::)* $right_tbl :: $right_col {
+                    type Output = $crate::expression::is_contained_in_group_by::No;
+                }
+
+                impl $crate::expression::IsContainedInGroupBy<$($right_begin ::)* $right_tbl :: $right_col> for $($left_begin ::)* $left_tbl :: $left_col {
+                    type Output = $crate::expression::is_contained_in_group_by::No;
+                }
+            }
+        }
+    };
+    (
+        left = [$left_col: tt],
+        right = [$right_col: tt],
+        left_tbl = [$($left_tbl:tt)?],
+        left_path = [$($left_begin:tt)*],
+        right_tbl = [$($right_tbl:tt)?],
+        right_path = [$($right_begin:tt)*],
+    ) => {
+        impl $crate::expression::IsContainedInGroupBy<$($left_begin ::)* $($left_tbl ::)? $left_col> for $($right_begin ::)* $($right_tbl ::)? $right_col {
+            type Output = $crate::expression::is_contained_in_group_by::No;
+        }
+
+        impl $crate::expression::IsContainedInGroupBy<$($right_begin ::)* $($right_tbl ::)? $right_col> for $($left_begin ::)* $($left_tbl ::)? $left_col {
+            type Output = $crate::expression::is_contained_in_group_by::No;
+        }
+    };
+
+}
+
+
+/// Allow two or more columns which are otherwise unrelated to be used together
+/// in a group by clause.
+///
+/// This macro must be invoked any time two columns need to appear in the same
+/// group by clause. When this macro is invoked with more than 2 columns, every
+/// combination of those columns will be allowed to appear together.
+///
+/// # Example
+///
+/// ```ignore
+/// // This would be required to do
+/// // `users::table.inner_join(posts::table).group_by((users::name, users::hair_color, posts::id))`
+/// allow_columns_to_appear_in_same_group_by_clause!(users::name, users::hair_color, posts::id);
+/// ```
+///
+/// When more than two columns are passed, the relevant code is generated for
+/// every combination of those columns. This code would be equivalent to the
+/// previous example.
+///
+/// ```ignore
+/// allow_columns_to_appear_in_same_group_by_clause!(users::name, users::hair_color);
+/// allow_columns_to_appear_in_same_group_by_clause!(users::name, posts::id);
+/// allow_columns_to_appear_in_same_group_by_clause!(users::hair_color, posts::id);
+/// ```
+#[macro_export(local_inner_macros)]
+macro_rules! allow_columns_to_appear_in_same_group_by_clause {
+    ($($left_path:tt)::+, $($right_path:tt)::+ $(,)?) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)::+],
+            $($right_path)::+,
+        }
+    };
+    ($($left_path:tt)::+, $($right_path:tt)::+, $($other: tt)*) => {
+        __diesel_impl_allow_in_same_group_by_clause! {
+            left = [$($left_path)::+],
+            $($right_path)::+,
+            $($other)*
+        }
+        allow_columns_to_appear_in_same_group_by_clause! {
+            $($right_path)::+,
+            $($other)*
+        }
+    };
+    ($last_col:ty,) => {};
+    () => {};
+}
+
+#[macro_export]
+#[doc(hidden)]
+macro_rules! __diesel_with_dollar_sign {
+    ($($body:tt)*) => {
+        macro_rules! __with_dollar_sign { $($body)* }
+        __with_dollar_sign!($);
+    }
+}
+
+
 // The order of these modules is important (at least for those which have tests).
 // Utility macros which don't call any others need to come first.
 #[macro_use]
@@ -1224,4 +1454,55 @@ mod tests {
             crate::debug_query::<Sqlite, _>(&bar::table.select(bar::id)).to_string()
         );
     }
+
+    mod tests_for_allow_combined_group_by_syntax {
+        table! {
+            a(b) {
+                b -> Text,
+                c -> Text,
+                d -> Text,
+                e -> Text,
+            }
+        }
+
+        table! {
+            b(a) {
+                a -> Text,
+                c -> Text,
+                d -> Text,
+            }
+        }
+
+        table! {
+            c(a) {
+                a -> Text,
+                b -> Text,
+                d -> Text,
+            }
+        }
+
+        // allow using table::collumn
+        allow_columns_to_appear_in_same_group_by_clause!(
+            a::b, b::a, a::d,
+        );
+
+        // allow using full paths
+        allow_columns_to_appear_in_same_group_by_clause!(
+            self::a::c, self::b::c, self::b::d,
+        );
+
+        use self::a::d as a_d;
+        use self::b::d as b_d;
+        use self::c::d as c_d;
+
+        // allow using plain identifiers
+        allow_columns_to_appear_in_same_group_by_clause!(
+            a_d, b_d, c_d
+        );
+
+        // allow mixing all variants
+        allow_columns_to_appear_in_same_group_by_clause!(
+            c_d, self::b::a, a::e,
+        );
+    }
 }