Skip to content

Commit

Permalink
Fix #3412
Browse files Browse the repository at this point in the history
This commits fixes an issue that disallowed the usage of
`BoxableExpression` as order clause due to a malformed trait impl in diesel.
  • Loading branch information
weiznich committed Jan 18, 2023
1 parent 96e8902 commit 793a91a
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 7 deletions.
9 changes: 7 additions & 2 deletions diesel/src/query_builder/select_statement/boxed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
14 changes: 9 additions & 5 deletions diesel/src/query_builder/select_statement/dsl_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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(
Expand Down
51 changes: 51 additions & 0 deletions diesel_tests/tests/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 793a91a

Please sign in to comment.