Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing impl for tuples in custom QueryFragment #4292

Open
Zstorm999 opened this issue Sep 30, 2024 · 4 comments
Open

Missing impl for tuples in custom QueryFragment #4292

Zstorm999 opened this issue Sep 30, 2024 · 4 comments

Comments

@Zstorm999
Copy link

Zstorm999 commented Sep 30, 2024

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
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<Self>;
}

impl<T> Paginate for T {
    fn paginate(self) -> Pagination<Self> {
        Pagination { query: self }
    }
}

#[derive(Debug, Clone, Copy, QueryId)]
pub struct Pagination<T> {
    query: T,
}

impl<T> QueryFragment<Pg> for Pagination<T>
where
    T: QueryFragment<Pg>,
{
    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<T: Query> Query for Pagination<T> {
    type SqlType = (T::SqlType, BigInt);
}

impl<T> diesel::RunQueryDsl<PgConnection> for Pagination<T> {}

// this is the function not working
impl<T> Pagination<T> {
    pub fn load_with_info<'a, U>(self, conn: &mut PgConnection) -> QueryResult<(Vec<U>, 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<dyn Error>> {
    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<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>): 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<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>)`, which is required by `Pagination<SelectStatement<FromClause<table>, query_builder::select_clause::SelectClause<(diesel::expression::select_by::SelectBy<PartA, _>, diesel::expression::select_by::SelectBy<PartB, _>)>>>: 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<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>), Pg>`
   = note: required for `((PartA, PartB), i64)` to implement `FromStaticSqlRow<((diesel::expression::select_by::SelectBy<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>), BigInt), Pg>`
   = note: required for `((PartA, PartB), i64)` to implement `Queryable<((diesel::expression::select_by::SelectBy<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>), BigInt), Pg>`
   = note: required for `((PartA, PartB), i64)` to implement `FromSqlRow<((diesel::expression::select_by::SelectBy<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>), BigInt), Pg>`
   = note: required for `((diesel::expression::select_by::SelectBy<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>), BigInt)` to implement `load_dsl::private::CompatibleType<((PartA, PartB), i64), Pg>`
   = note: required for `Pagination<SelectStatement<FromClause<table>, query_builder::select_clause::SelectClause<(diesel::expression::select_by::SelectBy<PartA, Pg>, diesel::expression::select_by::SelectBy<PartB, Pg>)>>>` to implement `LoadQuery<'_, PgConnection, ((PartA, PartB), i64)>`
note: required by a bound in `Pagination::<T>::load_with_info`
  --> src/main.rs:29:15
   |
27 |     pub fn load_with_info<'a, U>(self, conn: &mut PgConnection) -> QueryResult<(Vec<U>, i64)>
   |            -------------- required by a bound in this associated function
28 |     where
29 |         Self: LoadQuery<'a, PgConnection, (U, i64)>,
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `Pagination::<T>::load_with_info`

Originally posted by @Zstorm999 in #4234 (comment)

In particular, it seems that this impl

impl<$T1, $ST1, __DB> crate::deserialize::FromStaticSqlRow<($ST1,), __DB> for ($T1,) where
__DB: Backend,
$ST1: CompatibleType<$T1, __DB>,
$T1: FromSqlRow<<$ST1 as CompatibleType<$T1, __DB>>::SqlType, __DB>,
{
#[allow(non_snake_case, unused_variables, unused_mut)]
fn build_from_row<'a>(row: &impl Row<'a, __DB>)
-> deserialize::Result<Self>
{
Ok(($T1::build_from_row(row)?,))
}
}
};

and this one

impl<__T, $($ST,)* __DB> CompatibleType<__T, __DB> for ($($ST,)*)
where
__DB: Backend,
__T: FromSqlRow<($($ST,)*), __DB>,
{
type SqlType = Self;
}

do not behave correctly together

@Zstorm999 Zstorm999 changed the title Missing impl for tuples Missing impl for tuples in custom QueryFragment Sep 30, 2024
@prkbuilds
Copy link
Member

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

@weiznich
Copy link
Member

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

@prkbuilds
Copy link
Member

prkbuilds commented Nov 14, 2024

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?

@weiznich
Copy link
Member

weiznich commented Dec 6, 2024

@prkbuilds Sorry for taking that long to answer. I've not forgot about this, but I had not the capacity yet to investigate why this happens.
It might help to have those tuple impls, but I'm not sure about that. You need to try it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants