Merge pull request #3413 from weiznich/fix/3412

Fix #3412
diesel-rs · Nov 24, 2022 · 6ebee81 · 6ebee81
2 parents 8b68099 + a1ce341
commit 6ebee81
Show file tree

Hide file tree

Showing 5 changed files with 118 additions and 16 deletions.
diff --git 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
@@ -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_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
@@ -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
@@ -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);
+}