Skip to content

Commit 53c9ddf

Browse files
msfrohmsfroh
authored
Remove ApproximateIndexOrDocValuesQuery (#16273)
Looking into the query approximation framework, I realized that we don't actually need ApproximateIndexOrDocValuesQuery. We already have ApproximateScoreQuery that is able to choose between the approximate query and the original query. Assuming the range query is over an indexed field, it is (potentially) possible to approximate. If there are doc values, then the original query will be an IndexOrDocValuesQuery. Otherwise, it will just be a PointRangeQuery. Either way, we can wrap the original query into a generic ApproximateScoreQuery along with the approximate query. The approximation logic doesn't need to be specialized to the IndexOrDocValue case. --------- Signed-off-by: Michael Froh <froh@amazon.com>
1 parent b3459fd commit 53c9ddf

File tree

13 files changed

+119
-272
lines changed

13 files changed

+119
-272
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5454
- Remove identity-related feature flagged code from the RestController ([#15430](https://github.com/opensearch-project/OpenSearch/pull/15430))
5555
- Remove Identity FeatureFlag ([#16024](https://github.com/opensearch-project/OpenSearch/pull/16024))
5656
- Ensure RestHandler.Wrapper delegates all implementations to the wrapped handler ([#16154](https://github.com/opensearch-project/OpenSearch/pull/16154))
57+
- Code cleanup: Remove ApproximateIndexOrDocValuesQuery ([#16273](https://github.com/opensearch-project/OpenSearch/pull/16273))
5758

5859

5960
### Deprecated

‎server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@
6262
import org.opensearch.index.query.QueryRewriteContext;
6363
import org.opensearch.index.query.QueryShardContext;
6464
import org.opensearch.search.DocValueFormat;
65-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
6665
import org.opensearch.search.approximate.ApproximatePointRangeQuery;
66+
import org.opensearch.search.approximate.ApproximateScoreQuery;
6767
import org.opensearch.search.lookup.SearchLookup;
6868

6969
import java.io.IOException;
@@ -113,21 +113,6 @@ public static DateFormatter getDefaultDateTimeFormatter() {
113113
: LEGACY_DEFAULT_DATE_TIME_FORMATTER;
114114
}
115115

116-
public static Query getDefaultQuery(Query pointRangeQuery, Query dvQuery, String name, long l, long u) {
117-
return FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING)
118-
? new ApproximateIndexOrDocValuesQuery(
119-
pointRangeQuery,
120-
new ApproximatePointRangeQuery(name, pack(new long[] { l }).bytes, pack(new long[] { u }).bytes, new long[] { l }.length) {
121-
@Override
122-
protected String toString(int dimension, byte[] value) {
123-
return Long.toString(LongPoint.decodeDimension(value, 0));
124-
}
125-
},
126-
dvQuery
127-
)
128-
: new IndexOrDocValuesQuery(pointRangeQuery, dvQuery);
129-
}
130-
131116
/**
132117
* Resolution of the date time
133118
*
@@ -488,22 +473,42 @@ public Query rangeQuery(
488473
}
489474
DateMathParser parser = forcedDateParser == null ? dateMathParser : forcedDateParser;
490475
return dateRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, parser, context, resolution, (l, u) -> {
491-
Query pointRangeQuery = isSearchable() ? LongPoint.newRangeQuery(name(), l, u) : null;
492476
Query dvQuery = hasDocValues() ? SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u) : null;
493-
if (isSearchable() && hasDocValues()) {
494-
Query query = getDefaultQuery(pointRangeQuery, dvQuery, name(), l, u);
495-
if (context.indexSortedOnField(name())) {
496-
query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query);
477+
if (isSearchable()) {
478+
Query pointRangeQuery = LongPoint.newRangeQuery(name(), l, u);
479+
Query query;
480+
if (dvQuery != null) {
481+
query = new IndexOrDocValuesQuery(pointRangeQuery, dvQuery);
482+
if (context.indexSortedOnField(name())) {
483+
query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query);
484+
}
485+
} else {
486+
query = pointRangeQuery;
487+
}
488+
if (FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING)) {
489+
return new ApproximateScoreQuery(
490+
query,
491+
new ApproximatePointRangeQuery(
492+
name(),
493+
pack(new long[] { l }).bytes,
494+
pack(new long[] { u }).bytes,
495+
new long[] { l }.length
496+
) {
497+
@Override
498+
protected String toString(int dimension, byte[] value) {
499+
return Long.toString(LongPoint.decodeDimension(value, 0));
500+
}
501+
}
502+
);
497503
}
498504
return query;
499505
}
500-
if (hasDocValues()) {
501-
if (context.indexSortedOnField(name())) {
502-
dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, dvQuery);
503-
}
504-
return dvQuery;
506+
507+
// Not searchable. Must have doc values.
508+
if (context.indexSortedOnField(name())) {
509+
dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, dvQuery);
505510
}
506-
return pointRangeQuery;
511+
return dvQuery;
507512
});
508513
}
509514

‎server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Helper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.opensearch.common.lucene.search.function.FunctionScoreQuery;
2424
import org.opensearch.index.mapper.DateFieldMapper;
2525
import org.opensearch.index.query.DateRangeIncludingNowQuery;
26-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
26+
import org.opensearch.search.approximate.ApproximateScoreQuery;
2727
import org.opensearch.search.internal.SearchContext;
2828

2929
import java.io.IOException;
@@ -55,7 +55,7 @@ private Helper() {}
5555
queryWrappers.put(FunctionScoreQuery.class, q -> ((FunctionScoreQuery) q).getSubQuery());
5656
queryWrappers.put(DateRangeIncludingNowQuery.class, q -> ((DateRangeIncludingNowQuery) q).getQuery());
5757
queryWrappers.put(IndexOrDocValuesQuery.class, q -> ((IndexOrDocValuesQuery) q).getIndexQuery());
58-
queryWrappers.put(ApproximateIndexOrDocValuesQuery.class, q -> ((ApproximateIndexOrDocValuesQuery) q).getOriginalQuery());
58+
queryWrappers.put(ApproximateScoreQuery.class, q -> ((ApproximateScoreQuery) q).getOriginalQuery());
5959
}
6060

6161
/**

‎server/src/main/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQuery.java

Lines changed: 0 additions & 62 deletions
This file was deleted.

‎server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,6 @@ public boolean canApproximate(SearchContext context) {
433433
if (context.aggregations() != null) {
434434
return false;
435435
}
436-
if (!(context.query() instanceof ApproximateIndexOrDocValuesQuery)) {
437-
return false;
438-
}
439436
// size 0 could be set for caching
440437
if (context.from() + context.size() == 0) {
441438
this.setSize(10_000);

‎server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@
2121
* Entry-point for the approximation framework.
2222
* This class is heavily inspired by {@link org.apache.lucene.search.IndexOrDocValuesQuery}. It acts as a wrapper that consumer two queries, a regular query and an approximate version of the same. By default, it executes the regular query and returns {@link Weight#scorer} for the original query. At run-time, depending on certain constraints, we can re-write the {@code Weight} to use the approximate weight instead.
2323
*/
24-
public class ApproximateScoreQuery extends Query {
24+
public final class ApproximateScoreQuery extends Query {
2525

2626
private final Query originalQuery;
2727
private final ApproximateQuery approximationQuery;
2828

29-
protected Query resolvedQuery;
29+
Query resolvedQuery;
3030

3131
public ApproximateScoreQuery(Query originalQuery, ApproximateQuery approximationQuery) {
3232
this.originalQuery = originalQuery;

‎server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
import org.apache.lucene.index.MultiReader;
4242
import org.apache.lucene.index.SortedNumericDocValues;
4343
import org.apache.lucene.search.DocIdSetIterator;
44+
import org.apache.lucene.search.IndexOrDocValuesQuery;
4445
import org.apache.lucene.search.IndexSearcher;
4546
import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery;
4647
import org.apache.lucene.search.Query;
@@ -65,8 +66,8 @@
6566
import org.opensearch.index.query.DateRangeIncludingNowQuery;
6667
import org.opensearch.index.query.QueryRewriteContext;
6768
import org.opensearch.index.query.QueryShardContext;
68-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
6969
import org.opensearch.search.approximate.ApproximatePointRangeQuery;
70+
import org.opensearch.search.approximate.ApproximateScoreQuery;
7071
import org.joda.time.DateTimeZone;
7172

7273
import java.io.IOException;
@@ -212,8 +213,11 @@ public void testTermQuery() {
212213
MappedFieldType ft = new DateFieldType("field");
213214
String date = "2015-10-12T14:10:55";
214215
long instant = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date)).toInstant().toEpochMilli();
215-
Query expected = new ApproximateIndexOrDocValuesQuery(
216-
LongPoint.newRangeQuery("field", instant, instant + 999),
216+
Query expected = new ApproximateScoreQuery(
217+
new IndexOrDocValuesQuery(
218+
LongPoint.newRangeQuery("field", instant, instant + 999),
219+
SortedNumericDocValuesField.newSlowRangeQuery("field", instant, instant + 999)
220+
),
217221
new ApproximatePointRangeQuery(
218222
"field",
219223
pack(new long[] { instant }).bytes,
@@ -224,8 +228,7 @@ public void testTermQuery() {
224228
protected String toString(int dimension, byte[] value) {
225229
return Long.toString(LongPoint.decodeDimension(value, 0));
226230
}
227-
},
228-
SortedNumericDocValuesField.newSlowRangeQuery("field", instant, instant + 999)
231+
}
229232
);
230233
assumeThat(
231234
"Using Approximate Range Query as default",
@@ -278,8 +281,11 @@ public void testRangeQuery() throws IOException {
278281
String date2 = "2016-04-28T11:33:52";
279282
long instant1 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli();
280283
long instant2 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999;
281-
Query expected = new ApproximateIndexOrDocValuesQuery(
282-
LongPoint.newRangeQuery("field", instant1, instant2),
284+
Query expected = new ApproximateScoreQuery(
285+
new IndexOrDocValuesQuery(
286+
LongPoint.newRangeQuery("field", instant1, instant2),
287+
SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2)
288+
),
283289
new ApproximatePointRangeQuery(
284290
"field",
285291
pack(new long[] { instant1 }).bytes,
@@ -290,8 +296,7 @@ public void testRangeQuery() throws IOException {
290296
protected String toString(int dimension, byte[] value) {
291297
return Long.toString(LongPoint.decodeDimension(value, 0));
292298
}
293-
},
294-
SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2)
299+
}
295300
);
296301
assumeThat(
297302
"Using Approximate Range Query as default",
@@ -306,8 +311,11 @@ protected String toString(int dimension, byte[] value) {
306311
instant1 = nowInMillis;
307312
instant2 = instant1 + 100;
308313
expected = new DateRangeIncludingNowQuery(
309-
new ApproximateIndexOrDocValuesQuery(
310-
LongPoint.newRangeQuery("field", instant1, instant2),
314+
new ApproximateScoreQuery(
315+
new IndexOrDocValuesQuery(
316+
LongPoint.newRangeQuery("field", instant1, instant2),
317+
SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2)
318+
),
311319
new ApproximatePointRangeQuery(
312320
"field",
313321
pack(new long[] { instant1 }).bytes,
@@ -318,8 +326,7 @@ protected String toString(int dimension, byte[] value) {
318326
protected String toString(int dimension, byte[] value) {
319327
return Long.toString(LongPoint.decodeDimension(value, 0));
320328
}
321-
},
322-
SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2)
329+
}
323330
)
324331
);
325332
assumeThat(
@@ -388,8 +395,8 @@ public void testRangeQueryWithIndexSort() {
388395
"field",
389396
instant1,
390397
instant2,
391-
new ApproximateIndexOrDocValuesQuery(
392-
LongPoint.newRangeQuery("field", instant1, instant2),
398+
new ApproximateScoreQuery(
399+
new IndexOrDocValuesQuery(LongPoint.newRangeQuery("field", instant1, instant2), dvQuery),
393400
new ApproximatePointRangeQuery(
394401
"field",
395402
pack(new long[] { instant1 }).bytes,
@@ -400,8 +407,7 @@ public void testRangeQueryWithIndexSort() {
400407
protected String toString(int dimension, byte[] value) {
401408
return Long.toString(LongPoint.decodeDimension(value, 0));
402409
}
403-
},
404-
dvQuery
410+
}
405411
)
406412
);
407413
assumeThat(

‎server/src/test/java/org/opensearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@
5050
import org.opensearch.common.util.FeatureFlags;
5151
import org.opensearch.index.query.QueryShardContext;
5252
import org.opensearch.index.query.QueryStringQueryBuilder;
53-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
5453
import org.opensearch.search.approximate.ApproximatePointRangeQuery;
54+
import org.opensearch.search.approximate.ApproximateScoreQuery;
5555
import org.opensearch.test.AbstractQueryTestCase;
5656

5757
import java.io.IOException;
@@ -191,11 +191,14 @@ public void testDateRangeQuery() throws Exception {
191191
is(true)
192192
);
193193
assertEquals(
194-
new ApproximateIndexOrDocValuesQuery(
195-
LongPoint.newRangeQuery(
196-
DATE_FIELD_NAME,
197-
parser.parse(lowerBoundExact, () -> 0).toEpochMilli(),
198-
parser.parse(upperBoundExact, () -> 0).toEpochMilli()
194+
new ApproximateScoreQuery(
195+
new IndexOrDocValuesQuery(
196+
LongPoint.newRangeQuery(
197+
DATE_FIELD_NAME,
198+
parser.parse(lowerBoundExact, () -> 0).toEpochMilli(),
199+
parser.parse(upperBoundExact, () -> 0).toEpochMilli()
200+
),
201+
controlDv
199202
),
200203
new ApproximatePointRangeQuery(
201204
DATE_FIELD_NAME,
@@ -207,8 +210,7 @@ public void testDateRangeQuery() throws Exception {
207210
protected String toString(int dimension, byte[] value) {
208211
return Long.toString(LongPoint.decodeDimension(value, 0));
209212
}
210-
},
211-
controlDv
213+
}
212214
),
213215
queryOnDateField
214216
);

‎server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757
import org.opensearch.index.mapper.RangeFieldMapper.RangeFieldType;
5858
import org.opensearch.index.query.QueryShardContext;
5959
import org.opensearch.index.query.QueryShardException;
60-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
60+
import org.opensearch.search.approximate.ApproximateScoreQuery;
6161
import org.opensearch.test.IndexSettingsModule;
6262
import org.joda.time.DateTime;
6363
import org.junit.Before;
@@ -293,7 +293,7 @@ public void testDateRangeQueryUsingMappingFormatLegacy() {
293293
);
294294
assertEquals(
295295
"field:[1465975790000 TO 1466062190999]",
296-
((IndexOrDocValuesQuery) ((ApproximateIndexOrDocValuesQuery) queryOnDateField).getOriginalQuery()).getIndexQuery().toString()
296+
((IndexOrDocValuesQuery) ((ApproximateScoreQuery) queryOnDateField).getOriginalQuery()).getIndexQuery().toString()
297297
);
298298
}
299299

‎server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
import org.apache.lucene.search.TermQuery;
4343
import org.opensearch.core.common.ParsingException;
4444
import org.opensearch.index.search.MatchQuery.ZeroTermsQuery;
45-
import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery;
45+
import org.opensearch.search.approximate.ApproximateScoreQuery;
4646
import org.opensearch.test.AbstractQueryTestCase;
4747

4848
import java.io.IOException;
@@ -131,7 +131,7 @@ protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query q
131131
.or(instanceOf(PointRangeQuery.class))
132132
.or(instanceOf(IndexOrDocValuesQuery.class))
133133
.or(instanceOf(MatchNoDocsQuery.class))
134-
.or(instanceOf(ApproximateIndexOrDocValuesQuery.class))
134+
.or(instanceOf(ApproximateScoreQuery.class))
135135
);
136136
}
137137

0 commit comments

Comments
 (0)