From 6256d45adddc31a3951e62e1104f9f838633012b Mon Sep 17 00:00:00 2001
From: Georg Semmler <github@weiznich.de>
Date: Fri, 29 Apr 2022 19:11:05 +0200
Subject: [PATCH] Fix #3151

Allow `ORDER BY` clauses to be used in any combination subquery by
either put parenthesis around the subqueries (Postgres/Mysql) or adding
a wrapping subquery (Sqlite).
---
 .../src/query_builder/combination_clause.rs   | 30 +++++++++++--------
 diesel_tests/tests/combination.rs             | 30 +++++++++++++++++++
 2 files changed, 48 insertions(+), 12 deletions(-)

diff --git a/diesel/src/query_builder/combination_clause.rs b/diesel/src/query_builder/combination_clause.rs
index 5435bd395e37..23a7002e0a6c 100644
--- a/diesel/src/query_builder/combination_clause.rs
+++ b/diesel/src/query_builder/combination_clause.rs
@@ -26,8 +26,8 @@ where
 pub struct CombinationClause<Combinator, Rule, Source, Rhs> {
     combinator: Combinator,
     duplicate_rule: Rule,
-    source: Source,
-    rhs: RhsParenthesisWrapper<Rhs>,
+    source: ParenthesisWrapper<Source>,
+    rhs: ParenthesisWrapper<Rhs>,
 }
 
 impl<Combinator, Rule, Source, Rhs> CombinationClause<Combinator, Rule, Source, Rhs> {
@@ -41,8 +41,8 @@ impl<Combinator, Rule, Source, Rhs> CombinationClause<Combinator, Rule, Source,
         CombinationClause {
             combinator,
             duplicate_rule,
-            source,
-            rhs: RhsParenthesisWrapper(rhs),
+            source: ParenthesisWrapper(source),
+            rhs: ParenthesisWrapper(rhs),
         }
     }
 }
@@ -129,8 +129,8 @@ impl<Combinator, Rule, Source, Rhs, DB: Backend> QueryFragment<DB>
 where
     Combinator: QueryFragment<DB>,
     Rule: QueryFragment<DB>,
-    Source: QueryFragment<DB>,
-    RhsParenthesisWrapper<Rhs>: QueryFragment<DB>,
+    ParenthesisWrapper<Source>: QueryFragment<DB>,
+    ParenthesisWrapper<Rhs>: QueryFragment<DB>,
     DB: Backend + SupportsCombinationClause<Combinator, Rule> + DieselReserveSpecialization,
 {
     fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
@@ -215,7 +215,7 @@ pub trait SupportsCombinationClause<Combinator, Rule> {}
 
 #[derive(Debug, Copy, Clone, QueryId)]
 /// Wrapper used to wrap rhs sql in parenthesis when supported by backend
-pub struct RhsParenthesisWrapper<T>(T);
+pub struct ParenthesisWrapper<T>(T);
 
 #[cfg(feature = "postgres")]
 mod postgres {
@@ -224,7 +224,7 @@ mod postgres {
     use crate::query_builder::{AstPass, QueryFragment};
     use crate::QueryResult;
 
-    impl<T: QueryFragment<Pg>> QueryFragment<Pg> for RhsParenthesisWrapper<T> {
+    impl<T: QueryFragment<Pg>> QueryFragment<Pg> for ParenthesisWrapper<T> {
         fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> {
             out.push_sql("(");
             self.0.walk_ast(out.reborrow())?;
@@ -248,7 +248,7 @@ mod mysql {
     use crate::query_builder::{AstPass, QueryFragment};
     use crate::QueryResult;
 
-    impl<T: QueryFragment<Mysql>> QueryFragment<Mysql> for RhsParenthesisWrapper<T> {
+    impl<T: QueryFragment<Mysql>> QueryFragment<Mysql> for ParenthesisWrapper<T> {
         fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Mysql>) -> QueryResult<()> {
             out.push_sql("(");
             self.0.walk_ast(out.reborrow())?;
@@ -268,9 +268,15 @@ mod sqlite {
     use crate::sqlite::Sqlite;
     use crate::QueryResult;
 
-    impl<T: QueryFragment<Sqlite>> QueryFragment<Sqlite> for RhsParenthesisWrapper<T> {
-        fn walk_ast<'b>(&'b self, out: AstPass<'_, 'b, Sqlite>) -> QueryResult<()> {
-            self.0.walk_ast(out) // SQLite does not support parenthesis around Ths
+    impl<T: QueryFragment<Sqlite>> QueryFragment<Sqlite> for ParenthesisWrapper<T> {
+        fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Sqlite>) -> QueryResult<()> {
+            // SQLite does not support parenthesis around Ths
+            // we can emulate this by construct a fake outer
+            // SELECT * FROM (inner_query) statement
+            out.push_sql("SELECT * FROM (");
+            self.0.walk_ast(out.reborrow())?;
+            out.push_sql(")");
+            Ok(())
         }
     }
 
diff --git a/diesel_tests/tests/combination.rs b/diesel_tests/tests/combination.rs
index 04893e26535b..262fcde7ebb5 100644
--- a/diesel_tests/tests/combination.rs
+++ b/diesel_tests/tests/combination.rs
@@ -116,3 +116,33 @@ fn except() {
         .unwrap();
     assert_eq!(expected_data, data);
 }
+
+#[test]
+fn union_with_order() {
+    let conn = &mut connection();
+    let data = vec![
+        NewUser::new("Sean", None),
+        NewUser::new("Tess", None),
+        NewUser::new("Jim", None),
+    ];
+    insert_into(users::table)
+        .values(&data)
+        .execute(conn)
+        .unwrap();
+
+    let users = users::table
+        .select(users::name)
+        .order_by(users::id.asc())
+        .limit(1)
+        .union(
+            users::table
+                .order_by(users::id.desc())
+                .select(users::name)
+                .limit(1),
+        )
+        .positional_order_by(1)
+        .load::<String>(conn)
+        .unwrap();
+
+    assert_eq!(vec![String::from("Jim"), "Sean".into()], users);
+}