Skip to content

Redundant parameter in the Limit query #3242

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

Closed
sergey-morenets opened this issue Nov 24, 2023 · 5 comments
Closed

Redundant parameter in the Limit query #3242

sergey-morenets opened this issue Nov 24, 2023 · 5 comments
Assignees
Labels
type: enhancement A general enhancement

Comments

@sergey-morenets
Copy link

Hi

Spring Data JPA introduced new Limit functionality to provide dynamic limiting. So if earlier we wrote the following query:

	List<Product> findTop3By();
	
	List<Product> findFirst3By();

and now we can use new approach:

List<Product> findBy(Limit limit);

But I noticed that resulting SQL in the first two queries was :

SELECT * from Product p1_0 fetch first ? rows only

And now it's:

SELECT * from Product p1_0 offset ? rows fetch first ? rows only

So why do we need additional parameter offset? I guess it's redundant and not needed here.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 24, 2023
@mp911de
Copy link
Member

mp911de commented Nov 24, 2023

This is because we convert by default Limit into Pageable(0, limit) and set the Query.setFirstResult(…) value implicitly.

Actually, Hibernate could omit that value as not setting the offset and setting offset zero should yield the same result.

@quaff
Copy link
Contributor

quaff commented Nov 28, 2023

I will try to fix it at hibernate side.

@quaff
Copy link
Contributor

quaff commented Nov 28, 2023

I've created hibernate/hibernate-orm#7579 to fix this.

@sergey-morenets
Copy link
Author

I've created hibernate/hibernate-orm#7579 to fix this.

Thank you, @quaff

@mp911de mp911de added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 28, 2023
@mp911de mp911de closed this as not planned Won't fix, can't repro, duplicate, stale Nov 28, 2023
quaff added a commit to quaff/spring-data-jpa that referenced this issue Apr 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Avoid calling `Query.setFirstResult(0)` which maybe generated sql contains unwanted `offset 0`.

Fix spring-projectsGH-3242
@quaff
Copy link
Contributor

quaff commented May 29, 2024

I've created hibernate/hibernate-orm#7579 to fix this.

Thank you, @quaff

FYI, Hibernate team decline that.

@mp911de mp911de added type: task A general task and removed for: external-project For an external project and not something we can fix labels May 29, 2024
@mp911de mp911de assigned mp911de and unassigned gregturn May 29, 2024
@mp911de mp911de reopened this May 29, 2024
@mp911de mp911de added type: enhancement A general enhancement and removed type: task A general task labels Jun 27, 2024
@mp911de mp911de added this to the 3.2.8 (2023.1.8) milestone Jun 27, 2024
mp911de pushed a commit that referenced this issue Jun 27, 2024
Avoid calling `Query.setFirstResult(0)` which maybe generated sql contains unwanted `offset 0` when using query methods accepting `Limit` .

Closes #3242
Original pull request: #3454
mp911de added a commit that referenced this issue Jun 27, 2024
Skip Query.setFirstResult(…) if the pagable offset is not zero.

See #3242
Original pull request: #3454
mp911de pushed a commit that referenced this issue Jun 27, 2024
Avoid calling `Query.setFirstResult(0)` which maybe generated sql contains unwanted `offset 0` when using query methods accepting `Limit` .

Closes #3242
Original pull request: #3454
mp911de added a commit that referenced this issue Jun 27, 2024
Skip Query.setFirstResult(…) if the pagable offset is not zero.

See #3242
Original pull request: #3454
mp911de added a commit that referenced this issue Jun 27, 2024
Skip Query.setFirstResult(…) if the pagable offset is not zero.

See #3242
Original pull request: #3454
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants