Merge pull request #4264 from Aethelflaed/into-positional-expr-tuples

Implement IntoPositionalExpr for tuples
diesel-rs · Sep 18, 2024 · f7e9819 · f7e9819
2 parents 2b394ca + 4996787
commit f7e9819
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 14 deletions.
diff --git a/diesel/src/pg/expression/operators.rs b/diesel/src/pg/expression/operators.rs
@@ -4,7 +4,7 @@ use crate::expression::{TypedExpressionType, ValidGrouping};
 use crate::pg::Pg;
 use crate::query_builder::update_statement::changeset::AssignmentTarget;
 use crate::query_builder::{AstPass, QueryFragment, QueryId};
-use crate::query_dsl::positional_order_dsl::PositionalOrderExpr;
+use crate::query_dsl::positional_order_dsl::{IntoPositionalOrderExpr, PositionalOrderExpr};
 use crate::sql_types::{
     Array, Bigint, Bool, DieselNumericOps, Inet, Integer, Jsonb, SqlType, Text,
 };
@@ -75,6 +75,21 @@ __diesel_infix_operator!(
 impl<T: PositionalOrderExpr> PositionalOrderExpr for NullsFirst<T> {}
 impl<T: PositionalOrderExpr> PositionalOrderExpr for NullsLast<T> {}
 
+impl<T: PositionalOrderExpr> IntoPositionalOrderExpr for NullsFirst<T> {
+    type Output = NullsFirst<T>;
+
+    fn into_positional_expr(self) -> Self::Output {
+        self
+    }
+}
+impl<T: PositionalOrderExpr> IntoPositionalOrderExpr for NullsLast<T> {
+    type Output = NullsLast<T>;
+
+    fn into_positional_expr(self) -> Self::Output {
+        self
+    }
+}
+
 #[derive(Debug, Clone, Copy, QueryId, DieselNumericOps, ValidGrouping)]
 #[doc(hidden)]
 pub struct ArrayIndex<L, R> {

diff --git a/diesel/src/query_dsl/positional_order_dsl.rs b/diesel/src/query_dsl/positional_order_dsl.rs
@@ -23,31 +23,37 @@ pub trait PositionalOrderDsl<Expr: IntoPositionalOrderExpr> {
 
 pub trait PositionalOrderExpr: Expression {}
 
+impl PositionalOrderExpr for OrderColumn {}
+impl<T: PositionalOrderExpr> PositionalOrderExpr for Asc<T> {}
+impl<T: PositionalOrderExpr> PositionalOrderExpr for Desc<T> {}
+
 pub trait IntoPositionalOrderExpr {
     type Output: PositionalOrderExpr;
 
     fn into_positional_expr(self) -> Self::Output;
 }
 
-impl<T: PositionalOrderExpr> IntoPositionalOrderExpr for T {
-    type Output = T;
+impl IntoPositionalOrderExpr for u32 {
+    type Output = OrderColumn;
 
     fn into_positional_expr(self) -> Self::Output {
-        self
+        self.into()
     }
 }
-
-impl IntoPositionalOrderExpr for u32 {
-    type Output = OrderColumn;
+impl<T: PositionalOrderExpr> IntoPositionalOrderExpr for Asc<T> {
+    type Output = Asc<T>;
 
     fn into_positional_expr(self) -> Self::Output {
-        self.into()
+        self
     }
 }
+impl<T: PositionalOrderExpr> IntoPositionalOrderExpr for Desc<T> {
+    type Output = Desc<T>;
 
-impl PositionalOrderExpr for OrderColumn {}
-impl<T: PositionalOrderExpr> PositionalOrderExpr for Asc<T> {}
-impl<T: PositionalOrderExpr> PositionalOrderExpr for Desc<T> {}
+    fn into_positional_expr(self) -> Self::Output {
+        self
+    }
+}
 
 #[derive(Debug, Clone, Copy, QueryId)]
 pub struct OrderColumn(pub u32);
@@ -75,11 +81,23 @@ impl From<u32> for OrderColumn {
 macro_rules! impl_positional_order_expr_for_all_tuples {
     ($(
         $unused1:tt {
-            $(($idx:tt) -> $T:ident, $unused2:ident, $unused3:tt,)+
+            $(($idx:tt) -> $T:ident, $U:ident, $unused3:tt,)+
         }
     )+) => {
         $(
             impl<$($T: PositionalOrderExpr),+> PositionalOrderExpr for ($($T,)+) { }
+
+            impl<$($T, $U,)+> IntoPositionalOrderExpr for ($($T,)+)
+            where
+                $($T: IntoPositionalOrderExpr<Output = $U>,)+
+                $($U: PositionalOrderExpr,)+
+            {
+                type Output = ($($U,)+);
+
+                fn into_positional_expr(self) -> Self::Output {
+                    ($(self.$idx.into_positional_expr(),)+)
+                }
+            }
         )+
     };
 }

diff --git a/diesel_tests/tests/combination.rs b/diesel_tests/tests/combination.rs
@@ -266,12 +266,14 @@ fn positional_order_by() {
         .filter(id.le(jim.id))
         .union(users.filter(id.ge(jim.id)))
         .positional_order_by((
+            // hair color is the third column
+            // Also, we don't need OrderColumn here because .asc() is the default direction
+            #[cfg(not(feature = "postgres"))]
+            3,
             // postgres doesn't sort nulls first by default, so we need to call nulls_first().
             // This also tests whether or not NullsFirst implements PositionalOrderExpr
             #[cfg(feature = "postgres")]
             OrderColumn::from(3).asc().nulls_first(), // hair color is the third column
-            #[cfg(not(feature = "postgres"))]
-            OrderColumn::from(3).asc(), // hair color is the third column
             OrderColumn::from(2).desc(), // name is the second column
         ))
         .load(conn)