Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history
Skip Query.setFirstResult(…) if the pagable offset is not zero.

See #3242
Original pull request: #3454
  • Loading branch information
mp911de committed Jun 27, 2024
1 parent d14899f commit 018c833
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import jakarta.persistence.Query;

import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.query.QueryParameterSetter.ErrorHandling;
import org.springframework.data.jpa.support.PageableUtils;
import org.springframework.util.Assert;
Expand Down Expand Up @@ -98,16 +99,19 @@ Query bindAndPrepare(Query query, QueryParameterSetter.QueryMetadata metadata,

bind(query, metadata, accessor);

if (!useJpaForPaging || !parameters.hasLimitingParameters() || accessor.getPageable().isUnpaged()) {
Pageable pageable = accessor.getPageable();

if (!useJpaForPaging || !parameters.hasLimitingParameters() || pageable.isUnpaged()) {
return query;
}

// see #3242
if (!parameters.hasLimitParameter()) {
// offset is meaningless if Limit parameter present
query.setFirstResult(PageableUtils.getOffsetAsInteger(accessor.getPageable()));
// Apply offset only if it is not 0 (the default).
int offset = PageableUtils.getOffsetAsInteger(pageable);
if (offset != 0) {

This comment has been minimized.

Copy link
@quaff

quaff Jul 1, 2024

Contributor

In order to reuse statement cache, the Hibernate team recommend using offset ? even if the offset is 0, maybe we should keep that for pageable query even it's first page.

query.setFirstResult(offset);
}
query.setMaxResults(accessor.getPageable().getPageSize());

query.setMaxResults(pageable.getPageSize());

return query;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,37 +126,41 @@ void bindWorksWithNullForPageable() throws Exception {
verify(query).setParameter(eq(1), eq("foo"));
}

@Test
@Test // GH-3242
void bindAndPrepareWorksWithPageable() throws Exception {

Method validWithPageable = SampleRepository.class.getMethod("validWithPageable", String.class, Pageable.class);

Object[] values = { "foo", Pageable.ofSize(10).withPage(3) };

bindAndPrepare(validWithPageable, values);
verify(query).setParameter(eq(1), eq("foo"));
verify(query).setFirstResult(eq(30));
verify(query).setMaxResults(eq(10));

verify(query).setParameter(1, "foo");
verify(query).setFirstResult(30);
verify(query).setMaxResults(10);
}

@Test
@Test // GH-3242
void bindWorksWithNullForLimit() throws Exception {

Method validWithLimit = SampleRepository.class.getMethod("validWithLimit", String.class, Limit.class);

Object[] values = { "foo", null };

bind(validWithLimit, values);
verify(query).setParameter(eq(1), eq("foo"));

verify(query).setParameter(1, "foo");
verify(query, never()).setFirstResult(anyInt());
}

@Test
@Test // GH-3242
void bindAndPrepareWorksWithLimit() throws Exception {

Method validWithLimit = SampleRepository.class.getMethod("validWithLimit", String.class, Limit.class);

Object[] values = { "foo", Limit.of(10) };

bindAndPrepare(validWithLimit, values);
verify(query).setParameter(eq(1), eq("foo"));
verify(query).setMaxResults(eq(10));

verify(query).setParameter(1, "foo");
verify(query).setMaxResults(10);
verify(query, never()).setFirstResult(anyInt());
}

Expand Down

0 comments on commit 018c833

Please sign in to comment.