Skip to content

Commit 12ddb74

Browse files
authored
Fix the calculation of the requested number of documents.
Original Pull Request #3128 Closes #3127 Signed-off-by: Peter-Josef Meisch <pj.meisch@sothawo.com>
1 parent 6324b72 commit 12ddb74

File tree

5 files changed

+184
-35
lines changed

5 files changed

+184
-35
lines changed

src/main/java/org/springframework/data/elasticsearch/client/elc/RequestConverter.java

Lines changed: 13 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,6 @@ class RequestConverter extends AbstractQueryProcessor {
120120

121121
private static final Log LOGGER = LogFactory.getLog(RequestConverter.class);
122122

123-
// the default max result window size of Elasticsearch
124-
public static final Integer INDEX_MAX_RESULT_WINDOW = 10_000;
125-
126123
protected final JsonpMapper jsonpMapper;
127124
protected final ElasticsearchConverter elasticsearchConverter;
128125

@@ -751,9 +748,9 @@ private CreateOperation<?> bulkCreateOperation(IndexQuery query, IndexCoordinate
751748
}
752749
return co.elastic.clients.elasticsearch._types.Script.of(sb -> {
753750
sb.lang(scriptData.language())
754-
.params(params)
755-
.id(scriptData.scriptName());
756-
if (scriptData.script() != null){
751+
.params(params)
752+
.id(scriptData.scriptName());
753+
if (scriptData.script() != null) {
757754
sb.source(s -> s.scriptString(scriptData.script()));
758755
}
759756
return sb;
@@ -927,7 +924,7 @@ public co.elastic.clients.elasticsearch.core.ReindexRequest reindex(ReindexReque
927924
ReindexRequest.Script script = reindexRequest.getScript();
928925
if (script != null) {
929926
builder.script(sb -> {
930-
if (script.getSource() != null){
927+
if (script.getSource() != null) {
931928
sb.source(s -> s.scriptString(script.getSource()));
932929
}
933930
sb.lang(script.getLang());
@@ -1089,7 +1086,7 @@ public DeleteByQueryRequest documentDeleteByQueryRequest(DeleteQuery query, @Nul
10891086

10901087
uqb.script(sb -> {
10911088
sb.lang(query.getLang()).params(params);
1092-
if (query.getScript() != null){
1089+
if (query.getScript() != null) {
10931090
sb.source(s -> s.scriptString(query.getScript()));
10941091
}
10951092
sb.id(query.getId());
@@ -1257,8 +1254,8 @@ public MsearchTemplateRequest searchMsearchTemplateRequest(
12571254
.header(msearchHeaderBuilder(query, param.index(), routing))
12581255
.body(bb -> {
12591256
bb.explain(query.getExplain()) //
1260-
.id(query.getId()); //
1261-
if (query.getSource() != null){
1257+
.id(query.getId()); //
1258+
if (query.getSource() != null) {
12621259
bb.source(s -> s.scriptString(query.getSource()));
12631260
}
12641261

@@ -1296,15 +1293,8 @@ public MsearchRequest searchMsearchRequest(
12961293
.timeout(timeStringMs(query.getTimeout())) //
12971294
;
12981295

1299-
var offset = query.getPageable().isPaged() ? query.getPageable().getOffset() : 0;
1300-
var pageSize = query.getPageable().isPaged() ? query.getPageable().getPageSize()
1301-
: INDEX_MAX_RESULT_WINDOW;
1302-
// if we have both a page size and a max results, we take the min, this is necessary for
1303-
// searchForStream to work correctly (#3098) as there the page size defines what is
1304-
// returned in a single request, and the max result determines the total number of
1305-
// documents returned
1306-
var size = query.isLimiting() ? Math.min(pageSize, query.getMaxResults()) : pageSize;
1307-
bb.from((int) offset).size(size);
1296+
bb.from((int) (query.getPageable().isPaged() ? query.getPageable().getOffset() : 0))
1297+
.size(query.getRequestSize());
13081298

13091299
if (!isEmpty(query.getFields())) {
13101300
bb.fields(fb -> {
@@ -1476,14 +1466,8 @@ private <T> void prepareSearchRequest(Query query, @Nullable String routing, @Nu
14761466
builder.seqNoPrimaryTerm(true);
14771467
}
14781468

1479-
var offset = query.getPageable().isPaged() ? query.getPageable().getOffset() : 0;
1480-
var pageSize = query.getPageable().isPaged() ? query.getPageable().getPageSize() : INDEX_MAX_RESULT_WINDOW;
1481-
// if we have both a page size and a max results, we take the min, this is necessary for
1482-
// searchForStream to work correctly (#3098) as there the page size defines what is
1483-
// returned in a single request, and the max result determines the total number of
1484-
// documents returned
1485-
var size = query.isLimiting() ? Math.min(pageSize, query.getMaxResults()) : pageSize;
1486-
builder.from((int) offset).size(size);
1469+
builder.from((int) (query.getPageable().isPaged() ? query.getPageable().getOffset() : 0))
1470+
.size(query.getRequestSize());
14871471

14881472
if (!isEmpty(query.getFields())) {
14891473
var fieldAndFormats = query.getFields().stream().map(field -> FieldAndFormat.of(b -> b.field(field))).toList();
@@ -1943,8 +1927,8 @@ public PutScriptRequest scriptPut(Script script) {
19431927
return PutScriptRequest.of(b -> b //
19441928
.id(script.id()) //
19451929
.script(sb -> sb //
1946-
.lang(script.language()) //
1947-
.source(s -> s.scriptString(script.source()))));
1930+
.lang(script.language()) //
1931+
.source(s -> s.scriptString(script.source()))));
19481932
}
19491933

19501934
public GetScriptRequest scriptGet(String name) {

src/main/java/org/springframework/data/elasticsearch/core/query/BaseQuery.java

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.util.stream.Collectors;
2929

3030
import org.jspecify.annotations.Nullable;
31+
import org.springframework.data.domain.PageRequest;
3132
import org.springframework.data.domain.Pageable;
3233
import org.springframework.data.domain.Sort;
3334
import org.springframework.util.Assert;
@@ -47,10 +48,15 @@
4748
*/
4849
public class BaseQuery implements Query {
4950

51+
public static final int INDEX_MAX_RESULT_WINDOW = 10_000;
52+
5053
private static final int DEFAULT_REACTIVE_BATCH_SIZE = 500;
54+
// the instance to mark the query pageable initial status, needed to distinguish between the initial
55+
// value and a user-set unpaged value; values don't matter, the RequestConverter compares to the isntance.
56+
private static final Pageable UNSET_PAGE = PageRequest.of(0, 1);
5157

5258
@Nullable protected Sort sort;
53-
protected Pageable pageable = DEFAULT_PAGE;
59+
protected Pageable pageable = UNSET_PAGE;
5460
protected List<String> fields = new ArrayList<>();
5561
@Nullable protected List<String> storedFields;
5662
@Nullable protected SourceFilter sourceFilter;
@@ -78,7 +84,7 @@ public class BaseQuery implements Query {
7884
private boolean queryIsUpdatedByConverter = false;
7985
@Nullable private Integer reactiveBatchSize = null;
8086
@Nullable private Boolean allowNoIndices = null;
81-
private EnumSet<IndicesOptions.WildcardStates> expandWildcards;
87+
private EnumSet<IndicesOptions.WildcardStates> expandWildcards = EnumSet.noneOf(IndicesOptions.WildcardStates.class);
8288
private List<DocValueField> docValueFields = new ArrayList<>();
8389
private List<ScriptedField> scriptedFields = new ArrayList<>();
8490

@@ -87,7 +93,7 @@ public BaseQuery() {}
8793
public <Q extends BaseQuery, B extends BaseQueryBuilder<Q, B>> BaseQuery(BaseQueryBuilder<Q, B> builder) {
8894
this.sort = builder.getSort();
8995
// do a setPageable after setting the sort, because the pageable may contain an additional sort
90-
this.setPageable(builder.getPageable() != null ? builder.getPageable() : DEFAULT_PAGE);
96+
this.setPageable(builder.getPageable() != null ? builder.getPageable() : UNSET_PAGE);
9197
this.fields = builder.getFields();
9298
this.storedFields = builder.getStoredFields();
9399
this.sourceFilter = builder.getSourceFilter();
@@ -203,7 +209,7 @@ public SourceFilter getSourceFilter() {
203209
@Override
204210
@SuppressWarnings("unchecked")
205211
public final <T extends Query> T addSort(@Nullable Sort sort) {
206-
if (sort == null) {
212+
if (sort == null || sort.isUnsorted()) {
207213
return (T) this;
208214
}
209215

@@ -561,4 +567,52 @@ public void addScriptedField(ScriptedField scriptedField) {
561567
public List<ScriptedField> getScriptedFields() {
562568
return scriptedFields;
563569
}
570+
571+
@Override
572+
public Integer getRequestSize() {
573+
574+
var pageable = getPageable();
575+
Integer requestSize = null;
576+
577+
if (pageable.isPaged() && pageable != UNSET_PAGE) {
578+
// pagesize defined by the user
579+
if (!isLimiting()) {
580+
// no maxResults
581+
requestSize = pageable.getPageSize();
582+
} else {
583+
// if we have both a page size and a max results, we take the min, this is necessary for
584+
// searchForStream to work correctly (#3098) as there the page size defines what is
585+
// returned in a single request, and the max result determines the total number of
586+
// documents returned.
587+
requestSize = Math.min(pageable.getPageSize(), getMaxResults());
588+
}
589+
} else if (pageable == UNSET_PAGE) {
590+
// no user defined pageable
591+
if (isLimiting()) {
592+
// maxResults
593+
requestSize = getMaxResults();
594+
} else {
595+
requestSize = DEFAULT_PAGE_SIZE;
596+
}
597+
} else {
598+
// explicitly set unpaged
599+
if (!isLimiting()) {
600+
// no maxResults
601+
requestSize = INDEX_MAX_RESULT_WINDOW;
602+
} else {
603+
// if we have both a implicit page size and a max results, we take the min, this is necessary for
604+
// searchForStream to work correctly (#3098) as there the page size defines what is
605+
// returned in a single request, and the max result determines the total number of
606+
// documents returned.
607+
requestSize = Math.min(INDEX_MAX_RESULT_WINDOW, getMaxResults());
608+
}
609+
}
610+
611+
if (requestSize == null) {
612+
// this should not happen
613+
requestSize = DEFAULT_PAGE_SIZE;
614+
}
615+
616+
return requestSize;
617+
}
564618
}

src/main/java/org/springframework/data/elasticsearch/core/query/Query.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,13 @@ default Integer getReactiveBatchSize() {
484484
*/
485485
List<ScriptedField> getScriptedFields();
486486

487+
/**
488+
* @return the number of documents that should be requested from Elasticsearch in this query. Depends wether a
489+
* Pageable and/or maxResult size is set on the query.
490+
* @since 5.4.8 5.5.2
491+
*/
492+
public Integer getRequestSize();
493+
487494
/**
488495
* @since 4.3
489496
*/

src/test/java/org/springframework/data/elasticsearch/core/ElasticsearchIntegrationTests.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,6 @@
104104
@SpringIntegrationTest
105105
public abstract class ElasticsearchIntegrationTests {
106106

107-
static final Integer INDEX_MAX_RESULT_WINDOW = 10_000;
108-
109107
private static final String MULTI_INDEX_PREFIX = "test-index";
110108
private static final String MULTI_INDEX_ALL = MULTI_INDEX_PREFIX + "*";
111109
private static final String MULTI_INDEX_1_NAME = MULTI_INDEX_PREFIX + "-1";
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.elasticsearch.core.query;
17+
18+
import static org.assertj.core.api.Assertions.*;
19+
import static org.springframework.data.elasticsearch.core.query.BaseQuery.*;
20+
21+
import org.junit.jupiter.api.DisplayName;
22+
import org.junit.jupiter.api.Test;
23+
import org.springframework.data.domain.Pageable;
24+
25+
class BaseQueryTests {
26+
27+
private static final String MATCH_ALL_QUERY = "{\"match_all\":{}}";
28+
29+
@Test // #3127
30+
@DisplayName("query with no Pageable and no maxResults requests 10 docs from 0")
31+
void queryWithNoPageableAndNoMaxResultsRequests10DocsFrom0() {
32+
33+
var query = StringQuery.builder(MATCH_ALL_QUERY)
34+
.build();
35+
36+
var requestSize = query.getRequestSize();
37+
38+
assertThat(requestSize).isEqualTo(10);
39+
}
40+
41+
@Test // #3127
42+
@DisplayName("query with a Pageable and no MaxResults request with values from Pageable")
43+
void queryWithAPageableAndNoMaxResultsRequestWithValuesFromPageable() {
44+
var query = StringQuery.builder(MATCH_ALL_QUERY)
45+
.withPageable(Pageable.ofSize(42))
46+
.build();
47+
48+
var requestSize = query.getRequestSize();
49+
50+
assertThat(requestSize).isEqualTo(42);
51+
}
52+
53+
@Test // #3127
54+
@DisplayName("query with no Pageable and maxResults requests maxResults")
55+
void queryWithNoPageableAndMaxResultsRequestsMaxResults() {
56+
57+
var query = StringQuery.builder(MATCH_ALL_QUERY)
58+
.withMaxResults(12_345)
59+
.build();
60+
61+
var requestSize = query.getRequestSize();
62+
63+
assertThat(requestSize).isEqualTo(12_345);
64+
}
65+
66+
@Test // #3127
67+
@DisplayName("query with Pageable and maxResults requests with values from Pageable if Pageable is less than maxResults")
68+
void queryWithPageableAndMaxResultsRequestsWithValuesFromPageableIfPageableIsLessThanMaxResults() {
69+
70+
var query = StringQuery.builder(MATCH_ALL_QUERY)
71+
.withPageable(Pageable.ofSize(42))
72+
.withMaxResults(123)
73+
.build();
74+
75+
var requestSize = query.getRequestSize();
76+
77+
assertThat(requestSize).isEqualTo(42);
78+
}
79+
80+
@Test // #3127
81+
@DisplayName("query with Pageable and maxResults requests with values from maxResults if Pageable is more than maxResults")
82+
void queryWithPageableAndMaxResultsRequestsWithValuesFromMaxResultsIfPageableIsMoreThanMaxResults() {
83+
84+
var query = StringQuery.builder(MATCH_ALL_QUERY)
85+
.withPageable(Pageable.ofSize(420))
86+
.withMaxResults(123)
87+
.build();
88+
89+
var requestSize = query.getRequestSize();
90+
91+
assertThat(requestSize).isEqualTo(123);
92+
}
93+
94+
@Test // #3127
95+
@DisplayName("query with explicit unpaged request and no maxResults requests max request window size")
96+
void queryWithExplicitUnpagedRequestAndNoMaxResultsRequestsMaxRequestWindowSize() {
97+
98+
var query = StringQuery.builder(MATCH_ALL_QUERY)
99+
.withPageable(Pageable.unpaged())
100+
.build();
101+
102+
var requestSize = query.getRequestSize();
103+
104+
assertThat(requestSize).isEqualTo(INDEX_MAX_RESULT_WINDOW);
105+
}
106+
}

0 commit comments

Comments
 (0)