diesel-rs / diesel

A safe, extensible ORM and Query Builder for Rust
https://diesel.rs
Apache License 2.0
12.79k stars 1.08k forks source link

Missing impl for tuples in custom QueryFragment #4292

Open Zstorm999 opened 1 month ago

Zstorm999 commented 1 month ago

When building a custom QueryFragment, it seems some method do not behave the same when using the standard SelectBy trait or a custom implementation.

This happens in particular when trying to query two structs inside a tuple (see example below).

Minimum reproduction ```rust use std::error::Error; use diesel::{ pg::Pg, query_builder::{Query, QueryFragment, QueryId}, query_dsl::methods::{LoadQuery, SelectDsl}, sql_types::BigInt, Connection, PgConnection, QueryResult, Queryable, RunQueryDsl, Selectable, SelectableHelper, }; pub trait Paginate: Sized { fn paginate(self) -> Pagination; } impl Paginate for T { fn paginate(self) -> Pagination { Pagination { query: self } } } #[derive(Debug, Clone, Copy, QueryId)] pub struct Pagination { query: T, } impl QueryFragment for Pagination where T: QueryFragment, { fn walk_ast<'b>( &'b self, mut out: diesel::query_builder::AstPass<'_, 'b, Pg>, ) -> QueryResult<()> { out.push_sql("SELECT *, COUNT(*) OVER () FROM("); self.query.walk_ast(out.reborrow())?; out.push_sql(")"); Ok(()) } } impl Query for Pagination { type SqlType = (T::SqlType, BigInt); } impl diesel::RunQueryDsl for Pagination {} // this is the function not working impl Pagination { pub fn load_with_info<'a, U>(self, conn: &mut PgConnection) -> QueryResult<(Vec, i64)> where Self: LoadQuery<'a, PgConnection, (U, i64)>, { let results = self.load::<(U, i64)>(conn)?; let total = results.get(0).map(|x| x.1).unwrap_or(0); let records = results.into_iter().map(|x| x.0).collect(); Ok((records, total)) } } // define a simple table diesel::table! { my_table (field_a) { field_a -> Int4, field_b -> Int4, } } // define 2 query structs on this table #[derive(Queryable, Selectable)] #[diesel(table_name = my_table)] #[diesel(check_for_backend(diesel::pg::Pg))] struct PartA { field_a: i32, } #[derive(Queryable, Selectable)] #[diesel(table_name = my_table)] #[diesel(check_for_backend(diesel::pg::Pg))] struct PartB { field_b: i32, } fn main() -> Result<(), Box> { let mut conn = PgConnection::establish("").expect("Error connecting to database"); // this works let result: Vec<(_, _)> = my_table::table .select((PartA::as_select(), PartB::as_select())) .load(&mut conn)?; // this does not compile let (result, size): (Vec<(_, _)>, i64) = my_table::table .select((PartA::as_select(), PartB::as_select())) .paginate() .load_with_info(&mut conn)?; Ok(()) } ```

Here is the error produced by the compiler:

cargo error message ``` error[E0277]: the trait bound `(diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy): diesel::util::TupleSize` is not satisfied --> src/main.rs:96:10 | 96 | .load_with_info(&mut conn)?; | ^^^^^^^^^^^^^^ the trait `diesel::util::TupleSize` is not implemented for `(diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy)`, which is required by `Pagination, query_builder::select_clause::SelectClause<(diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy)>>>: LoadQuery<'_, PgConnection, (_, i64)>` | = help: the following other types implement trait `diesel::util::TupleSize`: (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 24 others = note: required for `(PartA, PartB)` to implement `StaticallySizedRow<(diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy), Pg>` = note: required for `((PartA, PartB), i64)` to implement `FromStaticSqlRow<((diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy), BigInt), Pg>` = note: required for `((PartA, PartB), i64)` to implement `Queryable<((diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy), BigInt), Pg>` = note: required for `((PartA, PartB), i64)` to implement `FromSqlRow<((diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy), BigInt), Pg>` = note: required for `((diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy), BigInt)` to implement `load_dsl::private::CompatibleType<((PartA, PartB), i64), Pg>` = note: required for `Pagination, query_builder::select_clause::SelectClause<(diesel::expression::select_by::SelectBy, diesel::expression::select_by::SelectBy)>>>` to implement `LoadQuery<'_, PgConnection, ((PartA, PartB), i64)>` note: required by a bound in `Pagination::::load_with_info` --> src/main.rs:29:15 | 27 | pub fn load_with_info<'a, U>(self, conn: &mut PgConnection) -> QueryResult<(Vec, i64)> | -------------- required by a bound in this associated function 28 | where 29 | Self: LoadQuery<'a, PgConnection, (U, i64)>, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Pagination::::load_with_info` ```

Originally posted by @Zstorm999 in https://github.com/diesel-rs/diesel/discussions/4234#discussioncomment-10557332

In particular, it seems that this impl https://github.com/diesel-rs/diesel/blob/79399de749dcc24283c2f2ddcd46be90042eb91a/diesel/src/type_impls/tuples.rs#L371-L384

and this one https://github.com/diesel-rs/diesel/blob/79399de749dcc24283c2f2ddcd46be90042eb91a/diesel/src/type_impls/tuples.rs#L342-L349

do not behave correctly together

prkbuilds commented 2 weeks ago

Hey @Zstorm999 @weiznich , can I work on this?

weiznich commented 2 weeks ago

@PratikFandade Sure, just let us know if you need some help there. Also expect to handle some rather complex trait relations there.

prkbuilds commented 1 week ago

I found the declaration of the SelectBy in this code, if I'm correct the SelectBy should be compatible using this right? https://github.com/diesel-rs/diesel/blob/master/diesel/src/query_dsl/load_dsl.rs#L208-L218

Do we make a new impl for the tuples of SelectBy?