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

Aggregate + non-aggregate expressions not allowed where they should be #3574

Open
3 tasks done
SteelAlloy opened this issue Mar 29, 2023 · 2 comments
Open
3 tasks done

Comments

@SteelAlloy
Copy link

Setup

Versions

  • Rust: 1.66.0
  • Diesel: 2.0.3
  • Database: PostgreSQL
  • Operating System: Linux

Feature Flags

  • diesel: ["uuid", "postgres", "r2d2", "numeric", "chrono", "serde_json"]

Problem Description

The following query gives some errors, mixing aggregate and non-aggregate expressions

diesel::table! {
    foo (uuid, date) {
        uuid -> Uuid,
        date -> Date,
    }
}

foo::table
        .select((foo::uuid, diesel::dsl::max(foo::date)))
        .group_by(foo::uuid)

What are you trying to accomplish?

I'm trying to translate this valid SQL query to diesel dsl

CREATE TABLE foo (
    uuid uuid PRIMARY KEY NOT NULL,
    date timestamp DEFAULT now() NOT NULL
);

SELECT uuid, max(date) date FROM foo GROUP BY uuid;

What is the expected output?

Being able to mix such expressions when it's valid sql

What is the actual output?

error[E0277]: the trait bound `diesel::expression::is_aggregate::No: MixedAggregates<diesel::expression::is_aggregate::Yes>` is not satisfied
   --> src/graphql/entities/user.rs:392:10
    |
392 |         .select((foo::uuid_1, diesel::dsl::max(foo::date)))
    |          ^^^^^^ the trait `MixedAggregates<diesel::expression::is_aggregate::Yes>` is not implemented for `diesel::expression::is_aggregate::No`
    |
    = help: the following other types implement trait `MixedAggregates<Other>`:
              <diesel::expression::is_aggregate::No as MixedAggregates<diesel::expression::is_aggregate::Never>>
              <diesel::expression::is_aggregate::No as MixedAggregates<diesel::expression::is_aggregate::No>>
    = note: required for `(db::schema::foo::columns::uuid_1, diesel::expression::functions::aggregate_ordering::max::max<diesel::sql_types::Date, db::schema::foo::columns::date>)` to implement `ValidGrouping<()>`
    = note: required for `SelectStatement<FromClause<db::schema::foo::table>>` to implement `SelectDsl<(db::schema::foo::columns::uuid_1, diesel::expression::functions::aggregate_ordering::max::max<diesel::sql_types::Date, db::schema::foo::columns::date>)>`

Are you seeing any additional errors?

No, many thanks for your hard work

Steps to reproduce

Checklist

If you are unsure if your issue is a duplicate of an existing issue please link the issue in question here

@SteelAlloy SteelAlloy added the bug label Mar 29, 2023
@weiznich
Copy link
Member

This is "expected" as we assume that the call to the group_by function appears before the select call. This assumption is made to have somewhat clearer error messages here (so that they point at least to the .select() call instead of only pointing at the query). So for reference the following query compiles:

foo::table
        .group_by(foo::uuid)
        .select((foo::uuid, diesel::dsl::max(foo::date)))

That written: I just checked the documentation and it does not mention this restriction at all. We should probably have that in the documentation of .select and .group_by. So that's at least a missing/bad documentation bug.

@SteelAlloy
Copy link
Author

Thanks, it does indeed work better in this order
I can add this to the documentation if I have time

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

2 participants