Repository: diesel-rs/diesel (language: rust)

Issue:
Aliases with group_by and select
<!--
If you want to report a bug, we added some points below which help us track down the problem faster.
-->

## Setup

### Versions

- **Rust:** rustc 1.63.0 (4b91a6ea7 2022-08-08)
- **Diesel:** 2.0.0
- **Database:** PostgreSQL 13
- **Operating System** Linux (Ubuntu 22.04)

### Feature Flags

- **diesel:** postgres, uuid, chrono

## Problem Description
I'm trying to add second alias to query with left_join and failing.
I have the following code which compiles fine:
```rust
let leafs = diesel::alias!(chunks as leafs);
let mut query = chunks::table
    .left_join(
        leafs.on(chunks::path
            .contains(leafs.field(chunks::path))
            .and(leafs.field(chunks::nchildren).eq(0))),
    )
    .group_by(chunks::id)
    .select((
        chunks::all_columns,
        sum(leafs.field(chunks::estimated).nullable()),
    ))
    .into_boxed();
```
But if I'm converting it to this, it fails to compile:
```rust
let (c, leafs) = diesel::alias!(chunks as c, chunks as leafs);
let mut query = c
    .left_join(
        leafs.on(c
            .field(chunks::path)
            .contains(leafs.field(chunks::path))
            .and(leafs.field(chunks::nchildren).eq(0))),
    )
    .group_by(c.field(chunks::id))
    .select((
        c.fields(chunks::all_columns),
        sum(leafs.field(chunks::estimated).nullable()),
    ))
    .into_boxed();
```

### What are you trying to accomplish?
I'm trying to add alias for main self-referencing table to make query more complex later

### What is the expected output?
It should compile :)

### What is the actual output?
```
60 |             .select((
   |              ^^^^^^ the trait `ValidGrouping<AliasedField<c, chunks::columns::id>>` is not implemented for `AliasedField<c, chunks::columns::parent_id>`
60 |             .select((
   |              ^^^^^^ the trait `ValidGrouping<AliasedField<c, chunks::columns::id>>` is not implemented for `AliasedField<c, chunks::columns::path>`
60 |             .select((
   |              ^^^^^^ the trait `ValidGrouping<AliasedField<c, chunks::columns::id>>` is not implemented for `AliasedField<c, chunks::columns::label>`
... and so on for each field in chunk model
```

### Are you seeing any additional errors?


### Steps to reproduce
corresponding model is the following:
```rust
diesel::table! {
    use diesel::sql_types::*;
    use diesel_ltree::sql_types::*;

    chunks (id) {
        id -> Int8,
        parent_id -> Nullable<Int8>,
        path -> Ltree,
        label -> Text,
        estimated -> Int4,
        nchildren -> Int4,
    }
}
```

<!--
Please include as much of your codebase as needed to reproduce the error.  If the relevant files are large, please consider linking to a public repository or a [Gist](https://gist.github.com/). This includes normally the following parts:

* The exact code where your hit the problem
* Relevant parts your schema, so any `table!` macro calls required for the
* Any other type definitions involved in the code, which produces your problem
-->

## Checklist

- [x] I have already looked over the [issue tracker](https://github.com/diesel-rs/diesel/issues)  and the [disussion forum](https://github.com/diesel-rs/diesel/discussions)  for similar possible closed issues.
<!--
If you are unsure if your issue is a duplicate of an existing issue please link the issue in question here
--> 
- [x] This issue can be reproduced on Rust's stable channel. (Your issue will be
  closed if this is not the case)
- [x] This issue can be reproduced without requiring a third party crate

<!--
Thank you for your submission!  You're helping make Diesel more robust 🎉

We'll try to respond as quickly as possible.
-->



Discussion / hints:
Thanks for reporting this bug. It seems like that this impl is not general enough. Likely it must be change in such a way that it accepts all combinations of `AliasedFields` that implement `ValidGrouping` on their own. https://github.com/diesel-rs/diesel/blob/9502a9fd3df61bcfd59c63fecb79e242802e99d6/diesel/src/query_source/aliasing/aliased_field.rs#L80-L86
PR's fixing this are welcome.

As workaround you can list all columns explicitly in your `group_by` clause. 
Thanks for explanation!
I'll try to create a PR, but I'm afraid I'm not competent enough in Rust to do it right and not break anything else :)
@mlesin What needs to be done is the following:

* The linked impl needs to be changed to the following:
```rust
impl<S, C1, C2> ValidGrouping<AliasField<S, C1>> for AliasedField<S, C2>
where 
    S: AliasSource, 
    C1: Column<Table = S::Target>
    C2: Column<Table = S::Target>
    C2: ValidGrouping<C1, IsAggregate = is_aggregate::Yes>
{
    type IsAggregate = is_aggregate::Yes; 
}
```

* Add a test based on your example to check that the code is compiling
* Open a PR with this changes

Fix patch:
diff --git a/diesel/src/query_source/aliasing/aliased_field.rs b/diesel/src/query_source/aliasing/aliased_field.rs
index b8dc1c34b04f..1e5db2f8f625 100644
--- a/diesel/src/query_source/aliasing/aliased_field.rs
+++ b/diesel/src/query_source/aliasing/aliased_field.rs
@@ -77,10 +77,13 @@ where
 {
     type IsAggregate = is_aggregate::No;
 }
-impl<S, C> ValidGrouping<AliasedField<S, C>> for AliasedField<S, C>
+
+impl<S, C1, C2> ValidGrouping<AliasedField<S, C1>> for AliasedField<S, C2>
 where
     S: AliasSource,
-    C: Column<Table = S::Target>,
+    C1: Column<Table = S::Target>,
+    C2: Column<Table = S::Target>,
+    C2: ValidGrouping<C1, IsAggregate = is_aggregate::Yes>,
 {
     type IsAggregate = is_aggregate::Yes;
 }


Test patch:
diff --git a/diesel_compile_tests/tests/fail/alias_and_group_by.rs b/diesel_compile_tests/tests/fail/alias_and_group_by.rs
new file mode 100644
index 000000000000..34b19eb562b1
--- /dev/null
+++ b/diesel_compile_tests/tests/fail/alias_and_group_by.rs
@@ -0,0 +1,37 @@
+extern crate diesel;
+
+use diesel::alias;
+use diesel::prelude::*;
+
+table! {
+    users {
+        id -> Integer,
+        name -> VarChar,
+    }
+}
+
+fn main() {
+    let conn = &mut PgConnection::establish("…").unwrap();
+    let user_alias = alias!(users as user1);
+
+    // allowed as this groups by the the same field
+    user_alias
+        .group_by(user_alias.field(users::name))
+        .select(user_alias.field(users::name))
+        .execute(conn)
+        .unwrap();
+
+    // allowed as this groups by the primary key
+    user_alias
+        .group_by(user_alias.field(users::id))
+        .select(user_alias.field(users::name))
+        .execute(conn)
+        .unwrap();
+
+    // fails as this groups by an incompatible field
+    user_alias
+        .group_by(user_alias.field(users::name))
+        .select(user_alias.field(users::id))
+        .execute(conn)
+        .unwrap();
+}
diff --git a/diesel_compile_tests/tests/fail/alias_and_group_by.stderr b/diesel_compile_tests/tests/fail/alias_and_group_by.stderr
new file mode 100644
index 000000000000..dac75ea14e09
--- /dev/null
+++ b/diesel_compile_tests/tests/fail/alias_and_group_by.stderr
@@ -0,0 +1,20 @@
+error[E0271]: type mismatch resolving `<columns::name as IsContainedInGroupBy<columns::id>>::Output == diesel::expression::is_contained_in_group_by::Yes`
+  --> tests/fail/alias_and_group_by.rs:34:10
+   |
+34 |         .select(user_alias.field(users::id))
+   |          ^^^^^^ expected struct `diesel::expression::is_contained_in_group_by::No`, found struct `diesel::expression::is_contained_in_group_by::Yes`
+   |
+note: required because of the requirements on the impl of `ValidGrouping<columns::name>` for `columns::id`
+  --> tests/fail/alias_and_group_by.rs:6:1
+   |
+6  | / table! {
+7  | |     users {
+8  | |         id -> Integer,
+9  | |         name -> VarChar,
+10 | |     }
+11 | | }
+   | |_^
+   = note: 1 redundant requirement hidden
+   = note: required because of the requirements on the impl of `ValidGrouping<AliasedField<user1, columns::name>>` for `AliasedField<user1, columns::id>`
+   = note: required because of the requirements on the impl of `SelectDsl<AliasedField<user1, columns::id>>` for `SelectStatement<FromClause<Alias<user1>>, diesel::query_builder::select_clause::DefaultSelectClause<FromClause<Alias<user1>>>, diesel::query_builder::distinct_clause::NoDistinctClause, diesel::query_builder::where_clause::NoWhereClause, diesel::query_builder::order_clause::NoOrderClause, diesel::query_builder::limit_offset_clause::LimitOffsetClause<diesel::query_builder::limit_clause::NoLimitClause, diesel::query_builder::offset_clause::NoOffsetClause>, diesel::query_builder::group_by_clause::GroupByClause<AliasedField<user1, columns::name>>>`
+   = note: this error originates in the macro `$crate::__diesel_column` which comes from the expansion of the macro `table` (in Nightly builds, run with -Z macro-backtrace for more info)
diff --git a/diesel_tests/tests/alias.rs b/diesel_tests/tests/alias.rs
index a8bea87853b4..d78582c46e03 100644
--- a/diesel_tests/tests/alias.rs
+++ b/diesel_tests/tests/alias.rs
@@ -173,3 +173,21 @@ fn visibility() {
         const USERS_ALIAS_2: Alias<UsersAlias2> = users as users_alias_2;
     }
 }
+
+// regression test for
+// https://github.com/diesel-rs/diesel/issues/3319
+#[test]
+fn aliasing_with_group_by_and_primary_key() {
+    let connection = &mut connection_with_sean_and_tess_in_users_table();
+    let user_alias = alias!(users as user1);
+
+    let res = user_alias
+        .group_by(user_alias.field(users::id))
+        .select(user_alias.field(users::name))
+        .order_by(user_alias.field(users::id))
+        .load::<String>(connection)
+        .unwrap();
+    assert!(res.len() == 2);
+    assert_eq!(res[0], "Sean");
+    assert_eq!(res[1], "Tess");
+}