From b71e8e208e187cb40156aed6e6ffe805912f41c0 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 12:11:02 +0100 Subject: [PATCH 01/23] wip --- diesel/src/pg/expression/array.rs | 156 +++++++++++++++++++++++++++--- 1 file changed, 142 insertions(+), 14 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 5b3193f0453d..98323e0322b3 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,18 +1,16 @@ +use crate::expression::subselect::Subselect; use crate::expression::{ - AppearsOnTable, AsExpressionList, Expression, SelectableExpression, ValidGrouping, + AppearsOnTable, AsExpressionList, Expression, SelectableExpression, TypedExpressionType, + ValidGrouping, }; use crate::pg::Pg; -use crate::query_builder::{AstPass, QueryFragment, QueryId}; -use crate::sql_types; +use crate::query_builder::combination_clause::CombinationClause; +use crate::query_builder::{ + AstPass, BoxedSelectStatement, QueryFragment, QueryId, SelectQuery, SelectStatement, +}; +use crate::sql_types::{self, SqlType}; use std::marker::PhantomData; -/// An ARRAY[...] literal. -#[derive(Debug, Clone, Copy, QueryId)] -pub struct ArrayLiteral { - elements: T, - _marker: PhantomData, -} - /// Creates an `ARRAY[...]` expression. /// /// The argument should be a tuple of expressions which can be represented by the @@ -43,20 +41,54 @@ pub struct ArrayLiteral { /// vec![2, 4], /// ]; /// assert_eq!(expected, ids); +/// +/// let ids = diesel::select(array(users.select(id))) +/// .get_results::>(connection)?; +/// assert_eq!(vec![1, 2], ids); /// # Ok(()) /// # } /// ``` #[cfg(feature = "postgres_backend")] -pub fn array(elements: T) -> ArrayLiteral +pub fn array(elements: T) -> >::ArrayExpression +where + T: IntoArrayExpression, + ST: SqlType + TypedExpressionType, +{ + elements.into_array_expression() +} + +pub trait IntoArrayExpression { + /// Type of the expression returned by [AsArrayExpression::as_in_expression] + type ArrayExpression: Expression>; + + /// Construct the diesel query dsl representation of + /// the `ARRAY (values)` clause for the given type + fn into_array_expression(self) -> Self::ArrayExpression; +} + +impl IntoArrayExpression for T where T: AsExpressionList, + ST: SqlType + TypedExpressionType, + T::Expression: Expression, { - ArrayLiteral { - elements: elements.as_expression_list(), - _marker: PhantomData, + type ArrayExpression = ArrayLiteral; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArrayLiteral { + elements: self.as_expression_list(), + _marker: PhantomData, + } } } +/// An ARRAY[...] literal. +#[derive(Debug, Clone, Copy, QueryId)] +pub struct ArrayLiteral { + elements: T, + _marker: PhantomData, +} + impl Expression for ArrayLiteral where ST: 'static, @@ -97,3 +129,99 @@ where { type IsAggregate = T::IsAggregate; } + +impl IntoArrayExpression + for SelectStatement +where + ST: SqlType + TypedExpressionType, + ArraySubselect: Expression>, + Self: SelectQuery, +{ + type ArrayExpression = ArraySubselect; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArraySubselect::new(self) + } +} + +impl<'a, ST, QS, DB, GB> IntoArrayExpression for BoxedSelectStatement<'a, ST, QS, DB, GB> +where + ST: SqlType + TypedExpressionType, + ArraySubselect, ST>: + Expression>, +{ + type ArrayExpression = ArraySubselect; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArraySubselect::new(self) + } +} + +impl IntoArrayExpression + for CombinationClause +where + ST: SqlType + TypedExpressionType, + Self: SelectQuery, + ArraySubselect: Expression>, +{ + type ArrayExpression = ArraySubselect; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArraySubselect::new(self) + } +} + +/// An ARRAY(...) subselect. +#[derive(Debug, Clone, Copy, QueryId)] +pub struct ArraySubselect { + elements: Subselect, +} + +impl ArraySubselect { + pub(crate) fn new(elements: T) -> Self { + Self { + elements: Subselect::new(elements), + } + } +} + +impl Expression for ArraySubselect +where + ST: 'static, + T: Expression, +{ + type SqlType = sql_types::Array; +} + +impl QueryFragment for ArraySubselect +where + T: QueryFragment, +{ + fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> crate::result::QueryResult<()> { + out.push_sql("ARRAY["); + QueryFragment::walk_ast(&self.elements, out.reborrow())?; + out.push_sql("]"); + Ok(()) + } +} + +impl SelectableExpression for ArraySubselect +where + T: SelectableExpression, + ArraySubselect: AppearsOnTable, +{ +} + +impl AppearsOnTable for ArraySubselect +where + T: AppearsOnTable, + ArraySubselect: Expression, +{ +} + +impl ValidGrouping for ArraySubselect +where + T: ValidGrouping, +{ + type IsAggregate = T::IsAggregate; +} From 91e18ee5d5d56b5286abc58bf3a62a42e4309de0 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 12:32:16 +0100 Subject: [PATCH 02/23] wip --- diesel/src/pg/expression/array.rs | 24 +++++++++++------------- diesel_tests/tests/expressions/mod.rs | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 13 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 98323e0322b3..021a3cabf19e 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -70,7 +70,6 @@ impl IntoArrayExpression for T where T: AsExpressionList, ST: SqlType + TypedExpressionType, - T::Expression: Expression, { type ArrayExpression = ArrayLiteral; @@ -92,7 +91,6 @@ pub struct ArrayLiteral { impl Expression for ArrayLiteral where ST: 'static, - T: Expression, { type SqlType = sql_types::Array; } @@ -174,13 +172,13 @@ where /// An ARRAY(...) subselect. #[derive(Debug, Clone, Copy, QueryId)] pub struct ArraySubselect { - elements: Subselect, + subquery: Subselect, } impl ArraySubselect { pub(crate) fn new(elements: T) -> Self { Self { - elements: Subselect::new(elements), + subquery: Subselect::new(elements), } } } @@ -188,40 +186,40 @@ impl ArraySubselect { impl Expression for ArraySubselect where ST: 'static, - T: Expression, + Subselect: Expression, { type SqlType = sql_types::Array; } impl QueryFragment for ArraySubselect where - T: QueryFragment, + Subselect: QueryFragment, { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> crate::result::QueryResult<()> { - out.push_sql("ARRAY["); - QueryFragment::walk_ast(&self.elements, out.reborrow())?; - out.push_sql("]"); + out.push_sql("ARRAY("); + QueryFragment::walk_ast(&self.subquery, out.reborrow())?; + out.push_sql(")"); Ok(()) } } impl SelectableExpression for ArraySubselect where - T: SelectableExpression, + Subselect: SelectableExpression, ArraySubselect: AppearsOnTable, { } impl AppearsOnTable for ArraySubselect where - T: AppearsOnTable, + Subselect: AppearsOnTable, ArraySubselect: Expression, { } impl ValidGrouping for ArraySubselect where - T: ValidGrouping, + Subselect: ValidGrouping, { - type IsAggregate = T::IsAggregate; + type IsAggregate = as ValidGrouping>::IsAggregate; } diff --git a/diesel_tests/tests/expressions/mod.rs b/diesel_tests/tests/expressions/mod.rs index 3f074c123e2d..caa8aaf816a1 100644 --- a/diesel_tests/tests/expressions/mod.rs +++ b/diesel_tests/tests/expressions/mod.rs @@ -466,6 +466,24 @@ fn test_arrays_b() { assert_eq!(value, vec![7, 14]); } +#[test] +#[cfg(feature = "postgres")] +fn test_arrays_c() { + use self::numbers::columns::*; + use self::numbers::table as numbers; + + let connection = &mut connection(); + diesel::sql_query("INSERT INTO numbers (n) VALUES (7)") + .execute(connection) + .unwrap(); + + let value = diesel::select(array(numbers.select(n))) + .first::>(connection) + .unwrap(); + + assert_eq!(value, vec![7]); +} + #[test] fn test_operator_precedence() { use self::numbers; From 351caa6d414a2b019aca62f0f5c0a74fdb33ed3a Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 12:57:17 +0100 Subject: [PATCH 03/23] fix test --- diesel/src/pg/expression/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 021a3cabf19e..b3ff4711cc2a 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -43,7 +43,7 @@ use std::marker::PhantomData; /// assert_eq!(expected, ids); /// /// let ids = diesel::select(array(users.select(id))) -/// .get_results::>(connection)?; +/// .first::>(connection)?; /// assert_eq!(vec![1, 2], ids); /// # Ok(()) /// # } From e11986ebbf9cc0314fc2c9ad5e0976b30fc4aea0 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 12:57:08 +0100 Subject: [PATCH 04/23] fix conflicting impl --- diesel/src/pg/expression/array.rs | 49 ++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index b3ff4711cc2a..4e172348d9ec 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,6 +1,6 @@ use crate::expression::subselect::Subselect; use crate::expression::{ - AppearsOnTable, AsExpressionList, Expression, SelectableExpression, TypedExpressionType, + AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, ValidGrouping, }; use crate::pg::Pg; @@ -48,7 +48,6 @@ use std::marker::PhantomData; /// # Ok(()) /// # } /// ``` -#[cfg(feature = "postgres_backend")] pub fn array(elements: T) -> >::ArrayExpression where T: IntoArrayExpression, @@ -66,21 +65,6 @@ pub trait IntoArrayExpression { fn into_array_expression(self) -> Self::ArrayExpression; } -impl IntoArrayExpression for T -where - T: AsExpressionList, - ST: SqlType + TypedExpressionType, -{ - type ArrayExpression = ArrayLiteral; - - fn into_array_expression(self) -> Self::ArrayExpression { - ArrayLiteral { - elements: self.as_expression_list(), - _marker: PhantomData, - } - } -} - /// An ARRAY[...] literal. #[derive(Debug, Clone, Copy, QueryId)] pub struct ArrayLiteral { @@ -91,6 +75,7 @@ pub struct ArrayLiteral { impl Expression for ArrayLiteral where ST: 'static, + T: Expression, { type SqlType = sql_types::Array; } @@ -223,3 +208,33 @@ where { type IsAggregate = as ValidGrouping>::IsAggregate; } + +// This has to be implemented for each tuple directly because an intermediate trait would cause +// conflicting impls. +// This is not implemented with other tuple impls because this is feature-flagged by +// `postgres-backend` +macro_rules! tuple_impls { + ($( + $Tuple:tt { + $(($idx:tt) -> $T:ident, $ST:ident, $TT:ident,)+ + } + )+) => { + $( + impl<$($T,)+ ST> IntoArrayExpression for ($($T,)+) where + $($T: AsExpression,)+ + ST: SqlType + TypedExpressionType, + { + type ArrayExpression = ArrayLiteral<($($T::Expression,)+), ST>; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArrayLiteral { + elements: ($(self.$idx.as_expression(),)+), + _marker: PhantomData, + } + } + } + )+ + } +} + +diesel_derives::__diesel_for_each_tuple!(tuple_impls); From 1fe73f53d7357c81686613ee8f8212aabc189a76 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 13:38:02 +0100 Subject: [PATCH 05/23] Attempt using AsExpression directly --- diesel/src/pg/expression/array.rs | 36 ++++++++++++++++--------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 4e172348d9ec..addd559a5996 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,3 +1,4 @@ +use crate::dsl; use crate::expression::subselect::Subselect; use crate::expression::{ AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, @@ -48,22 +49,22 @@ use std::marker::PhantomData; /// # Ok(()) /// # } /// ``` -pub fn array(elements: T) -> >::ArrayExpression +pub fn array(elements: T) -> dsl::AsExprOf> where - T: IntoArrayExpression, + T: AsExpression>, ST: SqlType + TypedExpressionType, { - elements.into_array_expression() + elements.as_expression() } -pub trait IntoArrayExpression { +/*pub trait IntoArrayExpression { /// Type of the expression returned by [AsArrayExpression::as_in_expression] type ArrayExpression: Expression>; /// Construct the diesel query dsl representation of /// the `ARRAY (values)` clause for the given type fn into_array_expression(self) -> Self::ArrayExpression; -} +}*/ /// An ARRAY[...] literal. #[derive(Debug, Clone, Copy, QueryId)] @@ -113,43 +114,44 @@ where type IsAggregate = T::IsAggregate; } -impl IntoArrayExpression +impl AsExpression> for SelectStatement where ST: SqlType + TypedExpressionType, ArraySubselect: Expression>, Self: SelectQuery, { - type ArrayExpression = ArraySubselect; + type Expression = ArraySubselect; - fn into_array_expression(self) -> Self::ArrayExpression { + fn as_expression(self) -> Self::Expression { ArraySubselect::new(self) } } -impl<'a, ST, QS, DB, GB> IntoArrayExpression for BoxedSelectStatement<'a, ST, QS, DB, GB> +impl<'a, ST, QS, DB, GB> AsExpression> + for BoxedSelectStatement<'a, ST, QS, DB, GB> where ST: SqlType + TypedExpressionType, ArraySubselect, ST>: Expression>, { - type ArrayExpression = ArraySubselect; + type Expression = ArraySubselect; - fn into_array_expression(self) -> Self::ArrayExpression { + fn as_expression(self) -> Self::Expression { ArraySubselect::new(self) } } -impl IntoArrayExpression +impl AsExpression> for CombinationClause where ST: SqlType + TypedExpressionType, Self: SelectQuery, ArraySubselect: Expression>, { - type ArrayExpression = ArraySubselect; + type Expression = ArraySubselect; - fn into_array_expression(self) -> Self::ArrayExpression { + fn as_expression(self) -> Self::Expression { ArraySubselect::new(self) } } @@ -220,13 +222,13 @@ macro_rules! tuple_impls { } )+) => { $( - impl<$($T,)+ ST> IntoArrayExpression for ($($T,)+) where + impl<$($T,)+ ST> AsExpression> for ($($T,)+) where $($T: AsExpression,)+ ST: SqlType + TypedExpressionType, { - type ArrayExpression = ArrayLiteral<($($T::Expression,)+), ST>; + type Expression = ArrayLiteral<($($T::Expression,)+), ST>; - fn into_array_expression(self) -> Self::ArrayExpression { + fn as_expression(self) -> Self::Expression { ArrayLiteral { elements: ($(self.$idx.as_expression(),)+), _marker: PhantomData, From 2e1f4dc39d2ceb2485202807c50e2c9164e3c959 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 13:38:06 +0100 Subject: [PATCH 06/23] Revert "Attempt using AsExpression directly" This reverts commit 1fe73f53d7357c81686613ee8f8212aabc189a76. --- diesel/src/pg/expression/array.rs | 36 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index addd559a5996..4e172348d9ec 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,4 +1,3 @@ -use crate::dsl; use crate::expression::subselect::Subselect; use crate::expression::{ AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, @@ -49,22 +48,22 @@ use std::marker::PhantomData; /// # Ok(()) /// # } /// ``` -pub fn array(elements: T) -> dsl::AsExprOf> +pub fn array(elements: T) -> >::ArrayExpression where - T: AsExpression>, + T: IntoArrayExpression, ST: SqlType + TypedExpressionType, { - elements.as_expression() + elements.into_array_expression() } -/*pub trait IntoArrayExpression { +pub trait IntoArrayExpression { /// Type of the expression returned by [AsArrayExpression::as_in_expression] type ArrayExpression: Expression>; /// Construct the diesel query dsl representation of /// the `ARRAY (values)` clause for the given type fn into_array_expression(self) -> Self::ArrayExpression; -}*/ +} /// An ARRAY[...] literal. #[derive(Debug, Clone, Copy, QueryId)] @@ -114,44 +113,43 @@ where type IsAggregate = T::IsAggregate; } -impl AsExpression> +impl IntoArrayExpression for SelectStatement where ST: SqlType + TypedExpressionType, ArraySubselect: Expression>, Self: SelectQuery, { - type Expression = ArraySubselect; + type ArrayExpression = ArraySubselect; - fn as_expression(self) -> Self::Expression { + fn into_array_expression(self) -> Self::ArrayExpression { ArraySubselect::new(self) } } -impl<'a, ST, QS, DB, GB> AsExpression> - for BoxedSelectStatement<'a, ST, QS, DB, GB> +impl<'a, ST, QS, DB, GB> IntoArrayExpression for BoxedSelectStatement<'a, ST, QS, DB, GB> where ST: SqlType + TypedExpressionType, ArraySubselect, ST>: Expression>, { - type Expression = ArraySubselect; + type ArrayExpression = ArraySubselect; - fn as_expression(self) -> Self::Expression { + fn into_array_expression(self) -> Self::ArrayExpression { ArraySubselect::new(self) } } -impl AsExpression> +impl IntoArrayExpression for CombinationClause where ST: SqlType + TypedExpressionType, Self: SelectQuery, ArraySubselect: Expression>, { - type Expression = ArraySubselect; + type ArrayExpression = ArraySubselect; - fn as_expression(self) -> Self::Expression { + fn into_array_expression(self) -> Self::ArrayExpression { ArraySubselect::new(self) } } @@ -222,13 +220,13 @@ macro_rules! tuple_impls { } )+) => { $( - impl<$($T,)+ ST> AsExpression> for ($($T,)+) where + impl<$($T,)+ ST> IntoArrayExpression for ($($T,)+) where $($T: AsExpression,)+ ST: SqlType + TypedExpressionType, { - type Expression = ArrayLiteral<($($T::Expression,)+), ST>; + type ArrayExpression = ArrayLiteral<($($T::Expression,)+), ST>; - fn as_expression(self) -> Self::Expression { + fn into_array_expression(self) -> Self::ArrayExpression { ArrayLiteral { elements: ($(self.$idx.as_expression(),)+), _marker: PhantomData, From 801da3a187480a49465c3f42462c61bff1f5549a Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 13:48:40 +0100 Subject: [PATCH 07/23] Improve doc of the ~deprecated AsArrayExpression --- diesel/src/pg/expression/array_comparison.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/diesel/src/pg/expression/array_comparison.rs b/diesel/src/pg/expression/array_comparison.rs index 6ccd6ced304f..28188aa7492d 100644 --- a/diesel/src/pg/expression/array_comparison.rs +++ b/diesel/src/pg/expression/array_comparison.rs @@ -130,6 +130,17 @@ where impl_selectable_expression!(All); +/// Deprecated trait used for implementing `any` and `all` (which are themselves deprecated). +/// +/// It has several quirks: +/// - `All>>` pretends to be an expression of type `ST`, +/// but it's in fact some custom that can really be only used in combination with the `=` +/// operator in some select places. +/// - Implementations that use Subelect below are lying: they pretend to be expressions of type +/// `Array`, but they're actually subselects, which are processed differently by Postgres +/// and may result in different query plans. +/// The `IntoArrayExpression` trait represents this more accurately, actually building an +/// actual expression of type array from a subselect (by wrapping it in `ARRAY(subselect)`). pub trait AsArrayExpression { type Expression: Expression>; From 7e6eddf9447ef191c53ac488b7a11d97c9579278 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:02:11 +0100 Subject: [PATCH 08/23] improve doc --- diesel/src/pg/expression/array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 4e172348d9ec..e3be67ea2841 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -11,10 +11,10 @@ use crate::query_builder::{ use crate::sql_types::{self, SqlType}; use std::marker::PhantomData; -/// Creates an `ARRAY[...]` expression. +/// Creates an `ARRAY[e1, e2, ...]` or `ARRAY(subselect)` expression. /// /// The argument should be a tuple of expressions which can be represented by the -/// same SQL type. +/// same SQL type, or a subquery. /// /// # Examples /// From 92eaf0df80d9bf0064252f8df27ffa94dba8a13d Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:06:11 +0100 Subject: [PATCH 09/23] Add helper type auto_type will work only if ST is explicitly specified when calling array, which looks reasonable --- diesel/src/pg/expression/array.rs | 3 ++- diesel/src/pg/expression/helper_types.rs | 6 ++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index e3be67ea2841..17cdd3015708 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,3 +1,4 @@ +use crate::dsl; use crate::expression::subselect::Subselect; use crate::expression::{ AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, @@ -48,7 +49,7 @@ use std::marker::PhantomData; /// # Ok(()) /// # } /// ``` -pub fn array(elements: T) -> >::ArrayExpression +pub fn array(elements: T) -> dsl::array where T: IntoArrayExpression, ST: SqlType + TypedExpressionType, diff --git a/diesel/src/pg/expression/helper_types.rs b/diesel/src/pg/expression/helper_types.rs index 42b51f785b2b..71cb8c62d586 100644 --- a/diesel/src/pg/expression/helper_types.rs +++ b/diesel/src/pg/expression/helper_types.rs @@ -1,6 +1,7 @@ use crate::dsl::{AsExpr, AsExprOf, SqlTypeOf}; use crate::expression::grouped::Grouped; use crate::expression::Expression; +use crate::pg::expression::array::IntoArrayExpression; use crate::pg::expression::expression_methods::private::{JsonIndex, JsonRemoveIndex}; use crate::pg::types::sql_types::Array; use crate::sql_types::{Inet, Integer, VarChar}; @@ -340,6 +341,11 @@ pub type NotLikeBinary = crate::dsl::NotLike; #[deprecated(note = "Use `dsl::Concat` instead")] pub type ConcatArray = crate::dsl::Concat; +/// Return type of [`array(tuple_or_subselect)`](super::dsl::array) +#[allow(non_camel_case_types)] +#[cfg(feature = "postgres_backend")] +pub type array = >::ArrayExpression; + /// Return type of [`array_to_string_with_null_string(arr, delim, null_str)`](super::functions::array_to_string_with_null_string) #[allow(non_camel_case_types)] #[cfg(feature = "postgres_backend")] From 07c6f34b05f3cd83166b0508888cc9d04d5ef3c0 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:20:11 +0100 Subject: [PATCH 10/23] move helper type to same path --- diesel/src/pg/expression/array.rs | 6 ++++++ diesel/src/pg/expression/helper_types.rs | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 17cdd3015708..14279661815a 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -57,6 +57,12 @@ where elements.into_array_expression() } +/// Return type of [`array(tuple_or_subselect)`](super::dsl::array) +#[allow(non_camel_case_types)] +#[cfg(feature = "postgres_backend")] +pub type array = >::ArrayExpression; + +/// Trait for types which can be converted into an expression of type `Array` pub trait IntoArrayExpression { /// Type of the expression returned by [AsArrayExpression::as_in_expression] type ArrayExpression: Expression>; diff --git a/diesel/src/pg/expression/helper_types.rs b/diesel/src/pg/expression/helper_types.rs index 71cb8c62d586..42b51f785b2b 100644 --- a/diesel/src/pg/expression/helper_types.rs +++ b/diesel/src/pg/expression/helper_types.rs @@ -1,7 +1,6 @@ use crate::dsl::{AsExpr, AsExprOf, SqlTypeOf}; use crate::expression::grouped::Grouped; use crate::expression::Expression; -use crate::pg::expression::array::IntoArrayExpression; use crate::pg::expression::expression_methods::private::{JsonIndex, JsonRemoveIndex}; use crate::pg::types::sql_types::Array; use crate::sql_types::{Inet, Integer, VarChar}; @@ -341,11 +340,6 @@ pub type NotLikeBinary = crate::dsl::NotLike; #[deprecated(note = "Use `dsl::Concat` instead")] pub type ConcatArray = crate::dsl::Concat; -/// Return type of [`array(tuple_or_subselect)`](super::dsl::array) -#[allow(non_camel_case_types)] -#[cfg(feature = "postgres_backend")] -pub type array = >::ArrayExpression; - /// Return type of [`array_to_string_with_null_string(arr, delim, null_str)`](super::functions::array_to_string_with_null_string) #[allow(non_camel_case_types)] #[cfg(feature = "postgres_backend")] From e28a82695e15a65b9a4de229a7a75cbc9511cff5 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:21:29 +0100 Subject: [PATCH 11/23] Make it backwards-compatible for people who checked the AsExpressionList bound --- diesel/src/expression/mod.rs | 24 +++++++++--------------- diesel/src/type_impls/tuples.rs | 16 ++-------------- 2 files changed, 11 insertions(+), 29 deletions(-) diff --git a/diesel/src/expression/mod.rs b/diesel/src/expression/mod.rs index 2ef4fe20d03a..65a5fbad89c1 100644 --- a/diesel/src/expression/mod.rs +++ b/diesel/src/expression/mod.rs @@ -1041,18 +1041,12 @@ impl<'a, QS, ST, DB, GB, IsAggregate> ValidGrouping 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 { - /// 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; diff --git a/diesel/src/type_impls/tuples.rs b/diesel/src/type_impls/tuples.rs index a59d273ed7bd..fadbe83daee9 100644 --- a/diesel/src/type_impls/tuples.rs +++ b/diesel/src/type_impls/tuples.rs @@ -4,9 +4,8 @@ use crate::deserialize::{ self, FromSqlRow, FromStaticSqlRow, Queryable, SqlTypeOrSelectable, StaticallySizedRow, }; use crate::expression::{ - is_contained_in_group_by, AppearsOnTable, AsExpression, AsExpressionList, Expression, - IsContainedInGroupBy, MixedAggregates, QueryMetadata, Selectable, SelectableExpression, - TypedExpressionType, ValidGrouping, + is_contained_in_group_by, AppearsOnTable, Expression, IsContainedInGroupBy, MixedAggregates, + QueryMetadata, Selectable, SelectableExpression, TypedExpressionType, ValidGrouping, }; use crate::insertable::{CanInsertInSingleQuery, InsertValues, Insertable, InsertableOptionHelper}; use crate::query_builder::*; @@ -239,17 +238,6 @@ macro_rules! tuple_impls { } } - impl<$($T,)+ ST> AsExpressionList for ($($T,)+) where - $($T: AsExpression,)+ - ST: SqlType + TypedExpressionType, - { - type Expression = ($($T::Expression,)+); - - fn as_expression_list(self) -> Self::Expression { - ($(self.$idx.as_expression(),)+) - } - } - impl_sql_type!($($T,)*); impl<$($T,)* __DB, $($ST,)*> Queryable<($($ST,)*), __DB> for ($($T,)*) From 2294ea22a847202fddf989f047d76c4f289fa56f Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:27:42 +0100 Subject: [PATCH 12/23] Improve doc & export trait --- diesel/src/pg/expression/array.rs | 2 ++ diesel/src/pg/expression/mod.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 14279661815a..c6cb072dff8f 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -63,6 +63,8 @@ where pub type array = >::ArrayExpression; /// Trait for types which can be converted into an expression of type `Array` +/// +/// This includes tuples of expressions with the same SQL type, and subselects with a single column. pub trait IntoArrayExpression { /// Type of the expression returned by [AsArrayExpression::as_in_expression] type ArrayExpression: Expression>; diff --git a/diesel/src/pg/expression/mod.rs b/diesel/src/pg/expression/mod.rs index 9a994541102b..4d2385a64a0c 100644 --- a/diesel/src/pg/expression/mod.rs +++ b/diesel/src/pg/expression/mod.rs @@ -27,7 +27,7 @@ pub mod dsl { pub use super::array_comparison::{all, any}; #[doc(inline)] - pub use super::array::array; + pub use super::array::{array, IntoArrayExpression}; #[doc(inline)] pub use super::extensions::*; From 7731f839afa142f4724f2970585bf8b4811a4e83 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:30:54 +0100 Subject: [PATCH 13/23] Add impl of `IntoArrayExpression` for types that have `AsExpression>` --- diesel/src/pg/expression/array.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index c6cb072dff8f..6ae8e62f7319 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -74,6 +74,18 @@ pub trait IntoArrayExpression { fn into_array_expression(self) -> Self::ArrayExpression; } +impl IntoArrayExpression for T +where + T: AsExpression>, + ST: SqlType + TypedExpressionType + 'static, +{ + type ArrayExpression = >>::Expression; + + fn into_array_expression(self) -> Self::ArrayExpression { + >>::as_expression(self) + } +} + /// An ARRAY[...] literal. #[derive(Debug, Clone, Copy, QueryId)] pub struct ArrayLiteral { From a8f5cd5af80b10087fa1e6f6b7bb2bb2d929ab5a Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:36:25 +0100 Subject: [PATCH 14/23] Update trybuild Unfortunately this keeps talking about AsExpressionList instead of IntoArrayExpression :( Hopefully Rust ends up fixing this --- ...ay_expressions_must_be_correct_type.stderr | 23 ++++++++-- ...array_expressions_must_be_same_type.stderr | 46 ++++++++++++++++--- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr b/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr index 1e8f274513e1..1713d4d2b459 100644 --- a/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr +++ b/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr @@ -217,8 +217,25 @@ error[E0277]: the trait bound `f64: diesel::Expression` is not satisfied note: required by a bound in `diesel::dsl::array` --> $DIESEL/src/pg/expression/array.rs | - | pub fn array(elements: T) -> ArrayLiteral + | pub fn array(elements: T) -> dsl::array | ----- required by a bound in this function | where - | T: AsExpressionList, - | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` + | T: IntoArrayExpression, + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` + +error[E0277]: the trait bound `(f64, f64): AsExpressionList` is not satisfied + --> tests/fail/array_expressions_must_be_correct_type.rs:9:12 + | +9 | select(array((1f64, 3f64))).get_result::>(&mut connection); + | ^^^^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `(f64, f64)` + | + = help: the following other types implement trait `AsExpressionList`: + (T0, T1) + (T0, T1, T2) + (T0, T1, T2, T3) + (T0, T1, T2, T3, T4) + (T0, T1, T2, T3, T4, T5) + (T0, T1, T2, T3, T4, T5, T6) + (T0, T1, T2, T3, T4, T5, T6, T7) + (T0, T1, T2, T3, T4, T5, T6, T7, T8) + and $N others diff --git a/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr b/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr index 10b7998e4468..6b24738f31bf 100644 --- a/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr +++ b/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr @@ -217,11 +217,11 @@ error[E0277]: the trait bound `f64: diesel::Expression` is not satisfied note: required by a bound in `diesel::dsl::array` --> $DIESEL/src/pg/expression/array.rs | - | pub fn array(elements: T) -> ArrayLiteral + | pub fn array(elements: T) -> dsl::array | ----- required by a bound in this function | where - | T: AsExpressionList, - | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` + | T: IntoArrayExpression, + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` error[E0277]: Cannot select `{integer}` from `NoFromClause` --> tests/fail/array_expressions_must_be_same_type.rs:18:12 @@ -442,8 +442,42 @@ error[E0277]: the trait bound `{integer}: diesel::Expression` is not satisfied note: required by a bound in `diesel::dsl::array` --> $DIESEL/src/pg/expression/array.rs | - | pub fn array(elements: T) -> ArrayLiteral + | pub fn array(elements: T) -> dsl::array | ----- required by a bound in this function | where - | T: AsExpressionList, - | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` + | T: IntoArrayExpression, + | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` + +error[E0277]: the trait bound `(i32, f64): AsExpressionList` is not satisfied + --> tests/fail/array_expressions_must_be_same_type.rs:15:12 + | +15 | select(array((1, 3f64))) + | ^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `(i32, f64)` + | + = help: the following other types implement trait `AsExpressionList`: + (T0, T1) + (T0, T1, T2) + (T0, T1, T2, T3) + (T0, T1, T2, T3, T4) + (T0, T1, T2, T3, T4, T5) + (T0, T1, T2, T3, T4, T5, T6) + (T0, T1, T2, T3, T4, T5, T6, T7) + (T0, T1, T2, T3, T4, T5, T6, T7, T8) + and $N others + +error[E0277]: the trait bound `({integer}, f64): AsExpressionList` is not satisfied + --> tests/fail/array_expressions_must_be_same_type.rs:18:12 + | +18 | select(array((1, 3f64))) + | ^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `({integer}, f64)` + | + = help: the following other types implement trait `AsExpressionList`: + (T0, T1) + (T0, T1, T2) + (T0, T1, T2, T3) + (T0, T1, T2, T3, T4) + (T0, T1, T2, T3, T4, T5) + (T0, T1, T2, T3, T4, T5, T6) + (T0, T1, T2, T3, T4, T5, T6, T7) + (T0, T1, T2, T3, T4, T5, T6, T7, T8) + and $N others From 079f403253ec7433aab97d93b7b9e4bfb98a8d8b Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:43:22 +0100 Subject: [PATCH 15/23] Improve error message --- diesel/src/pg/expression/array.rs | 5 +++++ .../fail/array_expressions_must_be_correct_type.stderr | 3 ++- .../tests/fail/array_expressions_must_be_same_type.stderr | 6 ++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 6ae8e62f7319..a368fa5aa45c 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -65,6 +65,11 @@ pub type array = >::ArrayExpression; /// Trait for types which can be converted into an expression of type `Array` /// /// This includes tuples of expressions with the same SQL type, and subselects with a single column. +#[diagnostic::on_unimplemented( + message = "Cannot convert `{Self}` into an expression of type `Array<{ST}>`", + note = "`the trait bound `{Self}: IntoArrayExpression<{ST}>` is not satisfied. \ + (`AsExpressionList` is a deprecated trait alias for `IntoArrayExpression`)" +)] pub trait IntoArrayExpression { /// Type of the expression returned by [AsArrayExpression::as_in_expression] type ArrayExpression: Expression>; diff --git a/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr b/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr index 1713d4d2b459..e21fcbbae491 100644 --- a/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr +++ b/diesel_compile_tests/tests/fail/array_expressions_must_be_correct_type.stderr @@ -223,12 +223,13 @@ note: required by a bound in `diesel::dsl::array` | T: IntoArrayExpression, | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` -error[E0277]: the trait bound `(f64, f64): AsExpressionList` is not satisfied +error[E0277]: Cannot convert `(f64, f64)` into an expression of type `Array` --> tests/fail/array_expressions_must_be_correct_type.rs:9:12 | 9 | select(array((1f64, 3f64))).get_result::>(&mut connection); | ^^^^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `(f64, f64)` | + = note: `the trait bound `(f64, f64): IntoArrayExpression` is not satisfied. (`AsExpressionList` is a deprecated trait alias for `IntoArrayExpression`) = help: the following other types implement trait `AsExpressionList`: (T0, T1) (T0, T1, T2) diff --git a/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr b/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr index 6b24738f31bf..ffd93cdb204e 100644 --- a/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr +++ b/diesel_compile_tests/tests/fail/array_expressions_must_be_same_type.stderr @@ -448,12 +448,13 @@ note: required by a bound in `diesel::dsl::array` | T: IntoArrayExpression, | ^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `array` -error[E0277]: the trait bound `(i32, f64): AsExpressionList` is not satisfied +error[E0277]: Cannot convert `(i32, f64)` into an expression of type `Array` --> tests/fail/array_expressions_must_be_same_type.rs:15:12 | 15 | select(array((1, 3f64))) | ^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `(i32, f64)` | + = note: `the trait bound `(i32, f64): IntoArrayExpression` is not satisfied. (`AsExpressionList` is a deprecated trait alias for `IntoArrayExpression`) = help: the following other types implement trait `AsExpressionList`: (T0, T1) (T0, T1, T2) @@ -465,12 +466,13 @@ error[E0277]: the trait bound `(i32, f64): AsExpressionList` is not satisfied +error[E0277]: Cannot convert `({integer}, f64)` into an expression of type `Array` --> tests/fail/array_expressions_must_be_same_type.rs:18:12 | 18 | select(array((1, 3f64))) | ^^^^^^^^^^^^^^^^ the trait `AsExpressionList` is not implemented for `({integer}, f64)` | + = note: `the trait bound `({integer}, f64): IntoArrayExpression` is not satisfied. (`AsExpressionList` is a deprecated trait alias for `IntoArrayExpression`) = help: the following other types implement trait `AsExpressionList`: (T0, T1) (T0, T1, T2) From 4877b592f3cf66e55ba8e1efb57494d9845566e9 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:47:49 +0100 Subject: [PATCH 16/23] improve test --- diesel_tests/tests/expressions/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/diesel_tests/tests/expressions/mod.rs b/diesel_tests/tests/expressions/mod.rs index caa8aaf816a1..5423cf56843c 100644 --- a/diesel_tests/tests/expressions/mod.rs +++ b/diesel_tests/tests/expressions/mod.rs @@ -473,15 +473,15 @@ fn test_arrays_c() { use self::numbers::table as numbers; let connection = &mut connection(); - diesel::sql_query("INSERT INTO numbers (n) VALUES (7)") + diesel::sql_query("INSERT INTO numbers (n) VALUES (7), (8)") .execute(connection) .unwrap(); - let value = diesel::select(array(numbers.select(n))) + let value = diesel::select(array(numbers.select(n).order_by(n))) .first::>(connection) .unwrap(); - assert_eq!(value, vec![7]); + assert_eq!(value, vec![7, 8]); } #[test] From 488d3b034e2cd75669ca12986a60f4c6de338935 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:50:37 +0100 Subject: [PATCH 17/23] add doc --- diesel/src/pg/expression/array.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index a368fa5aa45c..3cebec41d883 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -79,6 +79,7 @@ pub trait IntoArrayExpression { fn into_array_expression(self) -> Self::ArrayExpression; } +/// Implement as a no-op for things that were already arrays impl IntoArrayExpression for T where T: AsExpression>, From 6929b8677e9690a1fc594ead19c94be4aef65f48 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 14:51:37 +0100 Subject: [PATCH 18/23] improve comment --- diesel/src/pg/expression/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 3cebec41d883..c44dec971d10 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -79,7 +79,7 @@ pub trait IntoArrayExpression { fn into_array_expression(self) -> Self::ArrayExpression; } -/// Implement as a no-op for things that were already arrays +/// Implement as a no-op for things that were already arrays (that is, don't wrap in ARRAY[]). impl IntoArrayExpression for T where T: AsExpression>, From 97d5627103225e50e66636a1a7eba8153f6e9a3d Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 17:30:17 +0100 Subject: [PATCH 19/23] Allow using explicit array constructs in EqAny This removes the fake Expression implementation for `Many`, instead relying on an `InExpression` trait (which replaces `MaybeEmpty`) that represents that that query fragment is valid for use inside `IN` or `= ANY` --- diesel/src/expression/array_comparison.rs | 54 ++++++---- diesel/src/expression/mod.rs | 3 + diesel/src/expression/subselect.rs | 11 ++- diesel/src/pg/expression/array.rs | 98 ++++++++++++++----- .../pg/query_builder/query_fragment_impls.rs | 6 +- diesel_tests/tests/filter_operators.rs | 64 ++++++++++++ 6 files changed, 184 insertions(+), 52 deletions(-) diff --git a/diesel/src/expression/array_comparison.rs b/diesel/src/expression/array_comparison.rs index bade87beea01..66ffd11ff89a 100644 --- a/diesel/src/expression/array_comparison.rs +++ b/diesel/src/expression/array_comparison.rs @@ -1,9 +1,8 @@ //! This module contains the query dsl node definitions //! for array comparison operations like `IN` and `NOT IN` -use crate::backend::sql_dialect; -use crate::backend::Backend; -use crate::backend::SqlDialect; +use super::expression_types::NotSelectable; +use crate::backend::{sql_dialect, Backend, SqlDialect}; use crate::expression::subselect::Subselect; use crate::expression::{ AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, @@ -15,8 +14,7 @@ use crate::query_builder::{ }; use crate::result::QueryResult; use crate::serialize::ToSql; -use crate::sql_types::HasSqlType; -use crate::sql_types::{Bool, SingleValue, SqlType}; +use crate::sql_types::{Bool, HasSqlType, SingleValue, SqlType}; use std::marker::PhantomData; /// Query dsl node that represents a `left IN (values)` @@ -74,7 +72,7 @@ impl NotIn { impl Expression for In where T: Expression, - U: Expression, + U: InExpression, { type SqlType = Bool; } @@ -82,7 +80,7 @@ where impl Expression for NotIn where T: Expression, - U: Expression, + U: InExpression, { type SqlType = Bool; } @@ -102,7 +100,7 @@ where DB: Backend + SqlDialect, T: QueryFragment, - U: QueryFragment + MaybeEmpty, + U: QueryFragment + InExpression, { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> { if self.values.is_empty() { @@ -133,7 +131,7 @@ where DB: Backend + SqlDialect, T: QueryFragment, - U: QueryFragment + MaybeEmpty, + U: QueryFragment + InExpression, { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, DB>) -> QueryResult<()> { if self.values.is_empty() { @@ -167,9 +165,9 @@ impl_selectable_expression!(NotIn); /// This trait is exposed for custom third party backends so /// that they can restrict the [`QueryFragment`] implementations /// for [`In`] and [`NotIn`]. -pub trait AsInExpression { +pub trait AsInExpression { /// Type of the expression returned by [AsInExpression::as_in_expression] - type InExpression: MaybeEmpty + Expression; + type InExpression: InExpression; /// Construct the diesel query dsl representation of /// the `IN (values)` clause for the given type @@ -195,9 +193,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; @@ -206,7 +210,7 @@ pub trait MaybeEmpty { impl AsInExpression for SelectStatement where - ST: SqlType + TypedExpressionType, + ST: SqlType, Subselect: Expression, Self: SelectQuery, { @@ -219,7 +223,7 @@ where impl<'a, ST, QS, DB, GB> AsInExpression for BoxedSelectStatement<'a, ST, QS, DB, GB> where - ST: SqlType + TypedExpressionType, + ST: SqlType, Subselect, ST>: Expression, { type InExpression = Subselect; @@ -232,7 +236,7 @@ where impl AsInExpression for CombinationClause where - ST: SqlType + TypedExpressionType, + ST: SqlType, Self: SelectQuery, Subselect: Expression, { @@ -243,8 +247,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 @@ -275,10 +279,18 @@ impl Expression for Many 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 MaybeEmpty for Many { +impl InExpression for Many +where + ST: SqlType, +{ + type SqlType = ST; + fn is_empty(&self) -> bool { self.values.is_empty() } diff --git a/diesel/src/expression/mod.rs b/diesel/src/expression/mod.rs index 65a5fbad89c1..a1340528cca0 100644 --- a/diesel/src/expression/mod.rs +++ b/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; diff --git a/diesel/src/expression/subselect.rs b/diesel/src/expression/subselect.rs index 9bff4424a9f2..91cd5a715795 100644 --- a/diesel/src/expression/subselect.rs +++ b/diesel/src/expression/subselect.rs @@ -1,10 +1,15 @@ 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`] trait. #[derive(Debug, Copy, Clone, QueryId)] pub struct Subselect { values: T, @@ -24,10 +29,12 @@ impl Expression for Subselect where ST: SqlType + TypedExpressionType, { + // This is useful for `.single_value()` type SqlType = ST; } -impl MaybeEmpty for Subselect { +impl InExpression for Subselect { + type SqlType = ST; fn is_empty(&self) -> bool { false } diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index c44dec971d10..37b6dca50c83 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -1,4 +1,5 @@ use crate::dsl; +use crate::expression::array_comparison::{AsInExpression, InExpression}; use crate::expression::subselect::Subselect; use crate::expression::{ AppearsOnTable, AsExpression, Expression, SelectableExpression, TypedExpressionType, @@ -92,6 +93,37 @@ where } } +// This has to be implemented for each tuple directly because an intermediate trait would cause +// conflicting impls. (Compiler says people could implement AsExpression for +// SelectStatement) +// This is not implemented with other tuple impls because this is feature-flagged by +// `postgres-backend` +macro_rules! tuple_impls { + ($( + $Tuple:tt { + $(($idx:tt) -> $T:ident, $ST:ident, $TT:ident,)+ + } + )+) => { + $( + impl<$($T,)+ ST> IntoArrayExpression for ($($T,)+) where + $($T: AsExpression,)+ + ST: SqlType + TypedExpressionType, + { + type ArrayExpression = ArrayLiteral<($($T::Expression,)+), ST>; + + fn into_array_expression(self) -> Self::ArrayExpression { + ArrayLiteral { + elements: ($(self.$idx.as_expression(),)+), + _marker: PhantomData, + } + } + } + )+ + } +} + +diesel_derives::__diesel_for_each_tuple!(tuple_impls); + /// An ARRAY[...] literal. #[derive(Debug, Clone, Copy, QueryId)] pub struct ArrayLiteral { @@ -140,6 +172,28 @@ where type IsAggregate = T::IsAggregate; } +impl InExpression for ArrayLiteral +where + Self: Expression>, + ST: SqlType, +{ + type SqlType = ST; + fn is_empty(&self) -> bool { + false + } +} + +impl AsInExpression for ArrayLiteral +where + Self: Expression>, + ST: SqlType, +{ + type InExpression = Self; + fn as_in_expression(self) -> Self::InExpression { + self + } +} + impl IntoArrayExpression for SelectStatement where @@ -236,32 +290,24 @@ where type IsAggregate = as ValidGrouping>::IsAggregate; } -// This has to be implemented for each tuple directly because an intermediate trait would cause -// conflicting impls. -// This is not implemented with other tuple impls because this is feature-flagged by -// `postgres-backend` -macro_rules! tuple_impls { - ($( - $Tuple:tt { - $(($idx:tt) -> $T:ident, $ST:ident, $TT:ident,)+ - } - )+) => { - $( - impl<$($T,)+ ST> IntoArrayExpression for ($($T,)+) where - $($T: AsExpression,)+ - ST: SqlType + TypedExpressionType, - { - type ArrayExpression = ArrayLiteral<($($T::Expression,)+), ST>; - - fn into_array_expression(self) -> Self::ArrayExpression { - ArrayLiteral { - elements: ($(self.$idx.as_expression(),)+), - _marker: PhantomData, - } - } - } - )+ +impl InExpression for ArraySubselect +where + Self: Expression>, + ST: SqlType, +{ + type SqlType = ST; + fn is_empty(&self) -> bool { + false } } -diesel_derives::__diesel_for_each_tuple!(tuple_impls); +impl AsInExpression for ArraySubselect +where + Self: Expression>, + ST: SqlType, +{ + type InExpression = Self; + fn as_in_expression(self) -> Self::InExpression { + self + } +} diff --git a/diesel/src/pg/query_builder/query_fragment_impls.rs b/diesel/src/pg/query_builder/query_fragment_impls.rs index e8bc3cd74db9..224977569fab 100644 --- a/diesel/src/pg/query_builder/query_fragment_impls.rs +++ b/diesel/src/pg/query_builder/query_fragment_impls.rs @@ -1,4 +1,4 @@ -use crate::expression::array_comparison::{In, Many, MaybeEmpty, NotIn}; +use crate::expression::array_comparison::{In, InExpression, Many, NotIn}; use crate::pg::backend::PgStyleArrayComparison; use crate::pg::types::sql_types::Array; use crate::pg::Pg; @@ -63,7 +63,7 @@ impl QueryFragment for NoWait { impl QueryFragment for In where T: QueryFragment, - U: QueryFragment + MaybeEmpty, + U: QueryFragment + InExpression, { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> { self.left.walk_ast(out.reborrow())?; @@ -77,7 +77,7 @@ where impl QueryFragment for NotIn where T: QueryFragment, - U: QueryFragment + MaybeEmpty, + U: QueryFragment + InExpression, { fn walk_ast<'b>(&'b self, mut out: AstPass<'_, 'b, Pg>) -> QueryResult<()> { self.left.walk_ast(out.reborrow())?; diff --git a/diesel_tests/tests/filter_operators.rs b/diesel_tests/tests/filter_operators.rs index f95bd172b2ca..ebbbd169d7cd 100644 --- a/diesel_tests/tests/filter_operators.rs +++ b/diesel_tests/tests/filter_operators.rs @@ -264,6 +264,70 @@ fn filter_by_in() { ); } +#[test] +#[cfg(feature = "postgres")] +fn filter_by_in_explicit_array() { + use crate::schema::users::dsl::*; + + let connection = &mut connection_with_3_users(); + let sean = User::new(1, "Sean"); + let tess = User::new(2, "Tess"); + let jim = User::new(3, "Jim"); + + let users_alias = alias!(crate::schema::users as users_alias); + + let query_subselect = users + .filter(name.eq_any(dsl::array(users_alias.select(users_alias.field(name))))) + .order_by(id); + + let debug_subselect: String = debug_query::(&query_subselect).to_string(); + if !debug_subselect + .contains(r#"= ANY(ARRAY(SELECT "users_alias"."name" FROM "users" AS "users_alias"))"#) + { + panic!( + "Generated query (subselect) does not contain expected SQL: {}", + debug_subselect + ); + } + + assert_eq!( + &[sean.clone(), tess.clone(), jim] as &[_], + query_subselect.load(connection).unwrap() + ); + + define_sql_function! { + fn coalesce(x: sql_types::Nullable, y: sql_types::Text) -> sql_types::Text; + } + let query_array_construct = users + .filter( + name.eq_any(dsl::array::(( + coalesce( + users_alias + .filter(users_alias.field(id).eq(1)) + .select(users_alias.field(name)) + .single_value(), + "Jim", + ), + "Tess", + ))), + ) + .order_by(id); + + let debug_array_construct: String = + debug_query::(&query_array_construct).to_string(); + if !debug_array_construct.contains("= ANY(ARRAY[coalesce((SELECT") { + panic!( + "Generated query (array construct) does not contain expected SQL: {}", + debug_array_construct + ); + } + + assert_eq!( + &[sean, tess] as &[_], + query_array_construct.load(connection).unwrap() + ); +} + fn connection_with_3_users() -> TestConnection { let mut connection = connection_with_sean_and_tess_in_users_table(); diesel::sql_query("INSERT INTO users (id, name) VALUES (3, 'Jim')") From 7781df8e34b744a2608551de7384590c2fa4dd53 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 18:41:52 +0100 Subject: [PATCH 20/23] Add more comment on the Subselect type --- diesel/src/expression/subselect.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/diesel/src/expression/subselect.rs b/diesel/src/expression/subselect.rs index 91cd5a715795..25c98ea0c5a1 100644 --- a/diesel/src/expression/subselect.rs +++ b/diesel/src/expression/subselect.rs @@ -9,7 +9,11 @@ use crate::result::QueryResult; /// 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`] trait. +/// 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` although there is actually no such subquery in +/// the final SQL.) #[derive(Debug, Copy, Clone, QueryId)] pub struct Subselect { values: T, From 7a5c5a8e0a9b3bcd19a4b9c7654fff374bedffcc Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 19:00:51 +0100 Subject: [PATCH 21/23] WIP support .eq_any(array_of_nullable_expressions) --- diesel_tests/tests/filter_operators.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/diesel_tests/tests/filter_operators.rs b/diesel_tests/tests/filter_operators.rs index ebbbd169d7cd..ca0bc9329fdd 100644 --- a/diesel_tests/tests/filter_operators.rs +++ b/diesel_tests/tests/filter_operators.rs @@ -295,27 +295,22 @@ fn filter_by_in_explicit_array() { query_subselect.load(connection).unwrap() ); - define_sql_function! { - fn coalesce(x: sql_types::Nullable, y: sql_types::Text) -> sql_types::Text; - } let query_array_construct = users .filter( - name.eq_any(dsl::array::(( - coalesce( - users_alias - .filter(users_alias.field(id).eq(1)) - .select(users_alias.field(name)) - .single_value(), - "Jim", - ), + name.eq_any(dsl::array::, _>(( + users_alias + .filter(users_alias.field(id).eq(1)) + .select(users_alias.field(name)) + .single_value(), "Tess", + None::<&str>, ))), ) .order_by(id); let debug_array_construct: String = debug_query::(&query_array_construct).to_string(); - if !debug_array_construct.contains("= ANY(ARRAY[coalesce((SELECT") { + if !debug_array_construct.contains("= ANY(ARRAY[(SELECT") { panic!( "Generated query (array construct) does not contain expected SQL: {}", debug_array_construct From 18c44a5d5c9476d362d69df3360f067d1bbf5ea5 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 21:17:47 +0100 Subject: [PATCH 22/23] Showcase that it now works with nullable arrays --- diesel_tests/tests/filter_operators.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diesel_tests/tests/filter_operators.rs b/diesel_tests/tests/filter_operators.rs index ca0bc9329fdd..73f627dc123d 100644 --- a/diesel_tests/tests/filter_operators.rs +++ b/diesel_tests/tests/filter_operators.rs @@ -297,7 +297,7 @@ fn filter_by_in_explicit_array() { let query_array_construct = users .filter( - name.eq_any(dsl::array::, _>(( + name.nullable().eq_any(dsl::array(( users_alias .filter(users_alias.field(id).eq(1)) .select(users_alias.field(name)) From f9d768fbcdc2813b53c9c5df2f580b275b7697b7 Mon Sep 17 00:00:00 2001 From: Thomas BESSOU Date: Wed, 20 Nov 2024 21:25:01 +0100 Subject: [PATCH 23/23] minor doc improvement --- diesel/src/pg/expression/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diesel/src/pg/expression/array.rs b/diesel/src/pg/expression/array.rs index 37b6dca50c83..7e08c2c929bd 100644 --- a/diesel/src/pg/expression/array.rs +++ b/diesel/src/pg/expression/array.rs @@ -80,7 +80,7 @@ pub trait IntoArrayExpression { fn into_array_expression(self) -> Self::ArrayExpression; } -/// Implement as a no-op for things that were already arrays (that is, don't wrap in ARRAY[]). +/// Implement as a no-op for expressions that were already arrays (that is, don't wrap in ARRAY[]). impl IntoArrayExpression for T where T: AsExpression>,