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

ORM/Panache: rely on ORM getResultCount #41019

Closed
wants to merge 1 commit into from
Closed

ORM/Panache: rely on ORM getResultCount #41019

wants to merge 1 commit into from

Conversation

FroMage
Copy link
Member

@FroMage FroMage commented Jun 6, 2024

This allows us to not care about which parameters are set on the count query, since order by parameters are ignored by ORM. Which is not the case if we write our own count query.

Note that this is not yet available in HR, so we have to keep the count query logic around until that's available.

Fixes #40962

This requires ORM 6.5.3 for https://hibernate.atlassian.net/browse/HHH-18234
Also requires HR supporting getResultCount hibernate/hibernate-reactive#1932

This allows us to not care about which parameters are set on the count
query, since `order by` parameters are ignored by ORM. Which is not
the case if we write our own count query.

Note that this is not yet available in HR, so we have to keep the count
query logic around until that's available.

Fixes #40962
@gsmet
Copy link
Member

gsmet commented Jul 1, 2024

@FroMage what's the status of this? It has quite a lot of conflicts now but I suppose it could be revisited easily? Also this was pushed to branch of the main repository so I would be in favor of closing it and get a new PR from a fork if possible.

/cc @yrodiere

@FroMage
Copy link
Member Author

FroMage commented Jul 2, 2024

What was pushed?

@yrodiere
Copy link
Member

yrodiere commented Jul 2, 2024

What was pushed?

You created the branch on the main repo, not on your fork: https://github.com/quarkusio/quarkus/tree/40962

@FroMage
Copy link
Member Author

FroMage commented Jul 2, 2024

Oh, oops. OK, lemme fix that, and rebase.

@FroMage FroMage closed this Jul 2, 2024
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Jul 2, 2024
@gastaldi gastaldi deleted the 40962 branch September 3, 2024 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/panache triage/invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specifying parameters in sorting will throw an exception
3 participants