Skip to content

Commit 265aa09

Browse files
harshavamsiHarsh Kothari
harshavamsi
authored and
Harsh Kothari
committed
Remove feature flag for ApproximatePointRangeQuery (opensearch-project#17769)
Signed-off-by: Harsha Vamsi Kalluri <harshavamsi096@gmail.com> Signed-off-by: Harsh Kothari <techarsh@amazon.com>
1 parent 34c0227 commit 265aa09

File tree

17 files changed

+242
-335
lines changed

17 files changed

+242
-335
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
6868

6969
### Removed
7070
- Remove deprecated `batch_size` parameter from `_bulk` ([#14283](https://github.com/opensearch-project/OpenSearch/issues/14283))
71+
- Remove `FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING` since range query approximation is no longer experimental ([#17769](https://github.com/opensearch-project/OpenSearch/pull/17769))
7172

7273
### Fixed
7374
- Fix bytes parameter on `_cat/recovery` ([#17598](https://github.com/opensearch-project/OpenSearch/pull/17598))

‎modules/mapper-extras/src/test/java/org/opensearch/index/mapper/ScaledFloatFieldTypeTests.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.opensearch.index.fielddata.IndexNumericFieldData;
5252
import org.opensearch.index.fielddata.LeafNumericFieldData;
5353
import org.opensearch.index.fielddata.SortedNumericDoubleValues;
54+
import org.opensearch.search.approximate.ApproximateScoreQuery;
5455

5556
import java.io.IOException;
5657
import java.util.Arrays;
@@ -167,8 +168,8 @@ public void testRoundsLowerBoundCorrectly() {
167168
}
168169

169170
private String getQueryString(Query query) {
170-
assertTrue(query instanceof IndexOrDocValuesQuery);
171-
return ((IndexOrDocValuesQuery) query).getIndexQuery().toString();
171+
assertTrue(query instanceof ApproximateScoreQuery);
172+
return ((IndexOrDocValuesQuery) ((ApproximateScoreQuery) query).getOriginalQuery()).getIndexQuery().toString();
172173
}
173174

174175
public void testValueForSearch() {

‎server/src/main/java/org/opensearch/common/util/FeatureFlags.java

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,6 @@ public class FeatureFlags {
122122
Property.NodeScope
123123
);
124124

125-
/**
126-
* Gates the functionality of ApproximatePointRangeQuery where we approximate query results.
127-
*/
128-
public static final String APPROXIMATE_POINT_RANGE_QUERY = FEATURE_FLAG_PREFIX + "approximate_point_range_query.enabled";
129-
public static final Setting<Boolean> APPROXIMATE_POINT_RANGE_QUERY_SETTING = Setting.boolSetting(
130-
APPROXIMATE_POINT_RANGE_QUERY,
131-
false,
132-
Property.NodeScope
133-
);
134125
public static final String TERM_VERSION_PRECOMMIT_ENABLE = OS_EXPERIMENTAL_PREFIX + "optimization.termversion.precommit.enabled";
135126
public static final Setting<Boolean> TERM_VERSION_PRECOMMIT_ENABLE_SETTING = Setting.boolSetting(
136127
TERM_VERSION_PRECOMMIT_ENABLE,

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

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -486,23 +486,16 @@ public Query rangeQuery(
486486
} else {
487487
query = pointRangeQuery;
488488
}
489-
if (FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING)) {
490-
return new ApproximateScoreQuery(
491-
query,
492-
new ApproximatePointRangeQuery(
493-
name(),
494-
pack(new long[] { l }).bytes,
495-
pack(new long[] { u }).bytes,
496-
new long[] { l }.length
497-
) {
498-
@Override
499-
protected String toString(int dimension, byte[] value) {
500-
return Long.toString(LongPoint.decodeDimension(value, 0));
501-
}
502-
}
503-
);
504-
}
505-
return query;
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+
ApproximatePointRangeQuery.LONG_FORMAT
497+
)
498+
);
506499
}
507500

508501
// Not searchable. Must have doc values.

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

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@
6969
import org.opensearch.index.fielddata.plain.SortedNumericIndexFieldData;
7070
import org.opensearch.index.query.QueryShardContext;
7171
import org.opensearch.search.DocValueFormat;
72+
import org.opensearch.search.approximate.ApproximatePointRangeQuery;
73+
import org.opensearch.search.approximate.ApproximateScoreQuery;
7274
import org.opensearch.search.lookup.SearchLookup;
7375
import org.opensearch.search.query.BitmapDocValuesQuery;
7476
import org.opensearch.search.query.BitmapIndexQuery;
@@ -1058,23 +1060,34 @@ public Query rangeQuery(
10581060
QueryShardContext context
10591061
) {
10601062
return longRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, (l, u) -> {
1061-
if (isSearchable && hasDocValues) {
1062-
Query query = LongPoint.newRangeQuery(field, l, u);
1063-
Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery(field, l, u);
1064-
query = new IndexOrDocValuesQuery(query, dvQuery);
1065-
if (context.indexSortedOnField(field)) {
1066-
query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
1063+
Query dvQuery = hasDocValues ? SortedNumericDocValuesField.newSlowRangeQuery(field, l, u) : null;
1064+
if (isSearchable) {
1065+
Query pointRangeQuery = LongPoint.newRangeQuery(field, l, u);
1066+
Query query;
1067+
if (dvQuery != null) {
1068+
query = new IndexOrDocValuesQuery(pointRangeQuery, dvQuery);
1069+
if (context.indexSortedOnField(field)) {
1070+
query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
1071+
}
1072+
} else {
1073+
query = pointRangeQuery;
10671074
}
1068-
return query;
1075+
return new ApproximateScoreQuery(
1076+
query,
1077+
new ApproximatePointRangeQuery(
1078+
field,
1079+
LongPoint.pack(new long[] { l }).bytes,
1080+
LongPoint.pack(new long[] { u }).bytes,
1081+
new long[] { l }.length,
1082+
ApproximatePointRangeQuery.LONG_FORMAT
1083+
)
1084+
);
1085+
10691086
}
1070-
if (hasDocValues) {
1071-
Query query = SortedNumericDocValuesField.newSlowRangeQuery(field, l, u);
1072-
if (context.indexSortedOnField(field)) {
1073-
query = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, query);
1074-
}
1075-
return query;
1087+
if (context.indexSortedOnField(field)) {
1088+
dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(field, l, u, dvQuery);
10761089
}
1077-
return LongPoint.newRangeQuery(field, l, u);
1090+
return dvQuery;
10781091

10791092
});
10801093
}

‎server/src/main/java/org/opensearch/index/search/NestedHelper.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import org.apache.lucene.search.TermQuery;
4747
import org.opensearch.index.mapper.MapperService;
4848
import org.opensearch.index.mapper.ObjectMapper;
49+
import org.opensearch.search.approximate.ApproximateScoreQuery;
4950

5051
/** Utility class to filter parent and children clauses when building nested
5152
* queries.
@@ -85,6 +86,8 @@ public boolean mightMatchNestedDocs(Query query) {
8586
return mightMatchNestedDocs(((PointRangeQuery) query).getField());
8687
} else if (query instanceof IndexOrDocValuesQuery) {
8788
return mightMatchNestedDocs(((IndexOrDocValuesQuery) query).getIndexQuery());
89+
} else if (query instanceof ApproximateScoreQuery) {
90+
return mightMatchNestedDocs(((ApproximateScoreQuery) query).getOriginalQuery());
8891
} else if (query instanceof BooleanQuery) {
8992
final BooleanQuery bq = (BooleanQuery) query;
9093
final boolean hasRequiredClauses = bq.clauses().stream().anyMatch(BooleanClause::isRequired);
@@ -155,6 +158,8 @@ public boolean mightMatchNonNestedDocs(Query query, String nestedPath) {
155158
return mightMatchNonNestedDocs(((PointRangeQuery) query).getField(), nestedPath);
156159
} else if (query instanceof IndexOrDocValuesQuery) {
157160
return mightMatchNonNestedDocs(((IndexOrDocValuesQuery) query).getIndexQuery(), nestedPath);
161+
} else if (query instanceof ApproximateScoreQuery) {
162+
return mightMatchNonNestedDocs(((ApproximateScoreQuery) query).getOriginalQuery(), nestedPath);
158163
} else if (query instanceof BooleanQuery) {
159164
final BooleanQuery bq = (BooleanQuery) query;
160165
final boolean hasRequiredClauses = bq.clauses().stream().anyMatch(BooleanClause::isRequired);

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

Lines changed: 48 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.opensearch.search.approximate;
1010

11+
import org.apache.lucene.document.LongPoint;
1112
import org.apache.lucene.index.LeafReader;
1213
import org.apache.lucene.index.LeafReaderContext;
1314
import org.apache.lucene.index.PointValues;
@@ -16,6 +17,7 @@
1617
import org.apache.lucene.search.DocIdSetIterator;
1718
import org.apache.lucene.search.IndexSearcher;
1819
import org.apache.lucene.search.PointRangeQuery;
20+
import org.apache.lucene.search.Query;
1921
import org.apache.lucene.search.QueryVisitor;
2022
import org.apache.lucene.search.ScoreMode;
2123
import org.apache.lucene.search.Scorer;
@@ -24,41 +26,51 @@
2426
import org.apache.lucene.util.ArrayUtil;
2527
import org.apache.lucene.util.DocIdSetBuilder;
2628
import org.apache.lucene.util.IntsRef;
27-
import org.opensearch.index.query.RangeQueryBuilder;
2829
import org.opensearch.search.internal.SearchContext;
2930
import org.opensearch.search.sort.FieldSortBuilder;
3031
import org.opensearch.search.sort.SortOrder;
3132

3233
import java.io.IOException;
33-
import java.util.Arrays;
3434
import java.util.Objects;
35+
import java.util.function.Function;
3536

3637
/**
3738
* An approximate-able version of {@link PointRangeQuery}. It creates an instance of {@link PointRangeQuery} but short-circuits the intersect logic
3839
* after {@code size} is hit
3940
*/
40-
public abstract class ApproximatePointRangeQuery extends ApproximateQuery {
41+
public class ApproximatePointRangeQuery extends ApproximateQuery {
42+
public static final Function<byte[], String> LONG_FORMAT = bytes -> Long.toString(LongPoint.decodeDimension(bytes, 0));
4143
private int size;
4244

4345
private SortOrder sortOrder;
4446

4547
public final PointRangeQuery pointRangeQuery;
4648

47-
protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims) {
48-
this(field, lowerPoint, upperPoint, numDims, 10_000, null);
49+
public ApproximatePointRangeQuery(
50+
String field,
51+
byte[] lowerPoint,
52+
byte[] upperPoint,
53+
int numDims,
54+
Function<byte[], String> valueToString
55+
) {
56+
this(field, lowerPoint, upperPoint, numDims, SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO, null, valueToString);
4957
}
5058

51-
protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims, int size) {
52-
this(field, lowerPoint, upperPoint, numDims, size, null);
53-
}
54-
55-
protected ApproximatePointRangeQuery(String field, byte[] lowerPoint, byte[] upperPoint, int numDims, int size, SortOrder sortOrder) {
59+
protected ApproximatePointRangeQuery(
60+
String field,
61+
byte[] lowerPoint,
62+
byte[] upperPoint,
63+
int numDims,
64+
int size,
65+
SortOrder sortOrder,
66+
Function<byte[], String> valueToString
67+
) {
5668
this.size = size;
5769
this.sortOrder = sortOrder;
5870
this.pointRangeQuery = new PointRangeQuery(field, lowerPoint, upperPoint, numDims) {
5971
@Override
6072
protected String toString(int dimension, byte[] value) {
61-
return super.toString(field);
73+
return valueToString.apply(value);
6274
}
6375
};
6476
}
@@ -79,6 +91,11 @@ public void setSortOrder(SortOrder sortOrder) {
7991
this.sortOrder = sortOrder;
8092
}
8193

94+
@Override
95+
public Query rewrite(IndexSearcher indexSearcher) throws IOException {
96+
return super.rewrite(indexSearcher);
97+
}
98+
8299
@Override
83100
public void visit(QueryVisitor visitor) {
84101
pointRangeQuery.visit(visitor);
@@ -344,7 +361,6 @@ public ScorerSupplier scorerSupplier(LeafReaderContext context) throws IOExcepti
344361
if (checkValidPointValues(values) == false) {
345362
return null;
346363
}
347-
final Weight weight = this;
348364
if (size > values.size()) {
349365
return pointRangeQueryWeight.scorerSupplier(context);
350366
} else {
@@ -426,17 +442,26 @@ public boolean canApproximate(SearchContext context) {
426442
}
427443
// size 0 could be set for caching
428444
if (context.from() + context.size() == 0) {
429-
this.setSize(10_000);
445+
this.setSize(SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO);
446+
} else {
447+
this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()));
430448
}
431-
this.setSize(Math.max(context.from() + context.size(), context.trackTotalHitsUpTo()));
432449
if (context.request() != null && context.request().source() != null) {
433450
FieldSortBuilder primarySortField = FieldSortBuilder.getPrimaryFieldSortOrNull(context.request().source());
434-
if (primarySortField != null
435-
&& primarySortField.missing() == null
436-
&& primarySortField.getFieldName().equals(((RangeQueryBuilder) context.request().source().query()).fieldName())) {
437-
if (primarySortField.order() == SortOrder.DESC) {
438-
this.setSortOrder(SortOrder.DESC);
451+
if (primarySortField != null) {
452+
if (!primarySortField.fieldName().equals(pointRangeQuery.getField())) {
453+
return false;
454+
}
455+
if (primarySortField.missing() != null) {
456+
// Cannot sort documents missing this field.
457+
return false;
439458
}
459+
if (context.request().source().searchAfter() != null) {
460+
// TODO: We *could* optimize searchAfter, especially when this is the only sort field, but existing pruning is pretty
461+
// good.
462+
return false;
463+
}
464+
this.setSortOrder(primarySortField.order());
440465
}
441466
}
442467
return true;
@@ -453,56 +478,16 @@ public final boolean equals(Object o) {
453478
}
454479

455480
private boolean equalsTo(ApproximatePointRangeQuery other) {
456-
return Objects.equals(pointRangeQuery.getField(), other.pointRangeQuery.getField())
457-
&& pointRangeQuery.getNumDims() == other.pointRangeQuery.getNumDims()
458-
&& pointRangeQuery.getBytesPerDim() == other.pointRangeQuery.getBytesPerDim()
459-
&& Arrays.equals(pointRangeQuery.getLowerPoint(), other.pointRangeQuery.getLowerPoint())
460-
&& Arrays.equals(pointRangeQuery.getUpperPoint(), other.pointRangeQuery.getUpperPoint());
481+
return Objects.equals(pointRangeQuery, other.pointRangeQuery);
461482
}
462483

463484
@Override
464485
public final String toString(String field) {
465486
final StringBuilder sb = new StringBuilder();
466-
if (pointRangeQuery.getField().equals(field) == false) {
467-
sb.append(pointRangeQuery.getField());
468-
sb.append(':');
469-
}
470-
471-
// print ourselves as "range per dimension"
472-
for (int i = 0; i < pointRangeQuery.getNumDims(); i++) {
473-
if (i > 0) {
474-
sb.append(',');
475-
}
476-
477-
int startOffset = pointRangeQuery.getBytesPerDim() * i;
478-
479-
sb.append('[');
480-
sb.append(
481-
toString(
482-
i,
483-
ArrayUtil.copyOfSubArray(pointRangeQuery.getLowerPoint(), startOffset, startOffset + pointRangeQuery.getBytesPerDim())
484-
)
485-
);
486-
sb.append(" TO ");
487-
sb.append(
488-
toString(
489-
i,
490-
ArrayUtil.copyOfSubArray(pointRangeQuery.getUpperPoint(), startOffset, startOffset + pointRangeQuery.getBytesPerDim())
491-
)
492-
);
493-
sb.append(']');
494-
}
487+
sb.append("Approximate(");
488+
sb.append(pointRangeQuery.toString());
489+
sb.append(")");
495490

496491
return sb.toString();
497492
}
498-
499-
/**
500-
* Returns a string of a single value in a human-readable format for debugging. This is used by
501-
* {@link #toString()}.
502-
*
503-
* @param dimension dimension of the particular value
504-
* @param value single value, never null
505-
* @return human readable value for debugging
506-
*/
507-
protected abstract String toString(int dimension, byte[] value);
508493
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,10 @@ public ApproximateQuery getApproximationQuery() {
4242
}
4343

4444
@Override
45-
public final Query rewrite(IndexSearcher indexSearcher) throws IOException {
45+
public Query rewrite(IndexSearcher indexSearcher) throws IOException {
4646
if (resolvedQuery == null) {
47-
throw new IllegalStateException("Cannot rewrite resolved query without setContext being called");
47+
// Default to the original query. This suggests that we were not called from ContextIndexSearcher.
48+
return originalQuery.rewrite(indexSearcher);
4849
}
4950
return resolvedQuery.rewrite(indexSearcher);
5051
}

‎server/src/main/java/org/opensearch/search/internal/ContextIndexSearcher.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,9 @@ public void setAggregatedDfs(AggregatedDfs aggregatedDfs) {
191191

192192
@Override
193193
public Query rewrite(Query original) throws IOException {
194+
if (original instanceof ApproximateScoreQuery) {
195+
((ApproximateScoreQuery) original).setContext(searchContext);
196+
}
194197
if (profiler != null) {
195198
profiler.startRewriteTime();
196199
}
@@ -221,9 +224,6 @@ public Weight createWeight(Query query, ScoreMode scoreMode, float boost) throws
221224
profiler.pollLastElement();
222225
}
223226
return new ProfileWeight(query, weight, profile);
224-
} else if (query instanceof ApproximateScoreQuery) {
225-
((ApproximateScoreQuery) query).setContext(searchContext);
226-
return super.createWeight(query, scoreMode, boost);
227227
} else {
228228
return super.createWeight(query, scoreMode, boost);
229229
}

0 commit comments

Comments
 (0)