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

Issue:
pack order into BoxableExpression
### Discussed in https://github.com/diesel-rs/diesel/discussions/3411

<div type='discussions-op-text'>

<sup>Originally posted by **mlesin** November 15, 2022</sup>
I'm following documentation of BoxableExpression trait, trying to pack dynamically chosen order into BoxableExpression:

```rust
    let order_field: Box<dyn BoxableExpression<tasks::table, Pg, SqlType = NotSelectable>> =
        match order.field {
            TasksOrderField::Id => {
                if order.ascending {
                    Box::new(tasks::id.asc())
                } else {
                    Box::new(tasks::id.desc())
                }
            }
            TasksOrderField::Label => {
                if order.ascending {
                    Box::new(tasks::label.asc())
                } else {
                    Box::new(tasks::label.desc())
                }
            }
        };
    query = query.order(order_field);
```
But this is fails to compile with the following message:
```
   |
74 |     query = query.order(order_field);
   |                   ^^^^^ the trait `AppearsOnTable<FromClause<tasks::table>>` is not implemented for `dyn BoxableExpression<tasks::table, Pg, SqlType = NotSelectable>`
```

And I have no idea how to satisfy AppearsOnTable<FromClause<..>> trait because FromClause trait is private.
What am I doing wrong?</div>


Discussion / hints:

Fix patch:
diff --git a/diesel/src/query_builder/select_statement/boxed.rs b/diesel/src/query_builder/select_statement/boxed.rs
index 5dc2835d6385..c6935c9fbe1d 100644
--- a/diesel/src/query_builder/select_statement/boxed.rs
+++ b/diesel/src/query_builder/select_statement/boxed.rs
@@ -389,9 +389,12 @@ where
     }
 }
 
-impl<'a, ST, QS, DB, Order, GB> OrderDsl<Order> for BoxedSelectStatement<'a, ST, QS, DB, GB>
+// no impls for `NoFromClause` here because order is not really supported there yet
+impl<'a, ST, QS, DB, Order, GB> OrderDsl<Order>
+    for BoxedSelectStatement<'a, ST, FromClause<QS>, DB, GB>
 where
     DB: Backend,
+    QS: QuerySource,
     Order: QueryFragment<DB> + AppearsOnTable<QS> + Send + 'a,
 {
     type Output = Self;
@@ -402,9 +405,11 @@ where
     }
 }
 
-impl<'a, ST, QS, DB, Order, GB> ThenOrderDsl<Order> for BoxedSelectStatement<'a, ST, QS, DB, GB>
+impl<'a, ST, QS, DB, Order, GB> ThenOrderDsl<Order>
+    for BoxedSelectStatement<'a, ST, FromClause<QS>, DB, GB>
 where
     DB: Backend + 'a,
+    QS: QuerySource,
     Order: QueryFragment<DB> + AppearsOnTable<QS> + Send + 'a,
 {
     type Output = Self;
diff --git a/diesel/src/query_builder/select_statement/dsl_impls.rs b/diesel/src/query_builder/select_statement/dsl_impls.rs
index 579f46426040..8d475dca7aa7 100644
--- a/diesel/src/query_builder/select_statement/dsl_impls.rs
+++ b/diesel/src/query_builder/select_statement/dsl_impls.rs
@@ -261,15 +261,18 @@ where
     }
 }
 
+// no impls for `NoFromClause` here because order is not really supported there yet
 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>
+    for SelectStatement<FromClause<F>, S, D, W, O, LOf, G, H, LC>
 where
+    F: QuerySource,
     Expr: AppearsOnTable<F>,
     Self: SelectQuery<SqlType = ST>,
-    SelectStatement<F, S, D, W, OrderClause<Expr>, LOf, G, H, LC>: SelectQuery<SqlType = ST>,
+    SelectStatement<FromClause<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>;
+    type Output = SelectStatement<FromClause<F>, S, D, W, OrderClause<Expr>, LOf, G, H, LC>;
 
     fn order(self, expr: Expr) -> Self::Output {
         let order = OrderClause(expr);
@@ -288,11 +291,12 @@ where
 }
 
 impl<F, S, D, W, O, LOf, G, H, LC, Expr> ThenOrderDsl<Expr>
-    for SelectStatement<F, S, D, W, OrderClause<O>, LOf, G, H, LC>
+    for SelectStatement<FromClause<F>, S, D, W, OrderClause<O>, LOf, G, H, LC>
 where
+    F: QuerySource,
     Expr: AppearsOnTable<F>,
 {
-    type Output = SelectStatement<F, S, D, W, OrderClause<(O, Expr)>, LOf, G, H, LC>;
+    type Output = SelectStatement<FromClause<F>, S, D, W, OrderClause<(O, Expr)>, LOf, G, H, LC>;
 
     fn then_order_by(self, expr: Expr) -> Self::Output {
         SelectStatement::new(
diff --git a/diesel_bench/Cargo.toml b/diesel_bench/Cargo.toml
index 7fb85129eab0..888ea1dbf1b4 100644
--- a/diesel_bench/Cargo.toml
+++ b/diesel_bench/Cargo.toml
@@ -15,7 +15,7 @@ sqlx = {version = "0.6", features = ["runtime-tokio-rustls"], optional = true}
 tokio = {version = "1", optional = true}
 rusqlite = {version = "0.27", optional = true}
 rust_postgres = {version = "0.19", optional = true, package = "postgres"}
-rust_mysql = {version = "22.1", optional = true, package = "mysql"}
+rust_mysql = {version = "23.0", optional = true, package = "mysql"}
 rustorm = {version = "0.20", optional = true}
 rustorm_dao = {version = "0.20", optional = true}
 quaint = {version = "=0.2.0-alpha.13", optional = true, features = ["uuid"]}


Test patch:
diff --git a/diesel_compile_tests/tests/fail/boxed_queries_require_selectable_expression_for_order.stderr b/diesel_compile_tests/tests/fail/boxed_queries_require_selectable_expression_for_order.stderr
index b1fac0f2cf68..be3a35f38626 100644
--- a/diesel_compile_tests/tests/fail/boxed_queries_require_selectable_expression_for_order.stderr
+++ b/diesel_compile_tests/tests/fail/boxed_queries_require_selectable_expression_for_order.stderr
@@ -1,15 +1,37 @@
-error[E0277]: the trait bound `FromClause<users::table>: AppearsInFromClause<posts::table>` is not satisfied
+error[E0271]: type mismatch resolving `<users::table as AppearsInFromClause<posts::table>>::Count == diesel::query_source::Once`
   --> tests/fail/boxed_queries_require_selectable_expression_for_order.rs:21:37
    |
 21 |     users::table.into_boxed::<Pg>().order(posts::title.desc());
-   |                                     ^^^^^ the trait `AppearsInFromClause<posts::table>` is not implemented for `FromClause<users::table>`
+   |                                     ^^^^^ expected struct `diesel::query_source::Never`, found struct `diesel::query_source::Once`
    |
-   = help: the trait `AppearsInFromClause<QS1>` is implemented for `FromClause<QS2>`
-note: required because of the requirements on the impl of `AppearsOnTable<FromClause<users::table>>` for `posts::columns::title`
+note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `posts::columns::title`
   --> tests/fail/boxed_queries_require_selectable_expression_for_order.rs:16:9
    |
 16 |         title -> VarChar,
    |         ^^^^^
    = note: 1 redundant requirement hidden
-   = note: required because of the requirements on the impl of `AppearsOnTable<FromClause<users::table>>` for `diesel::expression::operators::Desc<posts::columns::title>`
+   = note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `diesel::expression::operators::Desc<posts::columns::title>`
+   = note: required because of the requirements on the impl of `OrderDsl<diesel::expression::operators::Desc<posts::columns::title>>` for `BoxedSelectStatement<'_, (diesel::sql_types::Integer, diesel::sql_types::Text), FromClause<users::table>, Pg>`
+
+error[E0277]: the trait bound `users::table: TableNotEqual<posts::table>` is not satisfied
+  --> tests/fail/boxed_queries_require_selectable_expression_for_order.rs:21:37
+   |
+21 |     users::table.into_boxed::<Pg>().order(posts::title.desc());
+   |                                     ^^^^^ the trait `TableNotEqual<posts::table>` is not implemented for `users::table`
+   |
+   = help: the following other types implement trait `TableNotEqual<T>`:
+             <Only<pg::metadata_lookup::pg_namespace::table> as TableNotEqual<pg::metadata_lookup::pg_type::table>>
+             <Only<pg::metadata_lookup::pg_type::table> as TableNotEqual<pg::metadata_lookup::pg_namespace::table>>
+             <pg::metadata_lookup::pg_namespace::table as TableNotEqual<Only<pg::metadata_lookup::pg_type::table>>>
+             <pg::metadata_lookup::pg_namespace::table as TableNotEqual<pg::metadata_lookup::pg_type::table>>
+             <pg::metadata_lookup::pg_type::table as TableNotEqual<Only<pg::metadata_lookup::pg_namespace::table>>>
+             <pg::metadata_lookup::pg_type::table as TableNotEqual<pg::metadata_lookup::pg_namespace::table>>
+   = note: required because of the requirements on the impl of `AppearsInFromClause<posts::table>` for `users::table`
+note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `posts::columns::title`
+  --> tests/fail/boxed_queries_require_selectable_expression_for_order.rs:16:9
+   |
+16 |         title -> VarChar,
+   |         ^^^^^
+   = note: 1 redundant requirement hidden
+   = note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `diesel::expression::operators::Desc<posts::columns::title>`
    = note: required because of the requirements on the impl of `OrderDsl<diesel::expression::operators::Desc<posts::columns::title>>` for `BoxedSelectStatement<'_, (diesel::sql_types::Integer, diesel::sql_types::Text), FromClause<users::table>, Pg>`
diff --git a/diesel_compile_tests/tests/fail/order_requires_column_from_same_table.stderr b/diesel_compile_tests/tests/fail/order_requires_column_from_same_table.stderr
index ba8e84a6c33b..04a352a8761a 100644
--- a/diesel_compile_tests/tests/fail/order_requires_column_from_same_table.stderr
+++ b/diesel_compile_tests/tests/fail/order_requires_column_from_same_table.stderr
@@ -1,11 +1,31 @@
-error[E0277]: the trait bound `FromClause<users::table>: AppearsInFromClause<posts::table>` is not satisfied
+error[E0271]: type mismatch resolving `<users::table as AppearsInFromClause<posts::table>>::Count == diesel::query_source::Once`
   --> tests/fail/order_requires_column_from_same_table.rs:18:31
    |
 18 |     let source = users::table.order(posts::id);
-   |                               ^^^^^ the trait `AppearsInFromClause<posts::table>` is not implemented for `FromClause<users::table>`
+   |                               ^^^^^ expected struct `diesel::query_source::Never`, found struct `diesel::query_source::Once`
    |
-   = help: the trait `AppearsInFromClause<QS1>` is implemented for `FromClause<QS2>`
-note: required because of the requirements on the impl of `AppearsOnTable<FromClause<users::table>>` for `posts::columns::id`
+note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `posts::columns::id`
+  --> tests/fail/order_requires_column_from_same_table.rs:13:9
+   |
+13 |         id -> Integer,
+   |         ^^
+   = note: required because of the requirements on the impl of `OrderDsl<posts::columns::id>` for `SelectStatement<FromClause<users::table>>`
+
+error[E0277]: the trait bound `users::table: TableNotEqual<posts::table>` is not satisfied
+  --> tests/fail/order_requires_column_from_same_table.rs:18:31
+   |
+18 |     let source = users::table.order(posts::id);
+   |                               ^^^^^ the trait `TableNotEqual<posts::table>` is not implemented for `users::table`
+   |
+   = help: the following other types implement trait `TableNotEqual<T>`:
+             <Only<pg::metadata_lookup::pg_namespace::table> as TableNotEqual<pg::metadata_lookup::pg_type::table>>
+             <Only<pg::metadata_lookup::pg_type::table> as TableNotEqual<pg::metadata_lookup::pg_namespace::table>>
+             <pg::metadata_lookup::pg_namespace::table as TableNotEqual<Only<pg::metadata_lookup::pg_type::table>>>
+             <pg::metadata_lookup::pg_namespace::table as TableNotEqual<pg::metadata_lookup::pg_type::table>>
+             <pg::metadata_lookup::pg_type::table as TableNotEqual<Only<pg::metadata_lookup::pg_namespace::table>>>
+             <pg::metadata_lookup::pg_type::table as TableNotEqual<pg::metadata_lookup::pg_namespace::table>>
+   = note: required because of the requirements on the impl of `AppearsInFromClause<posts::table>` for `users::table`
+note: required because of the requirements on the impl of `AppearsOnTable<users::table>` for `posts::columns::id`
   --> tests/fail/order_requires_column_from_same_table.rs:13:9
    |
 13 |         id -> Integer,
diff --git a/diesel_tests/tests/order.rs b/diesel_tests/tests/order.rs
index a65f257f4059..9addd2641540 100644
--- a/diesel_tests/tests/order.rs
+++ b/diesel_tests/tests/order.rs
@@ -78,3 +78,54 @@ fn order_by_descending_column() {
     let data: Vec<_> = users.order(name.desc()).load(conn).unwrap();
     assert_eq!(expected_data, data);
 }
+
+// regression test for #3412
+#[test]
+fn dynamic_order() {
+    use crate::schema::users;
+    use diesel::expression::expression_types::NotSelectable;
+
+    let conn = &mut connection_with_sean_and_tess_in_users_table();
+    let expected = &["Tess", "Sean"] as &[_];
+
+    let order_field: Box<dyn BoxableExpression<users::table, _, SqlType = NotSelectable>> =
+        Box::new(users::id.desc());
+
+    let result = users::table
+        .select(users::name)
+        .order(order_field)
+        .load::<String>(conn)
+        .unwrap();
+    assert_eq!(expected, &result);
+
+    let order_field: Box<dyn BoxableExpression<users::table, _, SqlType = NotSelectable>> =
+        Box::new(users::id.desc());
+
+    let result = users::table
+        .select(users::name)
+        .then_order_by(order_field)
+        .load::<String>(conn)
+        .unwrap();
+    assert_eq!(expected, &result);
+
+    let order_field: Box<dyn BoxableExpression<users::table, _, SqlType = NotSelectable>> =
+        Box::new(users::id.desc());
+    let result = users::table
+        .select(users::name)
+        .into_boxed()
+        .order(order_field)
+        .load::<String>(conn)
+        .unwrap();
+    assert_eq!(expected, &result);
+
+    let order_field: Box<dyn BoxableExpression<users::table, _, SqlType = NotSelectable>> =
+        Box::new(users::id.desc());
+
+    let result = users::table
+        .select(users::name)
+        .into_boxed()
+        .then_order_by(order_field)
+        .load::<String>(conn)
+        .unwrap();
+    assert_eq!(expected, &result);
+}