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

Issue:
Unable to use order_by with some columns after using distinct_on
## Setup

### Versions

- **Rust: 1.57.0**
- **Diesel: current main branch**
- **Database: Postgres 13.3**
- **Operating System: Arch Linux**

### Feature Flags

- **diesel: postgres, r2d2, chrono**

## Problem Description


### What are you trying to accomplish?

I'm trying to convert the following SQL query to Diesel DSL : 

```sql
select distinct on (o.id) o.id, g.id, u.id
from public.user u
inner join public.group g on u.group_id = g.id
inner join public.org o on g.org_id = o.id
order by o.id, g.created_at desc, u.created_at desc;
``` 

```rust
use diesel::{ExpressionMethods, allow_tables_to_appear_in_same_query, joinable, QueryDsl, table};

table! {
  user (id) {
    id -> Text,
	created_at -> Timestamp,
	group_id -> Text,
  }
}

table! {
  group (id) {
    id -> Text,
	created_at -> Timestamp,
	org_id -> Text,
  }
}

table! {
  org(id) {
    id -> Text,
  }
}

joinable!(user -> group (group_id));
joinable!(group -> org (org_id));

allow_tables_to_appear_in_same_query!(
    user,
    group,
    org,
);

fn main() {
    user::table
        .inner_join(group::table.inner_join(org::table))
        .distinct_on(org::id)
        .select((org::id, group::id, user::id))
        .order_by((org::id, group::created_at.desc(), user::created_at.desc()));
}
```

### What is the actual output?

```
error[E0277]: the trait bound `(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>): query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<org::columns::id>>` is not satisfied
  --> reports/src/main.rs:39:10
   |
39 |         .order_by((org::id, group::created_at.desc(), user::created_at.desc()));
   |          ^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<org::columns::id>>` is not implemented for `(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>)`
   |
   = note: required because of the requirements on the impl of `OrderDsl<(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>)>` for `SelectStatement<FromClause<JoinOn<diesel::query_source::joins::Join<user::table, SelectStatement<FromClause<JoinOn<diesel::query_source::joins::Join<group::table, org::table, Inner>, Grouped<diesel::expression::operators::Eq<diesel::expression::nullable::Nullable<group::columns::org_id>, diesel::expression::nullable::Nullable<org::columns::id>>>>>>, Inner>, Grouped<diesel::expression::operators::Eq<diesel::expression::nullable::Nullable<user::columns::group_id>, diesel::expression::nullable::Nullable<group::columns::id>>>>>, query_builder::select_clause::SelectClause<(user::columns::id, group::columns::id, org::columns::id)>, DistinctOnClause<org::columns::id>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `reports` due to previous error
```

## 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.
-->

Unable to use order_by with some columns after using distinct_on
## Setup

### Versions

- **Rust: 1.57.0**
- **Diesel: current main branch**
- **Database: Postgres 13.3**
- **Operating System: Arch Linux**

### Feature Flags

- **diesel: postgres, r2d2, chrono**

## Problem Description


### What are you trying to accomplish?

I'm trying to convert the following SQL query to Diesel DSL : 

```sql
select distinct on (o.id) o.id, g.id, u.id
from public.user u
inner join public.group g on u.group_id = g.id
inner join public.org o on g.org_id = o.id
order by o.id, g.created_at desc, u.created_at desc;
``` 

```rust
use diesel::{ExpressionMethods, allow_tables_to_appear_in_same_query, joinable, QueryDsl, table};

table! {
  user (id) {
    id -> Text,
	created_at -> Timestamp,
	group_id -> Text,
  }
}

table! {
  group (id) {
    id -> Text,
	created_at -> Timestamp,
	org_id -> Text,
  }
}

table! {
  org(id) {
    id -> Text,
  }
}

joinable!(user -> group (group_id));
joinable!(group -> org (org_id));

allow_tables_to_appear_in_same_query!(
    user,
    group,
    org,
);

fn main() {
    user::table
        .inner_join(group::table.inner_join(org::table))
        .distinct_on(org::id)
        .select((org::id, group::id, user::id))
        .order_by((org::id, group::created_at.desc(), user::created_at.desc()));
}
```

### What is the actual output?

```
error[E0277]: the trait bound `(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>): query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<org::columns::id>>` is not satisfied
  --> reports/src/main.rs:39:10
   |
39 |         .order_by((org::id, group::created_at.desc(), user::created_at.desc()));
   |          ^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<org::columns::id>>` is not implemented for `(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>)`
   |
   = note: required because of the requirements on the impl of `OrderDsl<(org::columns::id, Desc<group::columns::created_at>, Desc<user::columns::created_at>)>` for `SelectStatement<FromClause<JoinOn<diesel::query_source::joins::Join<user::table, SelectStatement<FromClause<JoinOn<diesel::query_source::joins::Join<group::table, org::table, Inner>, Grouped<diesel::expression::operators::Eq<diesel::expression::nullable::Nullable<group::columns::org_id>, diesel::expression::nullable::Nullable<org::columns::id>>>>>>, Inner>, Grouped<diesel::expression::operators::Eq<diesel::expression::nullable::Nullable<user::columns::group_id>, diesel::expression::nullable::Nullable<group::columns::id>>>>>, query_builder::select_clause::SelectClause<(user::columns::id, group::columns::id, org::columns::id)>, DistinctOnClause<org::columns::id>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `reports` due to previous error
```

## 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:
This is caused by the following trait bound: https://github.com/diesel-rs/diesel/blob/45823853ed795cbe5a78a8a7d7d0f7a4cd7883fd/diesel/src/query_builder/select_statement/dsl_impls.rs#L267

An existing impl exists, but not for `Expr` itself, but for `OrderClause<Expr>`:
https://github.com/diesel-rs/diesel/blob/45823853ed795cbe5a78a8a7d7d0f7a4cd7883fd/diesel/src/type_impls/tuples.rs#L373

A possible fix would:
1. Change the trait bound from `Expr: ValidOrderingForDistinct<D>` to `OrderClause<Expr>: ValidOrderingForDistinct<D>`
2. Verify that all existing tests pass
3. Add a few test cases to `diesel_tests` to verify possible combinations work fine. This should include the following cases:
    + The provided example query (the join is not relevant)
    + Additional cases that use `.asc()` or `.desc()` on the first item of the order clause
This is caused by the following trait bound: https://github.com/diesel-rs/diesel/blob/45823853ed795cbe5a78a8a7d7d0f7a4cd7883fd/diesel/src/query_builder/select_statement/dsl_impls.rs#L267

An existing impl exists, but not for `Expr` itself, but for `OrderClause<Expr>`:
https://github.com/diesel-rs/diesel/blob/45823853ed795cbe5a78a8a7d7d0f7a4cd7883fd/diesel/src/type_impls/tuples.rs#L373

A possible fix would:
1. Change the trait bound from `Expr: ValidOrderingForDistinct<D>` to `OrderClause<Expr>: ValidOrderingForDistinct<D>`
2. Verify that all existing tests pass
3. Add a few test cases to `diesel_tests` to verify possible combinations work fine. This should include the following cases:
    + The provided example query (the join is not relevant)
    + Additional cases that use `.asc()` or `.desc()` on the first item of the order clause

Fix patch:
diff --git a/diesel/src/query_builder/select_statement/dsl_impls.rs b/diesel/src/query_builder/select_statement/dsl_impls.rs
index a19668367a25..124570d4469c 100644
--- a/diesel/src/query_builder/select_statement/dsl_impls.rs
+++ b/diesel/src/query_builder/select_statement/dsl_impls.rs
@@ -264,9 +264,10 @@ where
 impl<ST, F, S, D, W, O, LOf, G, H, LC, Expr> OrderDsl<Expr>
     for SelectStatement<F, S, D, W, O, LOf, G, H, LC>
 where
-    Expr: AppearsOnTable<F> + ValidOrderingForDistinct<D>,
+    Expr: AppearsOnTable<F>,
     Self: SelectQuery<SqlType = ST>,
     SelectStatement<F, S, D, W, OrderClause<Expr>, LOf, G, H, LC>: SelectQuery<SqlType = ST>,
+    OrderClause<Expr>: ValidOrderingForDistinct<D>,
 {
     type Output = SelectStatement<F, S, D, W, OrderClause<Expr>, LOf, G, H, LC>;
 


Test patch:
diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs
index c63610867139..84133d4577e2 100644
--- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs
+++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.rs
@@ -21,6 +21,12 @@ fn main() {
     // verify that we could use distinct on with an order clause that contains also a different column
     let _ = users::table.order_by((users::name, users::id)).distinct_on(users::name);
 
+    // verify that we could use distinct on with a select expression and an order clause that contains a different column
+    let _ = users::table
+        .distinct_on(users::id)
+        .select(users::id)
+        .order_by((users::id, users::name));
+
     // verify that this works also with `then_order_by`
     let _ = users::table
         .order_by(users::name)
@@ -44,7 +50,7 @@ fn main() {
         .into_boxed();
 
     // compile fail section
-    // 
+    //
     // we do not allow queries with order clauses that does not contain the distinct value
     let _ = users::table.order_by(users::id).distinct_on(users::name);
 
diff --git a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr
index 72080a4f1357..e6d5a0dcd582 100644
--- a/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr
+++ b/diesel_compile_tests/tests/fail/distinct_on_requires_matching_order_clause.stderr
@@ -1,33 +1,25 @@
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:49:58
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:55:58
    |
-49 |     let _ = users::table.order_by(users::id).distinct_on(users::name);
+55 |     let _ = users::table.order_by(users::id).distinct_on(users::name);
    |                                                          ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:53:73
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:59:73
    |
-53 |     let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name);
+59 |     let _ = users::table.order_by((users::id, users::name)).distinct_on(users::name);
    |                                                                         ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:59:22
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:65:22
    |
-59 |         .distinct_on(users::name);
+65 |         .distinct_on(users::name);
    |                      ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
-error[E0277]: the trait bound `columns::id: query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not satisfied
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:63:60
-   |
-63 |     let _ = users::table.distinct_on(users::name).order_by(users::id);
-   |                                                            ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not implemented for `columns::id`
-   |
-   = note: required because of the requirements on the impl of `OrderDsl<columns::id>` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>>`
-
 error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<columns::id>: query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not satisfied
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:63:51
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:69:51
    |
-63 |     let _ = users::table.distinct_on(users::name).order_by(users::id);
+69 |     let _ = users::table.distinct_on(users::name).order_by(users::id);
    |                                                   ^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not implemented for `diesel::query_builder::order_clause::OrderClause<columns::id>`
    |
    = help: the following implementations were found:
@@ -36,39 +28,30 @@ error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<
              <diesel::query_builder::order_clause::OrderClause<(__D, T0, T1)> as query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<__D>>>
              <diesel::query_builder::order_clause::OrderClause<(__D, T0, T1, T2)> as query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<__D>>>
            and 14 others
-   = note: required because of the requirements on the impl of `SelectQuery` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>, diesel::query_builder::where_clause::NoWhereClause, diesel::query_builder::order_clause::OrderClause<columns::id>>`
    = note: required because of the requirements on the impl of `OrderDsl<columns::id>` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>>`
 
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:66:58
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:72:58
    |
-66 |     let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed();
+72 |     let _ = users::table.order_by(users::id).distinct_on(users::name).into_boxed();
    |                                                          ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:70:22
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:76:22
    |
-70 |         .distinct_on(users::name)
+76 |         .distinct_on(users::name)
    |                      ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
 error[E0308]: mismatched types
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:77:22
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:83:22
    |
-77 |         .distinct_on(users::name)
+83 |         .distinct_on(users::name)
    |                      ^^^^^^^^^^^ expected struct `columns::id`, found struct `columns::name`
 
-error[E0277]: the trait bound `columns::id: query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not satisfied
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:84:19
-   |
-84 |         .order_by(users::id)
-   |                   ^^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not implemented for `columns::id`
-   |
-   = note: required because of the requirements on the impl of `OrderDsl<columns::id>` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>>`
-
 error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<columns::id>: query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not satisfied
-  --> tests/fail/distinct_on_requires_matching_order_clause.rs:84:10
+  --> tests/fail/distinct_on_requires_matching_order_clause.rs:90:10
    |
-84 |         .order_by(users::id)
+90 |         .order_by(users::id)
    |          ^^^^^^^^ the trait `query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<columns::name>>` is not implemented for `diesel::query_builder::order_clause::OrderClause<columns::id>`
    |
    = help: the following implementations were found:
@@ -77,5 +60,4 @@ error[E0277]: the trait bound `diesel::query_builder::order_clause::OrderClause<
              <diesel::query_builder::order_clause::OrderClause<(__D, T0, T1)> as query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<__D>>>
              <diesel::query_builder::order_clause::OrderClause<(__D, T0, T1, T2)> as query_dsl::order_dsl::ValidOrderingForDistinct<DistinctOnClause<__D>>>
            and 14 others
-   = note: required because of the requirements on the impl of `SelectQuery` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>, diesel::query_builder::where_clause::NoWhereClause, diesel::query_builder::order_clause::OrderClause<columns::id>>`
    = note: required because of the requirements on the impl of `OrderDsl<columns::id>` for `SelectStatement<FromClause<users::table>, DefaultSelectClause<FromClause<users::table>>, DistinctOnClause<columns::name>>`
diff --git a/diesel_tests/tests/distinct.rs b/diesel_tests/tests/distinct.rs
index ec81258f229b..2d84090034ee 100644
--- a/diesel_tests/tests/distinct.rs
+++ b/diesel_tests/tests/distinct.rs
@@ -66,3 +66,28 @@ fn distinct_on_select_by() {
 
     assert_eq!(expected_data, data);
 }
+
+#[cfg(feature = "postgres")]
+#[test]
+fn distinct_on_select_order_by_two_columns() {
+    use crate::schema::users::dsl::*;
+
+    let connection = &mut connection();
+    connection
+        .execute(
+            "INSERT INTO users (name, hair_color) VALUES ('Sean', 'black'), ('Sean', 'aqua'), ('Tess', 'bronze'), ('Tess', 'champagne')",
+        )
+        .unwrap();
+
+    let source = users
+        .select((name, hair_color))
+        .order((name, hair_color.desc()))
+        .distinct_on(name);
+    let expected_data = vec![
+        NewUser::new("Sean", Some("black")),
+        NewUser::new("Tess", Some("champagne")),
+    ];
+    let data: Vec<_> = source.load(connection).unwrap();
+
+    assert_eq!(expected_data, data);
+}