Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new cluster setting for keyword indexordocvalues query #15637

Merged
merged 3 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- ClusterManagerTaskThrottler Improvements ([#15508](https://github.com/opensearch-project/OpenSearch/pull/15508))
- Reset DiscoveryNodes in all transport node actions request ([#15131](https://github.com/opensearch-project/OpenSearch/pull/15131))
- Relax the join validation for Remote State publication ([#15471](https://github.com/opensearch-project/OpenSearch/pull/15471))
- MultiTermQueries in keyword fields now default to `indexed` approach and gated behind cluster setting ([#15637](https://github.com/opensearch-project/OpenSearch/pull/15637))

### Dependencies
- Bump `netty` from 4.1.111.Final to 4.1.112.Final ([#15081](https://github.com/opensearch-project/OpenSearch/pull/15081))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ public void apply(Settings value, Settings current, Settings previous) {
SearchService.MAX_AGGREGATION_REWRITE_FILTERS,
SearchService.INDICES_MAX_CLAUSE_COUNT_SETTING,
SearchService.CARDINALITY_AGGREGATION_PRUNING_THRESHOLD,
SearchService.KEYWORD_INDEX_OR_DOC_VALUES_ENABLED,
CreatePitController.PIT_INIT_KEEP_ALIVE,
Node.WRITE_PORTS_FILE_SETTING,
Node.NODE_NAME_SETTING,
Expand Down
8 changes: 5 additions & 3 deletions server/src/main/java/org/opensearch/index/IndexService.java
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,7 @@ public IndexSettings getIndexSettings() {
* {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
*/
public QueryShardContext newQueryShardContext(int shardId, IndexSearcher searcher, LongSupplier nowInMillis, String clusterAlias) {
return newQueryShardContext(shardId, searcher, nowInMillis, clusterAlias, false);
return newQueryShardContext(shardId, searcher, nowInMillis, clusterAlias, false, false);
}

/**
Expand All @@ -867,7 +867,8 @@ public QueryShardContext newQueryShardContext(
IndexSearcher searcher,
LongSupplier nowInMillis,
String clusterAlias,
boolean validate
boolean validate,
boolean keywordIndexOrDocValuesEnabled
msfroh marked this conversation as resolved.
Show resolved Hide resolved
) {
final SearchIndexNameMatcher indexNameMatcher = new SearchIndexNameMatcher(
index().getName(),
Expand All @@ -893,7 +894,8 @@ public QueryShardContext newQueryShardContext(
indexNameMatcher,
allowExpensiveQueries,
valuesSourceRegistry,
validate
validate,
keywordIndexOrDocValuesEnabled
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,9 @@
failIfNotIndexedAndNoDocValues();
// has index and doc_values enabled
if (isSearchable() && hasDocValues()) {
if (!context.keywordFieldIndexOrDocValuesEnabled()) {
return super.termsQuery(values, context);
}
BytesRef[] bytesRefs = new BytesRef[values.size()];
for (int i = 0; i < bytesRefs.length; i++) {
bytesRefs[i] = indexedValueForSearch(values.get(i));
Expand Down Expand Up @@ -429,6 +432,9 @@
}
failIfNotIndexedAndNoDocValues();
if (isSearchable() && hasDocValues()) {
if (!context.keywordFieldIndexOrDocValuesEnabled()) {
return super.prefixQuery(value, method, caseInsensitive, context);
}
Query indexQuery = super.prefixQuery(value, method, caseInsensitive, context);
Query dvQuery = super.prefixQuery(value, MultiTermQuery.DOC_VALUES_REWRITE, caseInsensitive, context);
return new IndexOrDocValuesQuery(indexQuery, dvQuery);
Expand Down Expand Up @@ -461,6 +467,9 @@
}
failIfNotIndexedAndNoDocValues();
if (isSearchable() && hasDocValues()) {
if (!context.keywordFieldIndexOrDocValuesEnabled()) {
return super.regexpQuery(value, syntaxFlags, matchFlags, maxDeterminizedStates, method, context);
}
Query indexQuery = super.regexpQuery(value, syntaxFlags, matchFlags, maxDeterminizedStates, method, context);
Query dvQuery = super.regexpQuery(
value,
Expand Down Expand Up @@ -549,6 +558,9 @@
);
}
if (isSearchable() && hasDocValues()) {
if (!context.keywordFieldIndexOrDocValuesEnabled()) {
return super.fuzzyQuery(value, fuzziness, prefixLength, maxExpansions, transpositions, method, context);

Check warning on line 562 in server/src/main/java/org/opensearch/index/mapper/KeywordFieldMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/KeywordFieldMapper.java#L562

Added line #L562 was not covered by tests
}
Query indexQuery = super.fuzzyQuery(value, fuzziness, prefixLength, maxExpansions, transpositions, method, context);
Query dvQuery = super.fuzzyQuery(
value,
Expand Down Expand Up @@ -591,6 +603,9 @@
// wildcard
// query text
if (isSearchable() && hasDocValues()) {
if (!context.keywordFieldIndexOrDocValuesEnabled()) {
return super.wildcardQuery(value, method, caseInsensitive, true, context);
}
Query indexQuery = super.wildcardQuery(value, method, caseInsensitive, true, context);
Query dvQuery = super.wildcardQuery(value, MultiTermQuery.DOC_VALUES_REWRITE, caseInsensitive, true, context);
return new IndexOrDocValuesQuery(indexQuery, dvQuery);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ public class QueryShardContext extends QueryRewriteContext {
private final ValuesSourceRegistry valuesSourceRegistry;
private BitSetProducer parentFilter;
private DerivedFieldResolver derivedFieldResolver;
private boolean keywordIndexOrDocValuesEnabled;

public QueryShardContext(
int shardId,
Expand Down Expand Up @@ -208,7 +209,55 @@ public QueryShardContext(
),
allowExpensiveQueries,
valuesSourceRegistry,
validate
validate,
false
);
}

public QueryShardContext(
int shardId,
IndexSettings indexSettings,
BigArrays bigArrays,
BitsetFilterCache bitsetFilterCache,
TriFunction<MappedFieldType, String, Supplier<SearchLookup>, IndexFieldData<?>> indexFieldDataLookup,
MapperService mapperService,
SimilarityService similarityService,
ScriptService scriptService,
NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry,
Client client,
IndexSearcher searcher,
LongSupplier nowInMillis,
String clusterAlias,
Predicate<String> indexNameMatcher,
BooleanSupplier allowExpensiveQueries,
ValuesSourceRegistry valuesSourceRegistry,
boolean validate,
boolean keywordIndexOrDocValuesEnabled
) {
this(
shardId,
indexSettings,
bigArrays,
bitsetFilterCache,
indexFieldDataLookup,
mapperService,
similarityService,
scriptService,
xContentRegistry,
namedWriteableRegistry,
client,
searcher,
nowInMillis,
indexNameMatcher,
new Index(
RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName()),
indexSettings.getIndex().getUUID()
),
allowExpensiveQueries,
valuesSourceRegistry,
validate,
keywordIndexOrDocValuesEnabled
);
}

Expand All @@ -231,7 +280,8 @@ public QueryShardContext(QueryShardContext source) {
source.fullyQualifiedIndex,
source.allowExpensiveQueries,
source.valuesSourceRegistry,
source.validate()
source.validate(),
source.keywordIndexOrDocValuesEnabled
);
}

Expand All @@ -253,7 +303,8 @@ private QueryShardContext(
Index fullyQualifiedIndex,
BooleanSupplier allowExpensiveQueries,
ValuesSourceRegistry valuesSourceRegistry,
boolean validate
boolean validate,
boolean keywordIndexOrDocValuesEnabled
) {
super(xContentRegistry, namedWriteableRegistry, client, nowInMillis, validate);
this.shardId = shardId;
Expand All @@ -277,6 +328,7 @@ private QueryShardContext(
emptyList(),
indexSettings.isDerivedFieldAllowed()
);
this.keywordIndexOrDocValuesEnabled = keywordIndexOrDocValuesEnabled;
}

private void reset() {
Expand Down Expand Up @@ -414,6 +466,14 @@ public void setDerivedFieldResolver(DerivedFieldResolver derivedFieldResolver) {
this.derivedFieldResolver = derivedFieldResolver;
}

public boolean keywordFieldIndexOrDocValuesEnabled() {
return keywordIndexOrDocValuesEnabled;
}

public void setKeywordFieldIndexOrDocValuesEnabled(boolean keywordIndexOrDocValuesEnabled) {
this.keywordIndexOrDocValuesEnabled = keywordIndexOrDocValuesEnabled;
}

public void setAllowUnmappedFields(boolean allowUnmappedFields) {
this.allowUnmappedFields = allowUnmappedFields;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@
import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_ALL;
import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_AUTO;
import static org.opensearch.search.SearchService.CONCURRENT_SEGMENT_SEARCH_MODE_NONE;
import static org.opensearch.search.SearchService.KEYWORD_INDEX_OR_DOC_VALUES_ENABLED;
import static org.opensearch.search.SearchService.MAX_AGGREGATION_REWRITE_FILTERS;

/**
Expand Down Expand Up @@ -206,6 +207,7 @@ final class DefaultSearchContext extends SearchContext {
private final SetOnce<Boolean> requestShouldUseConcurrentSearch = new SetOnce<>();
private final int maxAggRewriteFilters;
private final int cardinalityAggregationPruningThreshold;
private final boolean keywordIndexOrDocValuesEnabled;

DefaultSearchContext(
ReaderContext readerContext,
Expand Down Expand Up @@ -256,7 +258,8 @@ final class DefaultSearchContext extends SearchContext {
this.searcher,
request::nowInMillis,
shardTarget.getClusterAlias(),
validate
validate,
evaluateKeywordIndexOrDocValuesEnabled()
);
queryBoost = request.indexBoost();
this.lowLevelCancellation = lowLevelCancellation;
Expand All @@ -265,6 +268,7 @@ final class DefaultSearchContext extends SearchContext {
this.maxAggRewriteFilters = evaluateFilterRewriteSetting();
this.cardinalityAggregationPruningThreshold = evaluateCardinalityAggregationPruningThreshold();
this.concurrentSearchDeciders = concurrentSearchDeciders;
this.keywordIndexOrDocValuesEnabled = evaluateKeywordIndexOrDocValuesEnabled();
}

@Override
Expand Down Expand Up @@ -1117,10 +1121,22 @@ public int cardinalityAggregationPruningThreshold() {
return cardinalityAggregationPruningThreshold;
}

@Override
public boolean keywordIndexOrDocValuesEnabled() {
return keywordIndexOrDocValuesEnabled;
}

private int evaluateCardinalityAggregationPruningThreshold() {
if (clusterService != null) {
return clusterService.getClusterSettings().get(CARDINALITY_AGGREGATION_PRUNING_THRESHOLD);
}
return 0;
}

public boolean evaluateKeywordIndexOrDocValuesEnabled() {
if (clusterService != null) {
return clusterService.getClusterSettings().get(KEYWORD_INDEX_OR_DOC_VALUES_ENABLED);
}
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,13 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv
Property.NodeScope
);

public static final Setting<Boolean> KEYWORD_INDEX_OR_DOC_VALUES_ENABLED = Setting.boolSetting(
"search.keyword_index_or_doc_values_enabled",
false,
Property.Dynamic,
Property.NodeScope
);

public static final int DEFAULT_SIZE = 10;
public static final int DEFAULT_FROM = 0;

Expand Down Expand Up @@ -1173,6 +1180,7 @@ private DefaultSearchContext createSearchContext(ReaderContext reader, ShardSear
context.getIndexSettings().isDerivedFieldAllowed() && allowDerivedField
);
context.setDerivedFieldResolver(derivedFieldResolver);
context.setKeywordFieldIndexOrDocValuesEnabled(searchContext.keywordIndexOrDocValuesEnabled());
msfroh marked this conversation as resolved.
Show resolved Hide resolved
searchContext.getQueryShardContext().setDerivedFieldResolver(derivedFieldResolver);
Rewriteable.rewrite(request.getRewriteable(), context, true);
assert searchContext.getQueryShardContext().isCacheable();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -526,4 +526,9 @@
public int cardinalityAggregationPruningThreshold() {
return 0;
}

public boolean keywordIndexOrDocValuesEnabled() {
return false;

Check warning on line 531 in server/src/main/java/org/opensearch/search/internal/SearchContext.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/internal/SearchContext.java#L531

Added line #L531 was not covered by tests
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public void testTermsQuery() {
new TermInSetQuery("field", terms),
new TermInSetQuery(MultiTermQuery.DOC_VALUES_REWRITE, "field", terms)
);
assertEquals(expected, ft.termsQuery(Arrays.asList("foo", "bar"), null));
assertEquals(expected, ft.termsQuery(Arrays.asList("foo", "bar"), MOCK_QSC_ENABLE_INDEX_DOC_VALUES));

MappedFieldType onlyIndexed = new KeywordFieldType("field", true, false, Collections.emptyMap());
Query expectedIndex = new TermInSetQuery("field", terms);
Expand Down Expand Up @@ -225,7 +225,7 @@ public void testRegexpQuery() {
new RegexpQuery(new Term("field", "foo.*")),
new RegexpQuery(new Term("field", "foo.*"), 0, 0, RegexpQuery.DEFAULT_PROVIDER, 10, MultiTermQuery.DOC_VALUES_REWRITE)
),
ft.regexpQuery("foo.*", 0, 0, 10, MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, MOCK_QSC)
ft.regexpQuery("foo.*", 0, 0, 10, MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, MOCK_QSC_ENABLE_INDEX_DOC_VALUES)
);

Query indexExpected = new RegexpQuery(new Term("field", "foo.*"));
Expand Down Expand Up @@ -267,7 +267,7 @@ public void testFuzzyQuery() {
new FuzzyQuery(new Term("field", "foo"), 2, 1, 50, true),
new FuzzyQuery(new Term("field", "foo"), 2, 1, 50, true, MultiTermQuery.DOC_VALUES_REWRITE)
),
ft.fuzzyQuery("foo", Fuzziness.fromEdits(2), 1, 50, true, null, MOCK_QSC)
ft.fuzzyQuery("foo", Fuzziness.fromEdits(2), 1, 50, true, null, MOCK_QSC_ENABLE_INDEX_DOC_VALUES)
);

Query indexExpected = new FuzzyQuery(new Term("field", "foo"), 2, 1, 50, true);
Expand Down Expand Up @@ -308,7 +308,7 @@ public void testWildCardQuery() {
MultiTermQuery.DOC_VALUES_REWRITE
)
);
assertEquals(expected, ft.wildcardQuery("foo*", MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, MOCK_QSC));
assertEquals(expected, ft.wildcardQuery("foo*", MultiTermQuery.CONSTANT_SCORE_BLENDED_REWRITE, MOCK_QSC_ENABLE_INDEX_DOC_VALUES));

Query indexExpected = new WildcardQuery(new Term("field", new BytesRef("foo*")));
MappedFieldType onlyIndexed = new KeywordFieldType("field", true, false, Collections.emptyMap());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
import java.io.IOException;
import java.util.Collections;

import static org.opensearch.index.mapper.FieldTypeTestCase.MOCK_QSC_ENABLE_INDEX_DOC_VALUES;

public class NestedHelperTests extends OpenSearchSingleNodeTestCase {

IndexService indexService;
Expand Down Expand Up @@ -132,28 +134,28 @@ public void testMatchNo() {
}

public void testTermsQuery() {
Query termsQuery = mapperService.fieldType("foo").termsQuery(Collections.singletonList("bar"), null);
Query termsQuery = mapperService.fieldType("foo").termsQuery(Collections.singletonList("bar"), MOCK_QSC_ENABLE_INDEX_DOC_VALUES);
assertFalse(new NestedHelper(mapperService).mightMatchNestedDocs(termsQuery));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested1"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested2"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested3"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested_missing"));

termsQuery = mapperService.fieldType("nested1.foo").termsQuery(Collections.singletonList("bar"), null);
termsQuery = mapperService.fieldType("nested1.foo").termsQuery(Collections.singletonList("bar"), MOCK_QSC_ENABLE_INDEX_DOC_VALUES);
assertTrue(new NestedHelper(mapperService).mightMatchNestedDocs(termsQuery));
assertFalse(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested1"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested2"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested3"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested_missing"));

termsQuery = mapperService.fieldType("nested2.foo").termsQuery(Collections.singletonList("bar"), null);
termsQuery = mapperService.fieldType("nested2.foo").termsQuery(Collections.singletonList("bar"), MOCK_QSC_ENABLE_INDEX_DOC_VALUES);
assertTrue(new NestedHelper(mapperService).mightMatchNestedDocs(termsQuery));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested1"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested2"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested3"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested_missing"));

termsQuery = mapperService.fieldType("nested3.foo").termsQuery(Collections.singletonList("bar"), null);
termsQuery = mapperService.fieldType("nested3.foo").termsQuery(Collections.singletonList("bar"), MOCK_QSC_ENABLE_INDEX_DOC_VALUES);
assertTrue(new NestedHelper(mapperService).mightMatchNestedDocs(termsQuery));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested1"));
assertTrue(new NestedHelper(mapperService).mightMatchNonNestedDocs(termsQuery, "nested2"));
Expand Down
Loading
Loading