Add .then_order_by, which appends rather than replaces

I was originally only going to implement this for boxed queries, but I
don't think there's any reason we need to do that here. My original
reasoning was I didn't want to write code that appended tuples, but
there's actually no difference in behavior between `.order((foo, bar,
baz))` and `.order(((foo, bar), baz))` (which is the type we get from
this).

Fixes diesel-rs#1311.

Author: sgrif

CHANGELOG.md

@@ -36,6 +36,12 @@ for Rust libraries in [RFC #1105](https://github.com/rust-lang/rfcs/blob/master/
 * Tables with more than 56 columns are now supported by enabling the
   `128-column-tables` feature.
 
+* Added `order_by` as an alias for `order`.
+
+* Added `then_order_by`, which appends to an `ORDER BY` clause rather than
+  replacing it. This is useful with boxed queries to dynamically construct an
+  order by clause containing an unknown number of columns.
+
 ### Changed
 
 * The bounds on `impl ToSql for Cow<'a, T>` have been loosened to no longer

diesel/src/lib.rs

@@ -220,6 +220,9 @@ pub mod helper_types {
     /// Represents the return type of `.order(ordering)`
     pub type Order<Source, Ordering> = <Source as OrderDsl<Ordering>>::Output;
 
+    /// Represents the return type of `.then_order_by(ordering)`
+    pub type ThenOrderBy<Source, Ordering> = <Source as ThenOrderDsl<Ordering>>::Output;
+
     /// Represents the return type of `.limit()`
     pub type Limit<Source> = <Source as LimitDsl>::Output;
 

diesel/src/query_builder/order_clause.rs

@@ -1 +1,20 @@
 simple_clause!(NoOrderClause, OrderClause, " ORDER BY ");
+
+impl<'a, DB, Expr> Into<Option<Box<QueryFragment<DB> + 'a>>> for OrderClause<Expr>
+where
+    DB: Backend,
+    Expr: QueryFragment<DB> + 'a,
+{
+    fn into(self) -> Option<Box<QueryFragment<DB> + 'a>> {
+        Some(Box::new(self.0))
+    }
+}
+
+impl<'a, DB> Into<Option<Box<QueryFragment<DB> + 'a>>> for NoOrderClause
+where
+    DB: Backend,
+{
+    fn into(self) -> Option<Box<QueryFragment<DB> + 'a>> {
+        None
+    }
+}

diesel/src/query_builder/select_statement/boxed.rs

@@ -24,7 +24,7 @@ pub struct BoxedSelectStatement<'a, ST, QS, DB> {
     from: QS,
     distinct: Box<QueryFragment<DB> + 'a>,
     where_clause: Option<Box<QueryFragment<DB> + 'a>>,
-    order: Box<QueryFragment<DB> + 'a>,
+    order: Option<Box<QueryFragment<DB> + 'a>>,
     limit: Box<QueryFragment<DB> + 'a>,
     offset: Box<QueryFragment<DB> + 'a>,
     group_by: Box<QueryFragment<DB> + 'a>,
@@ -38,7 +38,7 @@ impl<'a, ST, QS, DB> BoxedSelectStatement<'a, ST, QS, DB> {
         from: QS,
         distinct: Box<QueryFragment<DB> + 'a>,
         where_clause: Option<Box<QueryFragment<DB> + 'a>>,
-        order: Box<QueryFragment<DB> + 'a>,
+        order: Option<Box<QueryFragment<DB> + 'a>>,
         limit: Box<QueryFragment<DB> + 'a>,
         offset: Box<QueryFragment<DB> + 'a>,
         group_by: Box<QueryFragment<DB> + 'a>,
@@ -96,7 +96,12 @@ where
         }
 
         self.group_by.walk_ast(out.reborrow())?;
-        self.order.walk_ast(out.reborrow())?;
+
+        if let Some(ref order) = self.order {
+            out.push_sql(" ORDER BY ");
+            order.walk_ast(out.reborrow())?;
+        }
+
         self.limit.walk_ast(out.reborrow())?;
         self.offset.walk_ast(out.reborrow())?;
         Ok(())
@@ -257,7 +262,23 @@ where
     type Output = Self;
 
     fn order(mut self, order: Order) -> Self::Output {
-        self.order = Box::new(OrderClause(order));
+        self.order = OrderClause(order).into();
+        self
+    }
+}
+
+impl<'a, ST, QS, DB, Order> ThenOrderDsl<Order> for BoxedSelectStatement<'a, ST, QS, DB>
+where
+    DB: Backend + 'a,
+    Order: QueryFragment<DB> + AppearsOnTable<QS> + 'a,
+{
+    type Output = Self;
+
+    fn then_order_by(mut self, order: Order) -> Self::Output {
+        self.order = match self.order {
+            Some(old) => Some(Box::new((old, order))),
+            None => Some(Box::new(order)),
+        };
         self
     }
 }

diesel/src/query_builder/select_statement/dsl_impls.rs

@@ -178,6 +178,41 @@ where
     }
 }
 
+impl<F, S, D, W, O, L, Of, G, FU, Expr> ThenOrderDsl<Expr>
+    for SelectStatement<F, S, D, W, OrderClause<O>, L, Of, G, FU>
+where
+    Expr: AppearsOnTable<F>,
+{
+    type Output = SelectStatement<F, S, D, W, OrderClause<(O, Expr)>, L, Of, G, FU>;
+
+    fn then_order_by(self, expr: Expr) -> Self::Output {
+        SelectStatement::new(
+            self.select,
+            self.from,
+            self.distinct,
+            self.where_clause,
+            OrderClause((self.order.0, expr)),
+            self.limit,
+            self.offset,
+            self.group_by,
+            self.for_update,
+        )
+    }
+}
+
+impl<F, S, D, W, L, Of, G, FU, Expr> ThenOrderDsl<Expr>
+    for SelectStatement<F, S, D, W, NoOrderClause, L, Of, G, FU>
+where
+    Expr: Expression,
+    Self: OrderDsl<Expr>,
+{
+    type Output = ::dsl::Order<Self, Expr>;
+
+    fn then_order_by(self, expr: Expr) -> Self::Output {
+        self.order_by(expr)
+    }
+}
+
 #[doc(hidden)]
 pub type Limit = AsExprOf<i64, BigInt>;
 
@@ -280,7 +315,7 @@ where
     S: QueryFragment<DB> + SelectableExpression<F> + 'a,
     D: QueryFragment<DB> + 'a,
     W: Into<Option<Box<QueryFragment<DB> + 'a>>>,
-    O: QueryFragment<DB> + 'a,
+    O: Into<Option<Box<QueryFragment<DB> + 'a>>>,
     L: QueryFragment<DB> + 'a,
     Of: QueryFragment<DB> + 'a,
     G: QueryFragment<DB> + 'a,
@@ -293,7 +328,7 @@ where
             self.from,
             Box::new(self.distinct),
             self.where_clause.into(),
-            Box::new(self.order),
+            self.order.into(),
             Box::new(self.limit),
             Box::new(self.offset),
             Box::new(self.group_by),
@@ -310,7 +345,7 @@ where
     F::DefaultSelection: QueryFragment<DB> + 'a,
     D: QueryFragment<DB> + 'a,
     W: Into<Option<Box<QueryFragment<DB> + 'a>>>,
-    O: QueryFragment<DB> + 'a,
+    O: Into<Option<Box<QueryFragment<DB> + 'a>>>,
     L: QueryFragment<DB> + 'a,
     Of: QueryFragment<DB> + 'a,
     G: QueryFragment<DB> + 'a,
@@ -323,7 +358,7 @@ where
             self.from,
             Box::new(self.distinct),
             self.where_clause.into(),
-            Box::new(self.order),
+            self.order.into(),
             Box::new(self.limit),
             Box::new(self.offset),
             Box::new(self.group_by),

diesel/src/query_dsl/mod.rs

@@ -63,7 +63,7 @@ pub mod methods {
     pub use super::load_dsl::{ExecuteDsl, LoadQuery};
     pub use super::locking_dsl::ForUpdateDsl;
     pub use super::offset_dsl::OffsetDsl;
-    pub use super::order_dsl::OrderDsl;
+    pub use super::order_dsl::{OrderDsl, ThenOrderDsl};
     pub use super::select_dsl::SelectDsl;
 }
 
@@ -562,6 +562,8 @@ pub trait QueryDsl: Sized {
     ///
     /// Ordering by multiple columns can be achieved by passing a tuple of those
     /// columns.
+    /// To construct an order clause of an unknown number of columns,
+    /// see [`QueryDsl::then_order_by`](#method.then_order_by)
     ///
     /// # Examples
     ///
@@ -613,6 +615,68 @@ pub trait QueryDsl: Sized {
         methods::OrderDsl::order(self, expr)
     }
 
+    /// Alias for `order`
+    fn order_by<Expr>(self, expr: Expr) -> Order<Self, Expr>
+    where
+        Expr: Expression,
+        Self: methods::OrderDsl<Expr>,
+    {
+        QueryDsl::order(self, expr)
+    }
+
+    /// Appends to the `ORDER BY` clause of this SQL query.
+    ///
+    /// Unlike `.order`, this method will append rather than replace.
+    /// In other words,
+    /// `.order_by(foo).order_by(bar)` is equivalent to `.order_by(bar)`.
+    /// In contrast,
+    /// `.order_by(foo).then_order_by(bar)` is equivalent to `.order((foo, bar))`.
+    /// This method is only present on boxed queries.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # #[macro_use] extern crate diesel;
+    /// # include!("../doctest_setup.rs");
+    /// #
+    /// # fn main() {
+    /// #     run_test();
+    /// # }
+    /// #
+    /// # fn run_test() -> QueryResult<()> {
+    /// #     use schema::users::dsl::*;
+    /// #     let connection = establish_connection();
+    /// #     connection.execute("DELETE FROM users")?;
+    /// diesel::insert_into(users)
+    ///     .values(&vec![
+    ///         name.eq("Saul"),
+    ///         name.eq("Steve"),
+    ///         name.eq("Stan"),
+    ///         name.eq("Stan"),
+    ///     ])
+    ///     .execute(&connection)?;
+    ///
+    /// let data = users.select((name, id))
+    ///     .order_by(name.asc())
+    ///     .then_order_by(id.desc())
+    ///     .load(&connection)?;
+    /// let expected_data = vec![
+    ///     (String::from("Saul"), 3),
+    ///     (String::from("Stan"), 6),
+    ///     (String::from("Stan"), 5),
+    ///     (String::from("Steve"), 4),
+    /// ];
+    /// assert_eq!(expected_data, data);
+    /// #    Ok(())
+    /// # }
+    /// ```
+    fn then_order_by<Order>(self, order: Order) -> ThenOrderBy<Self, Order>
+    where
+        Self: methods::ThenOrderDsl<Order>,
+    {
+        methods::ThenOrderDsl::then_order_by(self, order)
+    }
+
     /// Sets the limit clause of the query.
     ///
     /// If there was already a limit clause, it will be overridden.

diesel/src/query_dsl/order_dsl.rs

@@ -28,3 +28,31 @@ where
         self.as_query().order(expr)
     }
 }
+
+/// The `then_order_by` method
+///
+/// This trait should not be relied on directly by most apps. Its behavior is
+/// provided by [`QueryDsl`]. However, you may need a where clause on this trait
+/// to call `then_order_by` from generic code.
+///
+/// [`QueryDsl`]: ../trait.QueryDsl.html
+pub trait ThenOrderDsl<Expr> {
+    /// The type returned by `.then_order_by`.
+    type Output;
+
+    /// See the trait documentation.
+    fn then_order_by(self, expr: Expr) -> Self::Output;
+}
+
+impl<T, Expr> ThenOrderDsl<Expr> for T
+where
+    Expr: Expression,
+    T: Table,
+    T::Query: ThenOrderDsl<Expr>,
+{
+    type Output = <T::Query as ThenOrderDsl<Expr>>::Output;
+
+    fn then_order_by(self, expr: Expr) -> Self::Output {
+        self.as_query().then_order_by(expr)
+    }
+}