Merge pull request #4353 from Ten0/array_subselect

`.eq_any(ARRAY(subselect))` and `.eq_any(ARRAY[..., ...])` support

Author: weiznich

diesel/src/expression/array_comparison.rs

@@ -1,6 +1,7 @@
 //! This module contains the query dsl node definitions
 //! for array comparison operations like `IN` and `NOT IN`
 
+use super::expression_types::NotSelectable;
 use crate::backend::{sql_dialect, Backend, SqlDialect};
 use crate::expression::subselect::Subselect;
 use crate::expression::{
@@ -71,7 +72,7 @@ impl<T, U> NotIn<T, U> {
 impl<T, U> Expression for In<T, U>
 where
     T: Expression,
-    U: Expression<SqlType = T::SqlType>,
+    U: InExpression<SqlType = T::SqlType>,
     T::SqlType: SqlType,
     sql_types::is_nullable::IsSqlTypeNullable<T::SqlType>:
         sql_types::MaybeNullableType<sql_types::Bool>,
@@ -85,7 +86,7 @@ where
 impl<T, U> Expression for NotIn<T, U>
 where
     T: Expression,
-    U: Expression<SqlType = T::SqlType>,
+    U: InExpression<SqlType = T::SqlType>,
     T::SqlType: SqlType,
     sql_types::is_nullable::IsSqlTypeNullable<T::SqlType>:
         sql_types::MaybeNullableType<sql_types::Bool>,
@@ -111,7 +112,7 @@ where
     DB: Backend
         + SqlDialect<ArrayComparison = sql_dialect::array_comparison::AnsiSqlArrayComparison>,
     T: QueryFragment<DB>,
-    U: QueryFragment<DB> + MaybeEmpty,
+    U: QueryFragment<DB> + InExpression,
 {
     fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         if self.values.is_empty() {
@@ -142,7 +143,7 @@ where
     DB: Backend
         + SqlDialect<ArrayComparison = sql_dialect::array_comparison::AnsiSqlArrayComparison>,
     T: QueryFragment<DB>,
-    U: QueryFragment<DB> + MaybeEmpty,
+    U: QueryFragment<DB> + InExpression,
 {
     fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> {
         if self.values.is_empty() {
@@ -176,9 +177,9 @@ impl_selectable_expression!(NotIn<T, U>);
 ///  This trait is exposed for custom third party backends so
 ///  that they can restrict the [`QueryFragment`] implementations
 ///  for [`In`] and [`NotIn`].
-pub trait AsInExpression<T: SqlType + TypedExpressionType> {
+pub trait AsInExpression<T: SqlType> {
     /// Type of the expression returned by [AsInExpression::as_in_expression]
-    type InExpression: MaybeEmpty + Expression<SqlType = T>;
+    type InExpression: InExpression<SqlType = T>;
 
     /// Construct the diesel query dsl representation of
     /// the `IN (values)` clause for the given type
@@ -204,9 +205,15 @@ where
     }
 }
 
-/// A helper trait to check if the values clause of
-/// an [`In`] or [`NotIn`] query dsl node is empty or not
-pub trait MaybeEmpty {
+/// A marker trait that identifies query fragments that can be used in `IN(...)` and `NOT IN(...)`
+/// clauses, (or `= ANY (...)` clauses on the Postgres backend)
+///
+/// These can be wrapped in [`In`] or [`NotIn`] query dsl nodes
+pub trait InExpression {
+    /// The SQL type of the inner values, which should be the same as the left of the `IN` or
+    /// `NOT IN` clause
+    type SqlType: SqlType;
+
     /// Returns `true` if self represents an empty collection
     /// Otherwise `false` is returned.
     fn is_empty(&self) -> bool;
@@ -215,7 +222,7 @@ pub trait MaybeEmpty {
 impl<ST, F, S, D, W, O, LOf, G, H, LC> AsInExpression<ST>
     for SelectStatement<F, S, D, W, O, LOf, G, H, LC>
 where
-    ST: SqlType + TypedExpressionType,
+    ST: SqlType,
     Subselect<Self, ST>: Expression<SqlType = ST>,
     Self: SelectQuery<SqlType = ST>,
 {
@@ -228,7 +235,7 @@ where
 
 impl<'a, ST, QS, DB, GB> AsInExpression<ST> for BoxedSelectStatement<'a, ST, QS, DB, GB>
 where
-    ST: SqlType + TypedExpressionType,
+    ST: SqlType,
     Subselect<BoxedSelectStatement<'a, ST, QS, DB, GB>, ST>: Expression<SqlType = ST>,
 {
     type InExpression = Subselect<Self, ST>;
@@ -241,7 +248,7 @@ where
 impl<ST, Combinator, Rule, Source, Rhs> AsInExpression<ST>
     for CombinationClause<Combinator, Rule, Source, Rhs>
 where
-    ST: SqlType + TypedExpressionType,
+    ST: SqlType,
     Self: SelectQuery<SqlType = ST>,
     Subselect<Self, ST>: Expression<SqlType = ST>,
 {
@@ -252,8 +259,8 @@ where
     }
 }
 
-/// Query dsl node for an `IN (values)` clause containing
-/// a variable number of bind values.
+/// Query dsl node for the `values` part of an `IN (values)` clause
+/// containing a variable number of bind values.
 ///
 /// Third party backend can customize the [`QueryFragment`]
 /// implementation of this query dsl node via
@@ -284,10 +291,18 @@ impl<ST, I> Expression for Many<ST, I>
 where
     ST: TypedExpressionType,
 {
-    type SqlType = ST;
+    // Comma-ed fake expressions are not usable directly in SQL
+    // This is only implemented so that we can use the usual SelectableExpression & co traits
+    // as constraints for the same implementations on [`In`] and [`NotIn`]
+    type SqlType = NotSelectable;
 }
 
-impl<ST, I> MaybeEmpty for Many<ST, I> {
+impl<ST, I> InExpression for Many<ST, I>
+where
+    ST: SqlType,
+{
+    type SqlType = ST;
+
     fn is_empty(&self) -> bool {
         self.values.is_empty()
     }

diesel/src/expression/mod.rs

@@ -139,6 +139,9 @@ pub mod expression_types {
     ///
     /// If you see an error message containing `FromSqlRow` and this type
     /// recheck that you have written a valid select clause
+    ///
+    /// These may notably be used as intermediate Expression nodes of the query builder
+    /// which do not map to actual SQL expressions (for implementation simplicity).
     #[derive(Debug, Clone, Copy)]
     pub struct NotSelectable;
 
@@ -1041,18 +1044,12 @@ impl<'a, QS, ST, DB, GB, IsAggregate> ValidGrouping<GB>
     type IsAggregate = IsAggregate;
 }
 
-/// Converts a tuple of values into a tuple of Diesel expressions.
-///
-/// This trait is similar to [`AsExpression`], but it operates on tuples.
-/// The expressions must all be of the same SQL type.
-///
-pub trait AsExpressionList<ST> {
-    /// The final output expression
-    type Expression;
-
-    /// Perform the conversion
-    // That's public API, we cannot change
-    // that to appease clippy
-    #[allow(clippy::wrong_self_convention)]
-    fn as_expression_list(self) -> Self::Expression;
-}
+// Some amount of backwards-compat
+// We used to require `AsExpressionList` on the `array` function.
+// Now we require `IntoArrayExpression` instead, which means something very different.
+// However for most people just checking this bound to call `array`, this won't break.
+// Only people who directly implement `AsExpressionList` would break, but I expect that to be
+// nobody.
+#[doc(hidden)]
+#[cfg(feature = "postgres_backend")]
+pub use crate::pg::expression::array::IntoArrayExpression as AsExpressionList;

diesel/src/expression/subselect.rs

@@ -1,10 +1,19 @@
 use std::marker::PhantomData;
 
-use crate::expression::array_comparison::MaybeEmpty;
+use crate::expression::array_comparison::InExpression;
 use crate::expression::*;
 use crate::query_builder::*;
 use crate::result::QueryResult;
 
+/// This struct tells our type system that the whatever we put in `values`
+/// will be handled by SQL as an expression of type `ST`.
+/// It also implements the usual `SelectableExpression` and `AppearsOnTable` traits
+/// (which is useful when using this as an expression). To enforce correctness here, it checks
+/// the dedicated [`ValidSubselect`]. This however does not check that the `SqlType` of
+/// [`SelectQuery`], matches `ST`, so appropriate constraints should be checked in places that
+/// construct Subselect. (It's not always equal, notably .single_value() makes `ST` nullable, and
+/// `exists` checks bounds on `SubSelect<T, Bool>` although there is actually no such subquery in
+/// the final SQL.)
 #[derive(Debug, Copy, Clone, QueryId)]
 pub struct Subselect<T, ST> {
     values: T,
@@ -24,10 +33,12 @@ impl<T: SelectQuery, ST> Expression for Subselect<T, ST>
 where
     ST: SqlType + TypedExpressionType,
 {
+    // This is useful for `.single_value()`
     type SqlType = ST;
 }
 
-impl<T, ST> MaybeEmpty for Subselect<T, ST> {
+impl<T, ST: SqlType> InExpression for Subselect<T, ST> {
+    type SqlType = ST;
     fn is_empty(&self) -> bool {
         false
     }