Skip to content

Conversation

@quaff
Copy link
Contributor

@quaff quaff commented Aug 5, 2024

Align to JPQL query, generated sql should contain offset ? rows and pass 0 as parameter.

[Please describe here what your change is about]


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-18461

@quaff quaff marked this pull request as draft August 5, 2024 09:08
@quaff quaff force-pushed the HHH-18461 branch 2 times, most recently from 177c6c0 to 7dd5419 Compare August 5, 2024 09:47
@quaff quaff marked this pull request as ready for review August 6, 2024 00:30
Copy link
Member

@gavinking gavinking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the motivation behind this.

@quaff
Copy link
Contributor Author

quaff commented Aug 29, 2024

https://hibernate.atlassian.net/browse/HHH-18461

Comment from @beikov :

We don't want to treat the 0 value specially, since the presence of the offset clause regardless of the value that is bound for that parameter potentially does have an effect on query planning. So it's up to the user to call or not call setFirstResult() to control whether the offset clause should be rendered or not.

@gavinking
Copy link
Member

Fine, then I'll let Christian make the call on this. To me it seems weird to change the SQL that was handwritten by the user if we don't absolutely need to, but whatever, it's fine.

@quaff
Copy link
Contributor Author

quaff commented Sep 14, 2024

@beikov Please make the call.

@yrodiere yrodiere requested a review from beikov October 3, 2025 17:53
Copy link
Member

@beikov beikov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the change looks good to me. Please rebase and then we can merge it.

Align to JPQL query, generated sql should contain `offset ? rows` and pass `0` as parameter.
@quaff
Copy link
Contributor Author

quaff commented Oct 9, 2025

Yeah, the change looks good to me. Please rebase and then we can merge it.

Rebased.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants