Skip to content

Commit 4ec8da0

Browse files
ShawnQiang1ShawnQiang1
committed
Add from and size parameters to terms lookup query for pagination support (opensearch-project#20923)
1 parent 9c29462 commit 4ec8da0

File tree

4 files changed

+138
-4
lines changed

4 files changed

+138
-4
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
77
### Added
88
- Add bitmap64 query support ([#20606](https://github.com/opensearch-project/OpenSearch/pull/20606))
99
- Add ProfilingWrapper interface for plugin access to delegates in profiling decorators ([#20607](https://github.com/opensearch-project/OpenSearch/pull/20607))
10+
- Add `from` and `size` parameters to terms lookup query to support pagination of lookup terms ([#20923](https://github.com/opensearch-project/OpenSearch/pull/20923))
1011
- Support expected cluster name with validation in CCS Sniff mode ([#20532](https://github.com/opensearch-project/OpenSearch/pull/20532))
1112
- Choose the best performing node when writing with append-only index ([#20065](https://github.com/opensearch-project/OpenSearch/pull/20065))
1213
- Add security policy to allow `accessUnixDomainSocket` in `transport-grpc` module ([#20463](https://github.com/opensearch-project/OpenSearch/pull/20463), [#20649](https://github.com/opensearch-project/OpenSearch/pull/20649))

‎rest-api-spec/src/main/resources/rest-api-spec/test/search/171_terms_lookup_query.yml‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,3 +210,74 @@
210210
query:
211211
match_all: {}
212212
- match: { hits.total: 5 } # Should always be 5
213+
214+
# --- TEST CASES FOR from AND size PARAMETERS --- #
215+
216+
# Test: size parameter limits the number of terms fetched
217+
- do:
218+
search:
219+
rest_total_hits_as_int: true
220+
index: main_index
221+
body:
222+
query:
223+
terms:
224+
user:
225+
index: lookup_index
226+
path: followers
227+
query:
228+
term:
229+
group: "g1"
230+
size: 1
231+
- match: { hits.total: 3 } # size=1 returns 3 docs: u1(foo), u2(bar), u5(foo)
232+
233+
# Test: from parameter skips the first N terms
234+
- do:
235+
search:
236+
rest_total_hits_as_int: true
237+
index: main_index
238+
body:
239+
query:
240+
terms:
241+
user:
242+
index: lookup_index
243+
path: followers
244+
query:
245+
term:
246+
group: "g1"
247+
from: 1
248+
- match: { hits.total: 1 } # from=1 returns 1 doc: u3(baz)
249+
250+
# Test: from and size combined
251+
- do:
252+
search:
253+
rest_total_hits_as_int: true
254+
index: main_index
255+
body:
256+
query:
257+
terms:
258+
user:
259+
index: lookup_index
260+
path: followers
261+
query:
262+
term:
263+
group: "g1"
264+
from: 1
265+
size: 1
266+
- match: { hits.total: 1 } # from=1, size=1 returns 1 doc: u3(baz)
267+
268+
# Test: size=0 should return no terms
269+
- do:
270+
search:
271+
rest_total_hits_as_int: true
272+
index: main_index
273+
body:
274+
query:
275+
terms:
276+
user:
277+
index: lookup_index
278+
path: followers
279+
query:
280+
term:
281+
group: "g1"
282+
size: 0
283+
- match: { hits.total: 0 }

‎server/src/main/java/org/opensearch/index/query/TermsQueryBuilder.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,17 @@ private void fetch(TermsLookup termsLookup, Client client, ActionListener<List<O
616616
int fetchSize = Math.min(Math.min(maxTermsCount, maxResultWindow), maxClauseCount);
617617

618618
try {
619-
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder().query(termsLookup.query()).size(fetchSize);
619+
// Use user-specified size if provided, otherwise use calculated fetchSize
620+
int effectiveSize = termsLookup.size() != null ? termsLookup.size() : fetchSize;
621+
// Validate that effectiveSize doesn't exceed fetchSize
622+
if (effectiveSize > fetchSize) {
623+
throw new IllegalArgumentException(
624+
"Terms lookup size [" + effectiveSize + "] exceeds maximum allowed [" + fetchSize + "]"
625+
);
626+
}
627+
SearchSourceBuilder sourceBuilder = new SearchSourceBuilder().query(termsLookup.query())
628+
.from(termsLookup.from())
629+
.size(effectiveSize);
620630

621631
// Use stored fields if possible, otherwise fetch source
622632
if (termsLookup.store()) {

‎server/src/main/java/org/opensearch/indices/TermsLookup.java‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,18 @@ public class TermsLookup implements Writeable, ToXContentFragment {
6464
private final String path;
6565
private String routing;
6666
private QueryBuilder query;
67+
private int from = 0;
68+
private Integer size;
6769

6870
public TermsLookup(String index, String id, String path) {
6971
this(index, id, path, null);
7072
}
7173

7274
public TermsLookup(String index, String id, String path, QueryBuilder query) {
75+
this(index, id, path, query, 0, null);
76+
}
77+
78+
public TermsLookup(String index, String id, String path, QueryBuilder query, int from, Integer size) {
7379
if (index == null) {
7480
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] index cannot be null or empty for TermsLookup");
7581
}
@@ -90,6 +96,8 @@ public TermsLookup(String index, String id, String path, QueryBuilder query) {
9096
this.id = id;
9197
this.path = path;
9298
this.query = query;
99+
this.from = from;
100+
this.size = size;
93101
}
94102

95103
public String index() {
@@ -117,6 +125,10 @@ public TermsLookup(StreamInput in) throws IOException {
117125
if (in.getVersion().onOrAfter(Version.V_3_2_0)) {
118126
query = in.readOptionalWriteable(inStream -> inStream.readNamedWriteable(QueryBuilder.class));
119127
}
128+
if (in.getVersion().onOrAfter(Version.V_3_0_0)) {
129+
from = in.readInt();
130+
size = in.readOptionalInt();
131+
}
120132
}
121133

122134
@Override
@@ -134,6 +146,10 @@ public void writeTo(StreamOutput out) throws IOException {
134146
if (out.getVersion().onOrAfter(Version.V_3_2_0)) {
135147
out.writeOptionalWriteable(query);
136148
}
149+
if (out.getVersion().onOrAfter(Version.V_3_5_0)) {
150+
out.writeInt(from);
151+
out.writeOptionalInt(size);
152+
}
137153
}
138154

139155
public String path() {
@@ -176,6 +192,30 @@ public void setQuery(QueryBuilder query) {
176192
this.query = query;
177193
}
178194

195+
public int from() {
196+
return from;
197+
}
198+
199+
public TermsLookup from(int from) {
200+
if (from < 0) {
201+
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] from cannot be negative.");
202+
}
203+
this.from = from;
204+
return this;
205+
}
206+
207+
public Integer size() {
208+
return size;
209+
}
210+
211+
public TermsLookup size(Integer size) {
212+
if (size != null && size < 0) {
213+
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] size cannot be negative.");
214+
}
215+
this.size = size;
216+
return this;
217+
}
218+
179219
public TermsLookup id(String id) {
180220
if (this.query != null && id != null) {
181221
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element cannot specify both id and query.");
@@ -189,6 +229,8 @@ public TermsLookup id(String id) {
189229
String id = (String) args[1]; // Optional id or query but not both
190230
String path = (String) args[2];
191231
QueryBuilder query = (QueryBuilder) args[3]; // Optional id or query but not both
232+
Integer from = (Integer) args[4]; // Optional
233+
Integer size = (Integer) args[5]; // Optional
192234

193235
// Validation: Either id or query must be present, but not both
194236
if (id == null && query == null) {
@@ -199,7 +241,7 @@ public TermsLookup id(String id) {
199241
if (id != null && query != null) {
200242
throw new IllegalArgumentException("[" + TermsQueryBuilder.NAME + "] query lookup element cannot specify both id and query.");
201243
}
202-
return new TermsLookup(index, id, path, query);
244+
return new TermsLookup(index, id, path, query, from != null ? from : 0, size);
203245
});
204246
static {
205247
PARSER.declareString(constructorArg(), new ParseField("index")); // Required
@@ -212,6 +254,8 @@ public TermsLookup id(String id) {
212254
throw new RuntimeException("Error parsing inner query builder", e);
213255
}
214256
}, new ParseField("query")); // Optional
257+
PARSER.declareInt(optionalConstructorArg(), new ParseField("from")); // Optional
258+
PARSER.declareInt(optionalConstructorArg(), new ParseField("size")); // Optional
215259
PARSER.declareString(TermsLookup::routing, new ParseField("routing")); // Optional
216260
PARSER.declareBoolean(TermsLookup::store, new ParseField("store")); // Optional
217261
}
@@ -239,6 +283,12 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
239283
builder.field("query");
240284
query.toXContent(builder, params);
241285
}
286+
if (from > 0) {
287+
builder.field("from", from);
288+
}
289+
if (size != null) {
290+
builder.field("size", size);
291+
}
242292
if (store) {
243293
builder.field("store", true);
244294
}
@@ -247,7 +297,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
247297

248298
@Override
249299
public int hashCode() {
250-
return Objects.hash(index, id, path, routing, store, query);
300+
return Objects.hash(index, id, path, routing, store, query, from, size);
251301
}
252302

253303
@Override
@@ -264,6 +314,8 @@ public boolean equals(Object obj) {
264314
&& Objects.equals(path, other.path)
265315
&& Objects.equals(routing, other.routing)
266316
&& Objects.equals(store, other.store)
267-
&& Objects.equals(query, other.query);
317+
&& Objects.equals(query, other.query)
318+
&& from == other.from
319+
&& Objects.equals(size, other.size);
268320
}
269321
}

0 commit comments

Comments
 (0)