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

Runtime Error for count() with order_by clause combined #3815

Open
3 tasks done
SamTV12345 opened this issue Oct 1, 2023 · 5 comments
Open
3 tasks done

Runtime Error for count() with order_by clause combined #3815

SamTV12345 opened this issue Oct 1, 2023 · 5 comments

Comments

@SamTV12345
Copy link

SamTV12345 commented Oct 1, 2023

Setup

Versions

  • Rust:: 1.72
  • Diesel:: 2.x.x
  • Database:: Postgres
  • Operating System: Windows/Linux/Mac

Feature Flags

  • diesel:: "chrono", "r2d2"

Problem Description

I am creating a hateoas API. A hateoas API contains besides the data also some metadata containing the amount of elements that are contained with the query (different to the current elements).
To not duplicate code I just clone the normal query and add a .count() to it.
Unfortunately it seems like an order by and a count operator are incompatible and generate incompatible SQL.

What are you trying to accomplish?

I try to create an API for the Etherpad website that allows different sorting by downloads/name etc.

What is the expected output?

The total count query should output the total number of items in this query so that I can lazy load other elements if needed

What is the actual output?

thread 'actix-rt|system:0|arbiter:0' panicked at 'called Result::unwrap() on an Err value: DatabaseError(Unknown, "column "datas.downloads" must appear in the GROUP BY clause or be used in an aggregate function")', src\entities\plugin.rs:162:88
stack backtrace:

Are you seeing any additional errors?

No. I had a look at the SQL statements and it is definetely faulty.

Steps to reproduce

  1. git clone https://github.com/SamTV12345/etherpad-plugin-data-extractor.git
  2. git checkout bug/failing-query
  3. curl localhost:8001/api/plugins
  4. See the error message

If you remove every order_by clause for the count query it works flawlessly. I am unfortunately unsure why this happens. Group by SQL statements shouldn't have anything to do with order by clauses

Checklist

  • This issue can be reproduced on Rust's stable channel. (Your issue will be
    closed if this is not the case)
  • This issue can be reproduced without requiring a third party crate
@SamTV12345 SamTV12345 added the bug label Oct 1, 2023
@weiznich
Copy link
Member

weiznich commented Oct 1, 2023

Thanks for filling this bug report. Its always a lot of work to find problems in applications, especially if the it requires several steps and a lot unrelated dependencies. Please try to provide a minimized example that does not required additional dependencies outside of diesel and rust-stdlib, otherwise this issue is not really actionable.

@SamTV12345
Copy link
Author

Thanks for filling this bug report. Its always a lot of work to find problems in applications, especially if the it requires several steps and a lot unrelated dependencies. Please try to provide a minimized example that does not required additional dependencies outside of diesel and rust-stdlib, otherwise this issue is not really actionable.

@weiznich I added a minimal project: https://gitlab.com/samtv12345/diesel-issue3815 . The only dependency it includes is dotenv for having a single DATABASE_URL

@weiznich
Copy link
Member

weiznich commented Oct 1, 2023

Thanks for providing the minimal example. That's helpful. I will try to have a look at that in the next days/weeks.

@SamTV12345
Copy link
Author

Thanks for the help <3. Sorry for misreading the last checkbox. I thought a third party crate would be related to diesel, not in general.

@weiznich
Copy link
Member

weiznich commented Oct 5, 2023

I can confirm this issue. This also affects queries like users::table.order_by(persons::age).select(persons::age). These queries are not allowed without additional group by clause by postgresql and should be rejected at compile time by diesel. That likely means that we need to restrict the order clause values in a similar way than we already do for select clauses. I personally would consider that a bug fix even if this breaks code that would compile now (especially as the generated queries are broken).
To workaround this kind of issue on current diesel releases you need to not write queries that mix non-aggregate and aggregate clauses in this way. So either add a group_by clause or remove the order clause.

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