From 9950cbad8e9fa27c38f0c47b0cee088ecd17e5c1 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Mon, 14 Oct 2024 00:01:23 -0700 Subject: [PATCH] Remove ApproximateIndexOrDocValuesQuery (#16273) (#16291) 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. --------- (cherry picked from commit 53c9ddf77c5d050d188fd342035f932600a41608) Signed-off-by: Michael Froh Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- CHANGELOG.md | 1 + .../index/mapper/DateFieldMapper.java | 59 ++++----- .../bucket/filterrewrite/Helper.java | 4 +- .../ApproximateIndexOrDocValuesQuery.java | 62 ---------- .../ApproximatePointRangeQuery.java | 3 - .../approximate/ApproximateScoreQuery.java | 4 +- .../index/mapper/DateFieldTypeTests.java | 40 ++++--- ...angeFieldQueryStringQueryBuilderTests.java | 18 +-- .../index/mapper/RangeFieldTypeTests.java | 4 +- .../query/MatchPhraseQueryBuilderTests.java | 4 +- .../query/QueryStringQueryBuilderTests.java | 17 +-- .../index/query/RangeQueryBuilderTests.java | 62 +++++----- ...ApproximateIndexOrDocValuesQueryTests.java | 113 ------------------ 13 files changed, 119 insertions(+), 272 deletions(-) delete mode 100644 server/src/main/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQuery.java delete mode 100644 server/src/test/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQueryTests.java diff --git a/CHANGELOG.md b/CHANGELOG.md index d238936748f19..388c391ef1ea7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Remove identity-related feature flagged code from the RestController ([#15430](https://github.com/opensearch-project/OpenSearch/pull/15430)) - Remove Identity FeatureFlag ([#16024](https://github.com/opensearch-project/OpenSearch/pull/16024)) - Ensure RestHandler.Wrapper delegates all implementations to the wrapped handler ([#16154](https://github.com/opensearch-project/OpenSearch/pull/16154)) +- Code cleanup: Remove ApproximateIndexOrDocValuesQuery ([#16273](https://github.com/opensearch-project/OpenSearch/pull/16273)) ### Deprecated diff --git a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java index 4df06c59b4508..778b130b75e55 100644 --- a/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java +++ b/server/src/main/java/org/opensearch/index/mapper/DateFieldMapper.java @@ -62,8 +62,8 @@ import org.opensearch.index.query.QueryRewriteContext; import org.opensearch.index.query.QueryShardContext; import org.opensearch.search.DocValueFormat; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; import org.opensearch.search.approximate.ApproximatePointRangeQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.search.lookup.SearchLookup; import java.io.IOException; @@ -112,21 +112,6 @@ public static DateFormatter getDefaultDateTimeFormatter() { : LEGACY_DEFAULT_DATE_TIME_FORMATTER; } - public static Query getDefaultQuery(Query pointRangeQuery, Query dvQuery, String name, long l, long u) { - return FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING) - ? new ApproximateIndexOrDocValuesQuery( - pointRangeQuery, - new ApproximatePointRangeQuery(name, pack(new long[] { l }).bytes, pack(new long[] { u }).bytes, new long[] { l }.length) { - @Override - protected String toString(int dimension, byte[] value) { - return Long.toString(LongPoint.decodeDimension(value, 0)); - } - }, - dvQuery - ) - : new IndexOrDocValuesQuery(pointRangeQuery, dvQuery); - } - /** * Resolution of the date time * @@ -487,22 +472,42 @@ public Query rangeQuery( } DateMathParser parser = forcedDateParser == null ? dateMathParser : forcedDateParser; return dateRangeQuery(lowerTerm, upperTerm, includeLower, includeUpper, timeZone, parser, context, resolution, (l, u) -> { - Query pointRangeQuery = isSearchable() ? LongPoint.newRangeQuery(name(), l, u) : null; Query dvQuery = hasDocValues() ? SortedNumericDocValuesField.newSlowRangeQuery(name(), l, u) : null; - if (isSearchable() && hasDocValues()) { - Query query = getDefaultQuery(pointRangeQuery, dvQuery, name(), l, u); - if (context.indexSortedOnField(name())) { - query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query); + if (isSearchable()) { + Query pointRangeQuery = LongPoint.newRangeQuery(name(), l, u); + Query query; + if (dvQuery != null) { + query = new IndexOrDocValuesQuery(pointRangeQuery, dvQuery); + if (context.indexSortedOnField(name())) { + query = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, query); + } + } else { + query = pointRangeQuery; + } + if (FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY_SETTING)) { + return new ApproximateScoreQuery( + query, + new ApproximatePointRangeQuery( + name(), + pack(new long[] { l }).bytes, + pack(new long[] { u }).bytes, + new long[] { l }.length + ) { + @Override + protected String toString(int dimension, byte[] value) { + return Long.toString(LongPoint.decodeDimension(value, 0)); + } + } + ); } return query; } - if (hasDocValues()) { - if (context.indexSortedOnField(name())) { - dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, dvQuery); - } - return dvQuery; + + // Not searchable. Must have doc values. + if (context.indexSortedOnField(name())) { + dvQuery = new IndexSortSortedNumericDocValuesRangeQuery(name(), l, u, dvQuery); } - return pointRangeQuery; + return dvQuery; }); } diff --git a/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Helper.java b/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Helper.java index 17da7e5712be8..356380a687003 100644 --- a/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Helper.java +++ b/server/src/main/java/org/opensearch/search/aggregations/bucket/filterrewrite/Helper.java @@ -23,7 +23,7 @@ import org.opensearch.common.lucene.search.function.FunctionScoreQuery; import org.opensearch.index.mapper.DateFieldMapper; import org.opensearch.index.query.DateRangeIncludingNowQuery; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.search.internal.SearchContext; import java.io.IOException; @@ -55,7 +55,7 @@ private Helper() {} queryWrappers.put(FunctionScoreQuery.class, q -> ((FunctionScoreQuery) q).getSubQuery()); queryWrappers.put(DateRangeIncludingNowQuery.class, q -> ((DateRangeIncludingNowQuery) q).getQuery()); queryWrappers.put(IndexOrDocValuesQuery.class, q -> ((IndexOrDocValuesQuery) q).getIndexQuery()); - queryWrappers.put(ApproximateIndexOrDocValuesQuery.class, q -> ((ApproximateIndexOrDocValuesQuery) q).getOriginalQuery()); + queryWrappers.put(ApproximateScoreQuery.class, q -> ((ApproximateScoreQuery) q).getOriginalQuery()); } /** diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQuery.java deleted file mode 100644 index b99e0a0cbf808..0000000000000 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQuery.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search.approximate; - -import org.apache.lucene.search.IndexOrDocValuesQuery; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.QueryVisitor; - -/** - * A wrapper around {@link IndexOrDocValuesQuery} that can be used to run approximate queries. - * It delegates to either {@link ApproximateQuery} or {@link IndexOrDocValuesQuery} based on whether the query can be approximated or not. - * @see ApproximateQuery - */ -public final class ApproximateIndexOrDocValuesQuery extends ApproximateScoreQuery { - - private final ApproximateQuery approximateIndexQuery; - private final IndexOrDocValuesQuery indexOrDocValuesQuery; - - public ApproximateIndexOrDocValuesQuery(Query indexQuery, ApproximateQuery approximateIndexQuery, Query dvQuery) { - super(new IndexOrDocValuesQuery(indexQuery, dvQuery), approximateIndexQuery); - this.approximateIndexQuery = approximateIndexQuery; - this.indexOrDocValuesQuery = new IndexOrDocValuesQuery(indexQuery, dvQuery); - } - - @Override - public String toString(String field) { - return "ApproximateIndexOrDocValuesQuery(indexQuery=" - + indexOrDocValuesQuery.getIndexQuery().toString(field) - + ", approximateIndexQuery=" - + approximateIndexQuery.toString(field) - + ", dvQuery=" - + indexOrDocValuesQuery.getRandomAccessQuery().toString(field) - + ")"; - } - - @Override - public void visit(QueryVisitor visitor) { - indexOrDocValuesQuery.visit(visitor); - } - - @Override - public boolean equals(Object obj) { - if (sameClassAs(obj) == false) { - return false; - } - return true; - } - - @Override - public int hashCode() { - int h = classHash(); - h = 31 * h + indexOrDocValuesQuery.getIndexQuery().hashCode(); - h = 31 * h + indexOrDocValuesQuery.getRandomAccessQuery().hashCode(); - return h; - } -} diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java index 8076da6ab970b..6ff01f5f39d36 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximatePointRangeQuery.java @@ -433,9 +433,6 @@ public boolean canApproximate(SearchContext context) { if (context.aggregations() != null) { return false; } - if (!(context.query() instanceof ApproximateIndexOrDocValuesQuery)) { - return false; - } // size 0 could be set for caching if (context.from() + context.size() == 0) { this.setSize(10_000); diff --git a/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java b/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java index d1dd32b239f28..2395142c606ae 100644 --- a/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java +++ b/server/src/main/java/org/opensearch/search/approximate/ApproximateScoreQuery.java @@ -21,12 +21,12 @@ * Entry-point for the approximation framework. * 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. */ -public class ApproximateScoreQuery extends Query { +public final class ApproximateScoreQuery extends Query { private final Query originalQuery; private final ApproximateQuery approximationQuery; - protected Query resolvedQuery; + Query resolvedQuery; public ApproximateScoreQuery(Query originalQuery, ApproximateQuery approximationQuery) { this.originalQuery = originalQuery; diff --git a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java index df800067143e4..fe5994011f1b9 100644 --- a/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/DateFieldTypeTests.java @@ -41,6 +41,7 @@ import org.apache.lucene.index.MultiReader; import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery; import org.apache.lucene.search.Query; @@ -65,8 +66,8 @@ import org.opensearch.index.query.DateRangeIncludingNowQuery; import org.opensearch.index.query.QueryRewriteContext; import org.opensearch.index.query.QueryShardContext; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; import org.opensearch.search.approximate.ApproximatePointRangeQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.joda.time.DateTimeZone; import java.io.IOException; @@ -213,8 +214,11 @@ public void testTermQuery() { MappedFieldType ft = new DateFieldType("field"); String date = "2015-10-12T14:10:55"; long instant = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date)).toInstant().toEpochMilli(); - Query expected = new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery("field", instant, instant + 999), + Query expected = new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery("field", instant, instant + 999), + SortedNumericDocValuesField.newSlowRangeQuery("field", instant, instant + 999) + ), new ApproximatePointRangeQuery( "field", pack(new long[] { instant }).bytes, @@ -225,8 +229,7 @@ public void testTermQuery() { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery("field", instant, instant + 999) + } ); assumeThat( "Using Approximate Range Query as default", @@ -279,8 +282,11 @@ public void testRangeQuery() throws IOException { String date2 = "2016-04-28T11:33:52"; long instant1 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date1)).toInstant().toEpochMilli(); long instant2 = DateFormatters.from(DateFieldMapper.getDefaultDateTimeFormatter().parse(date2)).toInstant().toEpochMilli() + 999; - Query expected = new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery("field", instant1, instant2), + Query expected = new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery("field", instant1, instant2), + SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2) + ), new ApproximatePointRangeQuery( "field", pack(new long[] { instant1 }).bytes, @@ -291,8 +297,7 @@ public void testRangeQuery() throws IOException { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2) + } ); assumeThat( "Using Approximate Range Query as default", @@ -307,8 +312,11 @@ protected String toString(int dimension, byte[] value) { instant1 = nowInMillis; instant2 = instant1 + 100; expected = new DateRangeIncludingNowQuery( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery("field", instant1, instant2), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery("field", instant1, instant2), + SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2) + ), new ApproximatePointRangeQuery( "field", pack(new long[] { instant1 }).bytes, @@ -319,8 +327,7 @@ protected String toString(int dimension, byte[] value) { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery("field", instant1, instant2) + } ) ); assumeThat( @@ -389,8 +396,8 @@ public void testRangeQueryWithIndexSort() { "field", instant1, instant2, - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery("field", instant1, instant2), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery(LongPoint.newRangeQuery("field", instant1, instant2), dvQuery), new ApproximatePointRangeQuery( "field", pack(new long[] { instant1 }).bytes, @@ -401,8 +408,7 @@ public void testRangeQueryWithIndexSort() { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - dvQuery + } ) ); assumeThat( diff --git a/server/src/test/java/org/opensearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java index 7a8ac829bdd97..a11a16f421783 100644 --- a/server/src/test/java/org/opensearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/RangeFieldQueryStringQueryBuilderTests.java @@ -50,8 +50,8 @@ import org.opensearch.common.util.FeatureFlags; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.query.QueryStringQueryBuilder; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; import org.opensearch.search.approximate.ApproximatePointRangeQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -191,11 +191,14 @@ public void testDateRangeQuery() throws Exception { is(true) ); assertEquals( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery( - DATE_FIELD_NAME, - parser.parse(lowerBoundExact, () -> 0).toEpochMilli(), - parser.parse(upperBoundExact, () -> 0).toEpochMilli() + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery( + DATE_FIELD_NAME, + parser.parse(lowerBoundExact, () -> 0).toEpochMilli(), + parser.parse(upperBoundExact, () -> 0).toEpochMilli() + ), + controlDv ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, @@ -207,8 +210,7 @@ public void testDateRangeQuery() throws Exception { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - controlDv + } ), queryOnDateField ); diff --git a/server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java b/server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java index b157c43e45451..04d4e6b3f852e 100644 --- a/server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java +++ b/server/src/test/java/org/opensearch/index/mapper/RangeFieldTypeTests.java @@ -57,7 +57,7 @@ import org.opensearch.index.mapper.RangeFieldMapper.RangeFieldType; import org.opensearch.index.query.QueryShardContext; import org.opensearch.index.query.QueryShardException; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.test.IndexSettingsModule; import org.joda.time.DateTime; import org.junit.Before; @@ -293,7 +293,7 @@ public void testDateRangeQueryUsingMappingFormatLegacy() { ); assertEquals( "field:[1465975790000 TO 1466062190999]", - ((IndexOrDocValuesQuery) ((ApproximateIndexOrDocValuesQuery) queryOnDateField).getOriginalQuery()).getIndexQuery().toString() + ((IndexOrDocValuesQuery) ((ApproximateScoreQuery) queryOnDateField).getOriginalQuery()).getIndexQuery().toString() ); } diff --git a/server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java index ddf58073a5206..48f697cff1bea 100644 --- a/server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/MatchPhraseQueryBuilderTests.java @@ -42,7 +42,7 @@ import org.apache.lucene.search.TermQuery; import org.opensearch.core.common.ParsingException; import org.opensearch.index.search.MatchQuery.ZeroTermsQuery; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.test.AbstractQueryTestCase; import java.io.IOException; @@ -131,7 +131,7 @@ protected void doAssertLuceneQuery(MatchPhraseQueryBuilder queryBuilder, Query q .or(instanceOf(PointRangeQuery.class)) .or(instanceOf(IndexOrDocValuesQuery.class)) .or(instanceOf(MatchNoDocsQuery.class)) - .or(instanceOf(ApproximateIndexOrDocValuesQuery.class)) + .or(instanceOf(ApproximateScoreQuery.class)) ); } diff --git a/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java index 5b030df20e889..6f295100d9e47 100644 --- a/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/QueryStringQueryBuilderTests.java @@ -47,6 +47,7 @@ import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.DisjunctionMaxQuery; import org.apache.lucene.search.FuzzyQuery; +import org.apache.lucene.search.IndexOrDocValuesQuery; import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.MultiTermQuery; @@ -76,8 +77,8 @@ import org.opensearch.index.mapper.FieldNamesFieldMapper; import org.opensearch.index.mapper.MapperService; import org.opensearch.index.search.QueryStringQueryParser; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; import org.opensearch.search.approximate.ApproximatePointRangeQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.test.AbstractQueryTestCase; import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; @@ -863,7 +864,7 @@ public void testToQueryDateWithTimeZone() throws Exception { FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY), is(true) ); - assertThat(query, instanceOf(ApproximateIndexOrDocValuesQuery.class)); + assertThat(query, instanceOf(ApproximateScoreQuery.class)); long lower = 0; // 1970-01-01T00:00:00.999 UTC long upper = 86399999; // 1970-01-01T23:59:59.999 UTC assertEquals(calculateExpectedDateQuery(lower, upper), query); @@ -872,9 +873,12 @@ public void testToQueryDateWithTimeZone() throws Exception { assertEquals(calculateExpectedDateQuery(lower + msPerHour, upper + msPerHour), qsq.timeZone("-01:00").toQuery(context)); } - private ApproximateIndexOrDocValuesQuery calculateExpectedDateQuery(long lower, long upper) { - return new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + private ApproximateScoreQuery calculateExpectedDateQuery(long lower, long upper) { + return new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, pack(new long[] { lower }).bytes, @@ -885,8 +889,7 @@ private ApproximateIndexOrDocValuesQuery calculateExpectedDateQuery(long lower, protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + } ); } diff --git a/server/src/test/java/org/opensearch/index/query/RangeQueryBuilderTests.java b/server/src/test/java/org/opensearch/index/query/RangeQueryBuilderTests.java index 601a66d229131..86b9a96dcf2f7 100644 --- a/server/src/test/java/org/opensearch/index/query/RangeQueryBuilderTests.java +++ b/server/src/test/java/org/opensearch/index/query/RangeQueryBuilderTests.java @@ -55,9 +55,9 @@ import org.opensearch.index.mapper.MappedFieldType; import org.opensearch.index.mapper.MappedFieldType.Relation; import org.opensearch.index.mapper.MapperService; -import org.opensearch.search.approximate.ApproximateIndexOrDocValuesQuery; import org.opensearch.search.approximate.ApproximatePointRangeQuery; import org.opensearch.search.approximate.ApproximateQuery; +import org.opensearch.search.approximate.ApproximateScoreQuery; import org.opensearch.test.AbstractQueryTestCase; import org.joda.time.DateTime; import org.joda.time.chrono.ISOChronology; @@ -196,10 +196,10 @@ protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query, FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY), is(true) ); - assertThat(query, instanceOf(ApproximateIndexOrDocValuesQuery.class)); - Query approximationQuery = ((ApproximateIndexOrDocValuesQuery) query).getApproximationQuery(); + assertThat(query, instanceOf(ApproximateScoreQuery.class)); + Query approximationQuery = ((ApproximateScoreQuery) query).getApproximationQuery(); assertThat(approximationQuery, instanceOf(ApproximateQuery.class)); - Query originalQuery = ((ApproximateIndexOrDocValuesQuery) query).getOriginalQuery(); + Query originalQuery = ((ApproximateScoreQuery) query).getOriginalQuery(); assertThat(originalQuery, instanceOf(IndexOrDocValuesQuery.class)); MapperService mapperService = context.getMapperService(); MappedFieldType mappedFieldType = mapperService.fieldType(expectedFieldName); @@ -250,8 +250,11 @@ protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query, } } assertEquals( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery(DATE_FIELD_NAME, minLong, maxLong), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery(DATE_FIELD_NAME, minLong, maxLong), + SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, minLong, maxLong) + ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, pack(new long[] { minLong }).bytes, @@ -262,8 +265,7 @@ protected void doAssertLuceneQuery(RangeQueryBuilder queryBuilder, Query query, protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, minLong, maxLong) + } ), query ); @@ -336,16 +338,19 @@ public void testDateRangeQueryFormat() throws IOException { FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY), is(true) ); - assertThat(parsedQuery, instanceOf(ApproximateIndexOrDocValuesQuery.class)); - Query approximationQuery = ((ApproximateIndexOrDocValuesQuery) parsedQuery).getApproximationQuery(); + assertThat(parsedQuery, instanceOf(ApproximateScoreQuery.class)); + Query approximationQuery = ((ApproximateScoreQuery) parsedQuery).getApproximationQuery(); assertThat(approximationQuery, instanceOf(ApproximateQuery.class)); - Query originalQuery = ((ApproximateIndexOrDocValuesQuery) parsedQuery).getOriginalQuery(); + Query originalQuery = ((ApproximateScoreQuery) parsedQuery).getOriginalQuery(); assertThat(originalQuery, instanceOf(IndexOrDocValuesQuery.class)); long lower = DateTime.parse("2012-01-01T00:00:00.000+00").getMillis(); long upper = DateTime.parse("2030-01-01T00:00:00.000+00").getMillis() - 1; assertEquals( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, pack(new long[] { lower }).bytes, @@ -356,8 +361,7 @@ public void testDateRangeQueryFormat() throws IOException { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + } ), parsedQuery ); @@ -394,13 +398,16 @@ public void testDateRangeBoundaries() throws IOException { FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY), is(true) ); - assertThat(parsedQuery, instanceOf(ApproximateIndexOrDocValuesQuery.class)); + assertThat(parsedQuery, instanceOf(ApproximateScoreQuery.class)); long lower = DateTime.parse("2014-11-01T00:00:00.000+00").getMillis(); long upper = DateTime.parse("2014-12-08T23:59:59.999+00").getMillis(); assertEquals( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, pack(new long[] { lower }).bytes, @@ -411,8 +418,7 @@ public void testDateRangeBoundaries() throws IOException { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + } ) , @@ -430,12 +436,15 @@ protected String toString(int dimension, byte[] value) { + " }\n" + "}"; parsedQuery = parseQuery(query).toQuery(createShardContext()); - assertThat(parsedQuery, instanceOf(ApproximateIndexOrDocValuesQuery.class)); + assertThat(parsedQuery, instanceOf(ApproximateScoreQuery.class)); lower = DateTime.parse("2014-11-30T23:59:59.999+00").getMillis() + 1; upper = DateTime.parse("2014-12-08T00:00:00.000+00").getMillis() - 1; assertEquals( - new ApproximateIndexOrDocValuesQuery( - LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + new ApproximateScoreQuery( + new IndexOrDocValuesQuery( + LongPoint.newRangeQuery(DATE_FIELD_NAME, lower, upper), + SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + ), new ApproximatePointRangeQuery( DATE_FIELD_NAME, pack(new long[] { lower }).bytes, @@ -446,8 +455,7 @@ protected String toString(int dimension, byte[] value) { protected String toString(int dimension, byte[] value) { return Long.toString(LongPoint.decodeDimension(value, 0)); } - }, - SortedNumericDocValuesField.newSlowRangeQuery(DATE_FIELD_NAME, lower, upper) + } ) , @@ -476,8 +484,8 @@ public void testDateRangeQueryTimezone() throws IOException { FeatureFlags.isEnabled(FeatureFlags.APPROXIMATE_POINT_RANGE_QUERY), is(true) ); - assertThat(parsedQuery, instanceOf(ApproximateIndexOrDocValuesQuery.class)); - parsedQuery = ((ApproximateIndexOrDocValuesQuery) parsedQuery).getApproximationQuery(); + assertThat(parsedQuery, instanceOf(ApproximateScoreQuery.class)); + parsedQuery = ((ApproximateScoreQuery) parsedQuery).getApproximationQuery(); assertThat(parsedQuery, instanceOf(ApproximateQuery.class)); // TODO what else can we assert diff --git a/server/src/test/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQueryTests.java b/server/src/test/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQueryTests.java deleted file mode 100644 index 47f87c6abf629..0000000000000 --- a/server/src/test/java/org/opensearch/search/approximate/ApproximateIndexOrDocValuesQueryTests.java +++ /dev/null @@ -1,113 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * - * The OpenSearch Contributors require contributions made to - * this file be licensed under the Apache-2.0 license or a - * compatible open source license. - */ - -package org.opensearch.search.approximate; - -import org.apache.lucene.document.LongPoint; -import org.apache.lucene.document.SortedNumericDocValuesField; -import org.apache.lucene.index.DirectoryReader; -import org.apache.lucene.index.IndexWriter; -import org.apache.lucene.search.ConstantScoreWeight; -import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.ScoreMode; -import org.apache.lucene.search.Weight; -import org.apache.lucene.store.Directory; -import org.opensearch.search.internal.SearchContext; -import org.opensearch.test.OpenSearchTestCase; -import org.junit.After; -import org.junit.Before; - -import java.io.IOException; - -import static org.apache.lucene.document.LongPoint.pack; - -public class ApproximateIndexOrDocValuesQueryTests extends OpenSearchTestCase { - private Directory dir; - private IndexWriter w; - private DirectoryReader reader; - private IndexSearcher searcher; - - @Before - public void initSearcher() throws IOException { - dir = newDirectory(); - w = new IndexWriter(dir, newIndexWriterConfig()); - } - - @After - public void closeAllTheReaders() throws IOException { - reader.close(); - w.close(); - dir.close(); - } - - public void testApproximateIndexOrDocValuesQueryWeight() throws IOException { - - long l = Long.MIN_VALUE; - long u = Long.MAX_VALUE; - Query indexQuery = LongPoint.newRangeQuery("test-index", l, u); - - ApproximateQuery approximateIndexQuery = new ApproximatePointRangeQuery( - "test-index", - pack(new long[] { l }).bytes, - pack(new long[] { u }).bytes, - new long[] { l }.length - ) { - protected String toString(int dimension, byte[] value) { - return Long.toString(LongPoint.decodeDimension(value, 0)); - } - }; - - Query dvQuery = SortedNumericDocValuesField.newSlowRangeQuery("test-index", l, u); - - ApproximateIndexOrDocValuesQuery approximateIndexOrDocValuesQuery = new ApproximateIndexOrDocValuesQuery( - indexQuery, - approximateIndexQuery, - dvQuery - ); - - reader = DirectoryReader.open(w); - searcher = newSearcher(reader); - - approximateIndexOrDocValuesQuery.resolvedQuery = indexQuery; - - Weight weight = approximateIndexOrDocValuesQuery.rewrite(searcher).createWeight(searcher, ScoreMode.COMPLETE, 1f); - - assertTrue(weight instanceof ConstantScoreWeight); - - ApproximateQuery approximateIndexQueryCanApproximate = new ApproximatePointRangeQuery( - "test-index", - pack(new long[] { l }).bytes, - pack(new long[] { u }).bytes, - new long[] { l }.length - ) { - protected String toString(int dimension, byte[] value) { - return Long.toString(LongPoint.decodeDimension(value, 0)); - } - - public boolean canApproximate(SearchContext context) { - return true; - } - }; - - ApproximateIndexOrDocValuesQuery approximateIndexOrDocValuesQueryCanApproximate = new ApproximateIndexOrDocValuesQuery( - indexQuery, - approximateIndexQueryCanApproximate, - dvQuery - ); - - approximateIndexOrDocValuesQueryCanApproximate.resolvedQuery = approximateIndexQueryCanApproximate; - - Weight approximateIndexOrDocValuesQueryCanApproximateWeight = approximateIndexOrDocValuesQueryCanApproximate.rewrite(searcher) - .createWeight(searcher, ScoreMode.COMPLETE, 1f); - - // we get ConstantScoreWeight since we're expecting to call ApproximatePointRangeQuery - assertTrue(approximateIndexOrDocValuesQueryCanApproximateWeight instanceof ConstantScoreWeight); - - } -}