From 27b4398387087f58065ed07d30829476e3772d12 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 20 Feb 2026 15:10:35 +0000 Subject: [PATCH 01/12] Create a new auto scope for each query store callback --- .../percolator/PercolatorQuerySearchIT.java | 58 +++++++++++++++++++ .../percolator/PercolateQueryBuilder.java | 25 +++++++- .../percolator/PercolatorFieldMapper.java | 2 +- .../query/FilteredSearchExecutionContext.java | 6 ++ 4 files changed, 87 insertions(+), 4 deletions(-) diff --git a/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java index 5e8ced116a1ff..bfc07ffc83078 100644 --- a/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java @@ -1368,4 +1368,62 @@ public void testKnnQueryNotSupportedInPercolator() throws IOException { assertThat(exception.getMessage(), containsString("the [knn] query is unsupported inside a percolator")); } + public void testPercolatorBooleanQueriesWithConcurrency() throws Exception { + assertAcked( + indicesAdmin().prepareCreate("test") + .setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1)) + .setMapping("field1", "type=long", "query", "type=percolator") + ); + + prepareIndex("test").setId("1") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14))) + .endObject() + ) + .get(); + prepareIndex("test").setId("2") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6))) + .endObject() + ) + .get(); + prepareIndex("test").setId("3") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14))) + .endObject() + ) + .get(); + prepareIndex("test").setId("4") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6))) + .endObject() + ) + .get(); + prepareIndex("test").setId("5") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(10).to(12)).must(rangeQuery("field1").from(12).to(14))) + .endObject() + ) + .get(); + prepareIndex("test").setId("6") + .setSource( + jsonBuilder().startObject() + .field("query", boolQuery().must(rangeQuery("field1").from(3).to(4)).must(rangeQuery("field1").from(4).to(6))) + .endObject() + ) + .get(); + + indicesAdmin().prepareRefresh().get(); + + BytesReference source = BytesReference.bytes(jsonBuilder().startObject().field("field1", 12).endObject()); + assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", source, XContentType.JSON)), response -> { + assertHitCount(response, 3); + }); + } + } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 2ce0f1af5b0c4..4c2beca70f31a 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -56,11 +56,13 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.index.query.AbstractQueryBuilder; +import org.elasticsearch.index.query.FilteredSearchExecutionContext; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardException; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.query.SearchExecutionContext; +import org.elasticsearch.index.query.support.AutoPrefilteringScope; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.logging.LogManager; @@ -572,8 +574,25 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy return context.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); }); - queryBuilder = Rewriteable.rewrite(queryBuilder, context); - return queryBuilder.toQuery(context); + // The QueryBuilder.toQuery() function modifies the search context's + // AutoPrefilteringScope in a way that is not thread safe. For other + // queries this is not a problem, but Percolate will call the returned + // PercolateQuery.QueryStore function from multiple threads. Here the + // solution is to create an AutoPrefilteringScope for each invocation + // of PercolateQuery.QueryStore + var safeContext = new FilteredSearchExecutionContext(context) { + + private AutoPrefilteringScope localAutoPrefilteringScope = new AutoPrefilteringScope(); + + @Override + public AutoPrefilteringScope autoPrefilteringScope() { + return localAutoPrefilteringScope; + } + }; + + queryBuilder = Rewriteable.rewrite(queryBuilder, safeContext); + // toQuery will access localAutoPrefilteringScope + return queryBuilder.toQuery(safeContext); } else { return null; } @@ -623,7 +642,7 @@ private static QueryBuilder readQueryBuilder( } } - static SearchExecutionContext wrap(SearchExecutionContext delegate) { + private static SearchExecutionContext wrap(SearchExecutionContext delegate) { return new SearchExecutionContext(delegate) { @Override diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 11688296426ca..da447e9fc6d46 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -573,7 +573,7 @@ static byte[] encodeRange(String rangeFieldName, byte[] minEncoded, byte[] maxEn // This is sane behavior for typical usage. But for percolator, the fields for the may not have any terms // Consequently, we may erroneously skip expanding those term fields. // This override allows mapped field values to expand via wildcard input, even if the field is empty in the shard. - static SearchExecutionContext wrapAllEmptyTextFields(SearchExecutionContext searchExecutionContext) { + private static SearchExecutionContext wrapAllEmptyTextFields(SearchExecutionContext searchExecutionContext) { return new FilteredSearchExecutionContext(searchExecutionContext) { @Override public boolean fieldExistsInIndex(String fieldname) { diff --git a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java index 5e26c6cbc99c8..3cf51a29b62f7 100644 --- a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java @@ -33,6 +33,7 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceLoader; import org.elasticsearch.index.mapper.SourceToParse; +import org.elasticsearch.index.query.support.AutoPrefilteringScope; import org.elasticsearch.index.query.support.NestedScope; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptContext; @@ -390,4 +391,9 @@ public void setRewriteToNamedQueries() { public boolean rewriteToNamedQuery() { return in.rewriteToNamedQuery(); } + + @Override + public AutoPrefilteringScope autoPrefilteringScope() { + return in.autoPrefilteringScope(); + } } From ba0dfce9ca12a388b33dda6eb8c859c0097f772f Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 27 Feb 2026 14:48:26 +0000 Subject: [PATCH 02/12] Use shallow copy of SEC --- .../percolator/PercolatorQuerySearchIT.java | 69 ++++++++++++++++++- .../percolator/PercolateQueryBuilder.java | 18 +---- .../query/FilteredSearchExecutionContext.java | 8 +++ .../index/query/SearchExecutionContext.java | 13 ++++ 4 files changed, 91 insertions(+), 17 deletions(-) diff --git a/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java b/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java index bfc07ffc83078..7782e805da27c 100644 --- a/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java +++ b/modules/percolator/src/internalClusterTest/java/org/elasticsearch/percolator/PercolatorQuerySearchIT.java @@ -889,7 +889,11 @@ public void testWithMultiplePercolatorFields() throws Exception { assertThat(e.getCause().getMessage(), equalTo("a document can only contain one percolator query")); } - public void testPercolateQueryWithNestedDocuments() throws Exception { + /** + * Mapping for percolator tests that use nested "employee" documents. + * Includes query (percolator), id (keyword), companyname (text), and employee (nested with name). + */ + private XContentBuilder nestedPercolatorMapping() throws IOException { XContentBuilder mapping = XContentFactory.jsonBuilder(); mapping.startObject() .startObject("properties") @@ -912,7 +916,11 @@ public void testPercolateQueryWithNestedDocuments() throws Exception { .endObject() .endObject() .endObject(); - assertAcked(indicesAdmin().prepareCreate("test").setMapping(mapping)); + return mapping; + } + + public void testPercolateQueryWithNestedDocuments() throws Exception { + assertAcked(indicesAdmin().prepareCreate("test").setMapping(nestedPercolatorMapping())); prepareIndex("test").setId("q1") .setSource( jsonBuilder().startObject() @@ -1426,4 +1434,61 @@ public void testPercolatorBooleanQueriesWithConcurrency() throws Exception { }); } + public void testPercolatorNestedQueriesWithConcurrency() throws Exception { + assertAcked( + indicesAdmin().prepareCreate("test") + .setSettings(Settings.builder().put(indexSettings()).put("index.number_of_shards", 1)) + .setMapping(nestedPercolatorMapping()) + ); + + QueryBuilder nestedVirginia = QueryBuilders.nestedQuery( + "employee", + QueryBuilders.matchQuery("employee.name", "virginia potts").operator(Operator.AND), + ScoreMode.Avg + ); + QueryBuilder nestedTony = QueryBuilders.nestedQuery( + "employee", + QueryBuilders.matchQuery("employee.name", "tony stark").operator(Operator.AND), + ScoreMode.Avg + ); + + prepareIndex("test").setId("1").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get(); + prepareIndex("test").setId("2").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get(); + prepareIndex("test").setId("3").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get(); + prepareIndex("test").setId("4").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get(); + prepareIndex("test").setId("5").setSource(jsonBuilder().startObject().field("query", nestedVirginia).endObject()).get(); + prepareIndex("test").setId("6").setSource(jsonBuilder().startObject().field("query", nestedTony).endObject()).get(); + + indicesAdmin().prepareRefresh().get(); + + BytesReference source = BytesReference.bytes( + jsonBuilder().startObject() + .startArray("employee") + .startObject() + .field("name", "virginia potts") + .endObject() + .startObject() + .field("name", "tony stark") + .endObject() + .endArray() + .endObject() + ); + assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", source, XContentType.JSON)), response -> { + assertHitCount(response, 6); + }); + + BytesReference sourceVirginiaOnly = BytesReference.bytes( + jsonBuilder().startObject() + .startArray("employee") + .startObject() + .field("name", "virginia potts") + .endObject() + .endArray() + .endObject() + ); + assertResponse(prepareSearch().setQuery(new PercolateQueryBuilder("query", sourceVirginiaOnly, XContentType.JSON)), response -> { + assertHitCount(response, 3); + }); + } + } diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 4c2beca70f31a..3a45919dd8e1e 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -56,13 +56,11 @@ import org.elasticsearch.index.mapper.ParsedDocument; import org.elasticsearch.index.mapper.SourceToParse; import org.elasticsearch.index.query.AbstractQueryBuilder; -import org.elasticsearch.index.query.FilteredSearchExecutionContext; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.QueryShardException; import org.elasticsearch.index.query.Rewriteable; import org.elasticsearch.index.query.SearchExecutionContext; -import org.elasticsearch.index.query.support.AutoPrefilteringScope; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.indices.breaker.NoneCircuitBreakerService; import org.elasticsearch.logging.LogManager; @@ -577,19 +575,9 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy // The QueryBuilder.toQuery() function modifies the search context's // AutoPrefilteringScope in a way that is not thread safe. For other // queries this is not a problem, but Percolate will call the returned - // PercolateQuery.QueryStore function from multiple threads. Here the - // solution is to create an AutoPrefilteringScope for each invocation - // of PercolateQuery.QueryStore - var safeContext = new FilteredSearchExecutionContext(context) { - - private AutoPrefilteringScope localAutoPrefilteringScope = new AutoPrefilteringScope(); - - @Override - public AutoPrefilteringScope autoPrefilteringScope() { - return localAutoPrefilteringScope; - } - }; - + // PercolateQuery.QueryStore function from multiple threads. + // Use a cloned SearchExecutionContext for each thread. + var safeContext = context.copyForConcurrentUse(); queryBuilder = Rewriteable.rewrite(queryBuilder, safeContext); // toQuery will access localAutoPrefilteringScope return queryBuilder.toQuery(safeContext); diff --git a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java index 3cf51a29b62f7..6d336a779843d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java @@ -396,4 +396,12 @@ public boolean rewriteToNamedQuery() { public AutoPrefilteringScope autoPrefilteringScope() { return in.autoPrefilteringScope(); } + + /** + * {@inheritDoc} + */ + @Override + public FilteredSearchExecutionContext copyForConcurrentUse() { + return new FilteredSearchExecutionContext(this); + } } diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java index f4d66a8d718df..e5b1bd5807bcd 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java @@ -264,6 +264,19 @@ private void reset() { this.autoPrefilteringScope = new AutoPrefilteringScope(); } + /** + * Creates a shallow copy of the context with new instances of the fields + * that are mutable - notably {@code AutoPrefilteringScope} and {@code NestedScope}. + * The mutable elements cannot be shared safely between threads, this function + * returns a copy that is safe to use in a concurrent environment without the + * overhead of a full copy. + * + * @return The shallow copy of the context. + */ + public SearchExecutionContext copyForConcurrentUse() { + return new SearchExecutionContext(this); + } + // Set alias filter, so it can be applied for queries that need it (e.g. knn query) public void setAliasFilter(QueryBuilder aliasFilter) { this.aliasFilter = aliasFilter; From e515b295163b8c4bfa5fe3f385181762295d3374 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 27 Feb 2026 15:14:22 +0000 Subject: [PATCH 03/12] fix da test --- .../percolator/QueryBuilderStoreTests.java | 71 +++++++++++++++---- 1 file changed, 56 insertions(+), 15 deletions(-) diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java index c98e70131b554..c8f8e4d6965b2 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java @@ -19,17 +19,24 @@ import org.apache.lucene.search.TermQuery; import org.apache.lucene.store.Directory; import org.elasticsearch.TransportVersion; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.CheckedFunction; +import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; +import org.elasticsearch.index.fielddata.FieldDataContext; +import org.elasticsearch.index.fielddata.IndexFieldData; import org.elasticsearch.index.fielddata.plain.BytesBinaryIndexFieldData; import org.elasticsearch.index.mapper.BinaryFieldMapper; import org.elasticsearch.index.mapper.DocumentParserContext; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperBuilderContext; +import org.elasticsearch.index.mapper.MapperMetrics; +import org.elasticsearch.index.mapper.MappingLookup; import org.elasticsearch.index.mapper.TestDocumentParserContext; +import org.elasticsearch.index.query.FilteredSearchExecutionContext; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.script.field.BinaryDocValuesField; @@ -37,13 +44,13 @@ import org.elasticsearch.search.aggregations.support.CoreValuesSourceType; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xcontent.NamedXContentRegistry; -import org.mockito.Mockito; +import org.elasticsearch.xcontent.XContentParserConfiguration; import java.io.IOException; import java.util.Collections; +import java.util.function.BiFunction; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; +import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS; public class QueryBuilderStoreTests extends ESTestCase { @@ -67,7 +74,6 @@ public void testStoringQueryBuilders() throws IOException { BinaryFieldMapper fieldMapper = PercolatorFieldMapper.Builder.createQueryBuilderFieldBuilder( MapperBuilderContext.root(false, false) ); - MappedFieldType.FielddataOperation fielddataOperation = MappedFieldType.FielddataOperation.SEARCH; try (IndexWriter indexWriter = new IndexWriter(directory, config)) { for (int i = 0; i < queryBuilders.length; i++) { @@ -84,18 +90,53 @@ public void testStoringQueryBuilders() throws IOException { } } - SearchExecutionContext searchExecutionContext = mock(SearchExecutionContext.class); - when(searchExecutionContext.indexVersionCreated()).thenReturn(IndexVersion.current()); - when(searchExecutionContext.getWriteableRegistry()).thenReturn(writableRegistry()); - when(searchExecutionContext.getParserConfig()).thenReturn(parserConfig()); - when(searchExecutionContext.getForField(fieldMapper.fieldType(), fielddataOperation)).thenReturn( - new BytesBinaryIndexFieldData(fieldMapper.fullPath(), CoreValuesSourceType.KEYWORD, BinaryDocValuesField::new) + IndexVersion indexVersion = IndexVersion.current(); + NamedWriteableRegistry writeableRegistry = writableRegistry(); + XContentParserConfiguration parserConfig = parserConfig(); + BytesBinaryIndexFieldData fieldData = new BytesBinaryIndexFieldData( + fieldMapper.fullPath(), + CoreValuesSourceType.KEYWORD, + BinaryDocValuesField::new ); - when(searchExecutionContext.getFieldType(Mockito.anyString())).thenAnswer(invocation -> { - final String fieldName = (String) invocation.getArguments()[0]; - return new KeywordFieldMapper.KeywordFieldType(fieldName); - }); - PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(fieldMapper.fieldType(), searchExecutionContext); + BiFunction> indexFieldDataLookup = (mft, fdc) -> fieldData; + Settings indexSettingsSettings = indexSettings(indexVersion, 1, 1).build(); + IndexSettings indexSettings = new IndexSettings( + IndexMetadata.builder("test").settings(indexSettingsSettings).build(), + Settings.EMPTY + ); + SearchExecutionContext searchExecutionContext = new SearchExecutionContext( + 0, + 0, + indexSettings, + null, + indexFieldDataLookup, + null, + MappingLookup.EMPTY, + null, + null, + parserConfig, + writeableRegistry, + null, + null, + System::currentTimeMillis, + null, + null, + () -> true, + null, + Collections.emptyMap(), + null, + MapperMetrics.NOOP, + SHARD_SEARCH_STATS + ); + + var filteredSEC = new FilteredSearchExecutionContext(searchExecutionContext) { + @Override + public MappedFieldType getFieldType(String name) { + return new KeywordFieldMapper.KeywordFieldType(name); + } + }; + + PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(fieldMapper.fieldType(), filteredSEC); try (IndexReader indexReader = DirectoryReader.open(directory)) { LeafReaderContext leafContext = indexReader.leaves().get(0); From c190831544530687372f86baf57b48dc2b256610 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 27 Feb 2026 15:24:04 +0000 Subject: [PATCH 04/12] comment --- .../org/elasticsearch/percolator/PercolateQueryBuilder.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 3a45919dd8e1e..1c35de9b0938c 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -576,7 +576,9 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy // AutoPrefilteringScope in a way that is not thread safe. For other // queries this is not a problem, but Percolate will call the returned // PercolateQuery.QueryStore function from multiple threads. - // Use a cloned SearchExecutionContext for each thread. + // The context's NestedScope is also vulnerable to concurrent modification. + // Use a cloned SearchExecutionContext for each thread with new instances of + // the mutable fields. var safeContext = context.copyForConcurrentUse(); queryBuilder = Rewriteable.rewrite(queryBuilder, safeContext); // toQuery will access localAutoPrefilteringScope From 566bdfd54ca38dd6d72b954876f53c8e3331d201 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 27 Feb 2026 16:07:08 +0000 Subject: [PATCH 05/12] another comment --- .../elasticsearch/index/query/SearchExecutionContext.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java index 3dc2ddc6fa5c6..07e4cbf9e44ed 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java @@ -90,6 +90,13 @@ * * This context is used in several components of search execution, including * building queries and fetching hits. + * + * The context is not designed to be thread-safe and is not expected to be + * shared between multiple threads. The exception is the Percolator that + * runs multiple queries simultaneously with the same context and will mutate + * elements of the context that are not threadsafe. {@link #copyForConcurrentUse()} + * addresses the Percolator use by creating a shallow copy with new instances of the + * mutable elements. */ public class SearchExecutionContext extends QueryRewriteContext { From e39e73143b5933641b9b427925c28c61d08a2b20 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Fri, 6 Mar 2026 16:40:14 +0000 Subject: [PATCH 06/12] clone FilteredSearchExecutionContext --- .../query/FilteredSearchExecutionContext.java | 25 +++++- .../FilteredSearchExecutionContextTests.java | 79 +++++++++++++++++++ .../query/SearchExecutionContextTests.java | 18 +++++ 3 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java diff --git a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java index 6d336a779843d..898ba883cf47d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java @@ -16,6 +16,7 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.similarities.Similarity; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -58,8 +59,8 @@ * * Do NOT use this if you mean to clone the context as you are planning to make modifications */ -public class FilteredSearchExecutionContext extends SearchExecutionContext { - private final SearchExecutionContext in; +public class FilteredSearchExecutionContext extends SearchExecutionContext implements Cloneable { + private SearchExecutionContext in; public FilteredSearchExecutionContext(SearchExecutionContext in) { super(in); @@ -398,10 +399,26 @@ public AutoPrefilteringScope autoPrefilteringScope() { } /** - * {@inheritDoc} + * FilteredSearchExecutionContext is often subclassed in anonymous classes + * that override one or more of the methods. Cloning is used here rather than a + * copy constructor as a copy would lose the behaviour of the overridden functions. + * FilteredSearchExecutionContext may wrap another FilteredSearchExecutionContext + * so the copy is recursive */ @Override public FilteredSearchExecutionContext copyForConcurrentUse() { - return new FilteredSearchExecutionContext(this); + try { + var clone = (FilteredSearchExecutionContext) this.clone(); + clone.in = in.copyForConcurrentUse(); + return clone; + } catch (CloneNotSupportedException e) { + // This should never happen as the class implements cloneable + throw new ElasticsearchException("failed to clone FilteredSearchExecutionContext", e); + } + } + + // for testing + SearchExecutionContext getInnerContext() { + return in; } } diff --git a/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java new file mode 100644 index 0000000000000..0cc00e945e8c4 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java @@ -0,0 +1,79 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the "Elastic License + * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side + * Public License v 1"; you may not use this file except in compliance with, at + * your election, the "Elastic License 2.0", the "GNU Affero General Public + * License v3.0 only", or the "Server Side Public License, v 1". + */ + +package org.elasticsearch.index.query; + +import org.elasticsearch.test.ESTestCase; + +import java.util.List; + +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.sameInstance; +import static org.mockito.Mockito.mock; + +public class FilteredSearchExecutionContextTests extends ESTestCase { + public void testCopyForConcurrentUse() { + SearchExecutionContext innerContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); + FilteredSearchExecutionContext filteredContext = new FilteredSearchExecutionContext(innerContext); + + FilteredSearchExecutionContext clone = filteredContext.copyForConcurrentUse(); + assertThat(filteredContext, is(not((sameInstance(clone))))); + // The copy should have a different SearchExecutionContext at it's root + assertThat(clone.getInnerContext(), is(not((sameInstance(innerContext))))); + } + + public void testCloneForConcurrentUsePreservesOverriddenMethods() { + SearchExecutionContext innerContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); + // override a method + FilteredSearchExecutionContext anonymousClass = new FilteredSearchExecutionContext(innerContext) { + @Override + public List defaultFields() { + return List.of("this is the anonymous filtered context"); + } + }; + + FilteredSearchExecutionContext clone = anonymousClass.copyForConcurrentUse(); + // test that the cloned instance preserves the anonymous class override + assertThat(clone, is(not((sameInstance(anonymousClass))))); + assertThat(clone.defaultFields(), is(List.of("this is the anonymous filtered context"))); + } + + public void testCopyForConcurrentUseWithDeeplyNestedFilteredContexts() { + SearchExecutionContext coreContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); + FilteredSearchExecutionContext innerContext = new FilteredSearchExecutionContext(coreContext) { + @Override + public List defaultFields() { + return List.of("this is the anonymous filtered context"); + } + }; + FilteredSearchExecutionContext sandwichFillingContext = new FilteredSearchExecutionContext(innerContext); + FilteredSearchExecutionContext outerContext = new FilteredSearchExecutionContext(sandwichFillingContext); + // Check outer delegates to inner + assertThat(outerContext.defaultFields(), is(List.of("this is the anonymous filtered context"))); + + FilteredSearchExecutionContext clone = outerContext.copyForConcurrentUse(); + assertThat(outerContext, is(not((sameInstance(clone))))); + assertThat(clone.defaultFields(), is(List.of("this is the anonymous filtered context"))); + + FilteredSearchExecutionContext unwrappedSandwichFillingContext = (FilteredSearchExecutionContext) clone.getInnerContext(); + assertThat(unwrappedSandwichFillingContext, is(not((sameInstance(sandwichFillingContext))))); + FilteredSearchExecutionContext unwrappedInnerContext = (FilteredSearchExecutionContext) unwrappedSandwichFillingContext + .getInnerContext(); + assertThat(unwrappedInnerContext, is(not((sameInstance(innerContext))))); + + // The copy should have a different SearchExecutionContext at the root + assertThat(unwrappedInnerContext.getInnerContext(), is(not((sameInstance(coreContext))))); + + // check modifying the original's prefiltering scope does not change the clone's + outerContext.autoPrefilteringScope().push(List.of(mock(QueryBuilder.class))); + assertThat(clone.autoPrefilteringScope().getPrefilters(), is(empty())); + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java index b0c050cbe968a..3557618dd1d1e 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java @@ -58,6 +58,7 @@ import org.elasticsearch.index.mapper.MappingParserContext; import org.elasticsearch.index.mapper.MetadataFieldMapper; import org.elasticsearch.index.mapper.MockFieldMapper; +import org.elasticsearch.index.mapper.NestedObjectMapper; import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.RootObjectMapper; @@ -99,8 +100,11 @@ import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -566,6 +570,20 @@ public void testAllowedFields() { assertThat(getFieldNames(context.getAllFields()), containsInAnyOrder("pig", "cat", "runtimecat", "runtime")); } + public void testCopyForConcurrentUse() { + SearchExecutionContext context = createSearchExecutionContext("uuid", null); + SearchExecutionContext copy = context.copyForConcurrentUse(); + assertThat(context, is(not((sameInstance(copy))))); + + assertThat(context.autoPrefilteringScope(), is(not((sameInstance(copy.autoPrefilteringScope()))))); + copy.autoPrefilteringScope().push(List.of(mock(QueryBuilder.class))); + assertThat(context.autoPrefilteringScope().getPrefilters(), empty()); + + assertThat(context.nestedScope(), is(not((sameInstance(copy.nestedScope()))))); + copy.nestedScope().nextLevel(mock(NestedObjectMapper.class)); + assertNull(context.nestedScope().getObjectMapper()); + } + private static List getFieldNames(Iterable> fields) { List fieldNames = new ArrayList<>(); for (Map.Entry field : fields) { From a93dbcf7c1bb6ddb428537e770f7cd517375586f Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 11:57:35 +0000 Subject: [PATCH 07/12] Revert "clone FilteredSearchExecutionContext" This reverts commit e39e73143b5933641b9b427925c28c61d08a2b20. --- .../query/FilteredSearchExecutionContext.java | 25 +----- .../FilteredSearchExecutionContextTests.java | 79 ------------------- .../query/SearchExecutionContextTests.java | 18 ----- 3 files changed, 4 insertions(+), 118 deletions(-) delete mode 100644 server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java diff --git a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java index 898ba883cf47d..6d336a779843d 100644 --- a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java @@ -16,7 +16,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.similarities.Similarity; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.internal.Client; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; @@ -59,8 +58,8 @@ * * Do NOT use this if you mean to clone the context as you are planning to make modifications */ -public class FilteredSearchExecutionContext extends SearchExecutionContext implements Cloneable { - private SearchExecutionContext in; +public class FilteredSearchExecutionContext extends SearchExecutionContext { + private final SearchExecutionContext in; public FilteredSearchExecutionContext(SearchExecutionContext in) { super(in); @@ -399,26 +398,10 @@ public AutoPrefilteringScope autoPrefilteringScope() { } /** - * FilteredSearchExecutionContext is often subclassed in anonymous classes - * that override one or more of the methods. Cloning is used here rather than a - * copy constructor as a copy would lose the behaviour of the overridden functions. - * FilteredSearchExecutionContext may wrap another FilteredSearchExecutionContext - * so the copy is recursive + * {@inheritDoc} */ @Override public FilteredSearchExecutionContext copyForConcurrentUse() { - try { - var clone = (FilteredSearchExecutionContext) this.clone(); - clone.in = in.copyForConcurrentUse(); - return clone; - } catch (CloneNotSupportedException e) { - // This should never happen as the class implements cloneable - throw new ElasticsearchException("failed to clone FilteredSearchExecutionContext", e); - } - } - - // for testing - SearchExecutionContext getInnerContext() { - return in; + return new FilteredSearchExecutionContext(this); } } diff --git a/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java deleted file mode 100644 index 0cc00e945e8c4..0000000000000 --- a/server/src/test/java/org/elasticsearch/index/query/FilteredSearchExecutionContextTests.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -package org.elasticsearch.index.query; - -import org.elasticsearch.test.ESTestCase; - -import java.util.List; - -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; -import static org.hamcrest.Matchers.sameInstance; -import static org.mockito.Mockito.mock; - -public class FilteredSearchExecutionContextTests extends ESTestCase { - public void testCopyForConcurrentUse() { - SearchExecutionContext innerContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); - FilteredSearchExecutionContext filteredContext = new FilteredSearchExecutionContext(innerContext); - - FilteredSearchExecutionContext clone = filteredContext.copyForConcurrentUse(); - assertThat(filteredContext, is(not((sameInstance(clone))))); - // The copy should have a different SearchExecutionContext at it's root - assertThat(clone.getInnerContext(), is(not((sameInstance(innerContext))))); - } - - public void testCloneForConcurrentUsePreservesOverriddenMethods() { - SearchExecutionContext innerContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); - // override a method - FilteredSearchExecutionContext anonymousClass = new FilteredSearchExecutionContext(innerContext) { - @Override - public List defaultFields() { - return List.of("this is the anonymous filtered context"); - } - }; - - FilteredSearchExecutionContext clone = anonymousClass.copyForConcurrentUse(); - // test that the cloned instance preserves the anonymous class override - assertThat(clone, is(not((sameInstance(anonymousClass))))); - assertThat(clone.defaultFields(), is(List.of("this is the anonymous filtered context"))); - } - - public void testCopyForConcurrentUseWithDeeplyNestedFilteredContexts() { - SearchExecutionContext coreContext = SearchExecutionContextTests.createSearchExecutionContext("uuid", null); - FilteredSearchExecutionContext innerContext = new FilteredSearchExecutionContext(coreContext) { - @Override - public List defaultFields() { - return List.of("this is the anonymous filtered context"); - } - }; - FilteredSearchExecutionContext sandwichFillingContext = new FilteredSearchExecutionContext(innerContext); - FilteredSearchExecutionContext outerContext = new FilteredSearchExecutionContext(sandwichFillingContext); - // Check outer delegates to inner - assertThat(outerContext.defaultFields(), is(List.of("this is the anonymous filtered context"))); - - FilteredSearchExecutionContext clone = outerContext.copyForConcurrentUse(); - assertThat(outerContext, is(not((sameInstance(clone))))); - assertThat(clone.defaultFields(), is(List.of("this is the anonymous filtered context"))); - - FilteredSearchExecutionContext unwrappedSandwichFillingContext = (FilteredSearchExecutionContext) clone.getInnerContext(); - assertThat(unwrappedSandwichFillingContext, is(not((sameInstance(sandwichFillingContext))))); - FilteredSearchExecutionContext unwrappedInnerContext = (FilteredSearchExecutionContext) unwrappedSandwichFillingContext - .getInnerContext(); - assertThat(unwrappedInnerContext, is(not((sameInstance(innerContext))))); - - // The copy should have a different SearchExecutionContext at the root - assertThat(unwrappedInnerContext.getInnerContext(), is(not((sameInstance(coreContext))))); - - // check modifying the original's prefiltering scope does not change the clone's - outerContext.autoPrefilteringScope().push(List.of(mock(QueryBuilder.class))); - assertThat(clone.autoPrefilteringScope().getPrefilters(), is(empty())); - } -} diff --git a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java index 3557618dd1d1e..b0c050cbe968a 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SearchExecutionContextTests.java @@ -58,7 +58,6 @@ import org.elasticsearch.index.mapper.MappingParserContext; import org.elasticsearch.index.mapper.MetadataFieldMapper; import org.elasticsearch.index.mapper.MockFieldMapper; -import org.elasticsearch.index.mapper.NestedObjectMapper; import org.elasticsearch.index.mapper.NumberFieldMapper; import org.elasticsearch.index.mapper.ObjectMapper; import org.elasticsearch.index.mapper.RootObjectMapper; @@ -100,11 +99,8 @@ import static java.util.Collections.singletonMap; import static org.hamcrest.Matchers.containsInAnyOrder; -import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.sameInstance; @@ -570,20 +566,6 @@ public void testAllowedFields() { assertThat(getFieldNames(context.getAllFields()), containsInAnyOrder("pig", "cat", "runtimecat", "runtime")); } - public void testCopyForConcurrentUse() { - SearchExecutionContext context = createSearchExecutionContext("uuid", null); - SearchExecutionContext copy = context.copyForConcurrentUse(); - assertThat(context, is(not((sameInstance(copy))))); - - assertThat(context.autoPrefilteringScope(), is(not((sameInstance(copy.autoPrefilteringScope()))))); - copy.autoPrefilteringScope().push(List.of(mock(QueryBuilder.class))); - assertThat(context.autoPrefilteringScope().getPrefilters(), empty()); - - assertThat(context.nestedScope(), is(not((sameInstance(copy.nestedScope()))))); - copy.nestedScope().nextLevel(mock(NestedObjectMapper.class)); - assertNull(context.nestedScope().getObjectMapper()); - } - private static List getFieldNames(Iterable> fields) { List fieldNames = new ArrayList<>(); for (Map.Entry field : fields) { From a060af56e467781d483a054722074c2d2fcc45b7 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 13:34:55 +0000 Subject: [PATCH 08/12] Delete FilteredSEC --- .../query/FilteredSearchExecutionContext.java | 407 ------------------ 1 file changed, 407 deletions(-) delete mode 100644 server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java diff --git a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java deleted file mode 100644 index 6d336a779843d..0000000000000 --- a/server/src/main/java/org/elasticsearch/index/query/FilteredSearchExecutionContext.java +++ /dev/null @@ -1,407 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the "Elastic License - * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side - * Public License v 1"; you may not use this file except in compliance with, at - * your election, the "Elastic License 2.0", the "GNU Affero General Public - * License v3.0 only", or the "Server Side Public License, v 1". - */ - -package org.elasticsearch.index.query; - -import org.apache.lucene.analysis.Analyzer; -import org.apache.lucene.index.IndexReader; -import org.apache.lucene.index.LeafReaderContext; -import org.apache.lucene.search.IndexSearcher; -import org.apache.lucene.search.Query; -import org.apache.lucene.search.join.BitSetProducer; -import org.apache.lucene.search.similarities.Similarity; -import org.elasticsearch.action.ActionListener; -import org.elasticsearch.client.internal.Client; -import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.core.Nullable; -import org.elasticsearch.index.Index; -import org.elasticsearch.index.IndexSettings; -import org.elasticsearch.index.IndexVersion; -import org.elasticsearch.index.analysis.IndexAnalyzers; -import org.elasticsearch.index.analysis.NamedAnalyzer; -import org.elasticsearch.index.fielddata.IndexFieldData; -import org.elasticsearch.index.mapper.DocumentParsingException; -import org.elasticsearch.index.mapper.MappedFieldType; -import org.elasticsearch.index.mapper.MappingLookup; -import org.elasticsearch.index.mapper.NestedLookup; -import org.elasticsearch.index.mapper.ParsedDocument; -import org.elasticsearch.index.mapper.SourceLoader; -import org.elasticsearch.index.mapper.SourceToParse; -import org.elasticsearch.index.query.support.AutoPrefilteringScope; -import org.elasticsearch.index.query.support.NestedScope; -import org.elasticsearch.script.Script; -import org.elasticsearch.script.ScriptContext; -import org.elasticsearch.search.NestedDocuments; -import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry; -import org.elasticsearch.search.lookup.LeafFieldLookupProvider; -import org.elasticsearch.search.lookup.SearchLookup; -import org.elasticsearch.search.lookup.SourceFilter; -import org.elasticsearch.search.lookup.SourceProvider; -import org.elasticsearch.xcontent.XContentParserConfiguration; - -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.function.BiConsumer; -import java.util.function.Function; -import java.util.function.Predicate; - -/** - * This is NOT a simple clone of the SearchExecutionContext. - * While it does "clone-esque" things, it delegates everything it can to the passed search execution context. - * - * Do NOT use this if you mean to clone the context as you are planning to make modifications - */ -public class FilteredSearchExecutionContext extends SearchExecutionContext { - private final SearchExecutionContext in; - - public FilteredSearchExecutionContext(SearchExecutionContext in) { - super(in); - this.in = in; - } - - @Override - public Similarity getSearchSimilarity() { - return in.getSearchSimilarity(); - } - - @Override - public Similarity getDefaultSimilarity() { - return in.getDefaultSimilarity(); - } - - @Override - public List defaultFields() { - return in.defaultFields(); - } - - @Override - public boolean queryStringLenient() { - return in.queryStringLenient(); - } - - @Override - public boolean queryStringAnalyzeWildcard() { - return in.queryStringAnalyzeWildcard(); - } - - @Override - public boolean queryStringAllowLeadingWildcard() { - return in.queryStringAllowLeadingWildcard(); - } - - @Override - public BitSetProducer bitsetFilter(Query filter) { - return in.bitsetFilter(filter); - } - - @Override - public > IFD getForField( - MappedFieldType fieldType, - MappedFieldType.FielddataOperation fielddataOperation - ) { - return in.getForField(fieldType, fielddataOperation); - } - - @Override - public void addNamedQuery(String name, Query query) { - in.addNamedQuery(name, query); - } - - @Override - public Map copyNamedQueries() { - return in.copyNamedQueries(); - } - - @Override - public ParsedDocument parseDocument(SourceToParse source) throws DocumentParsingException { - return in.parseDocument(source); - } - - @Override - public NestedLookup nestedLookup() { - return in.nestedLookup(); - } - - @Override - public boolean hasMappings() { - return in.hasMappings(); - } - - @Override - public boolean isFieldMapped(String name) { - return in.isFieldMapped(name); - } - - @Override - public boolean isMetadataField(String field) { - return in.isMetadataField(field); - } - - @Override - public boolean isMultiField(String field) { - return in.isMultiField(field); - } - - @Override - public Set sourcePath(String fullName) { - return in.sourcePath(fullName); - } - - @Override - public boolean isSourceEnabled() { - return in.isSourceEnabled(); - } - - @Override - public boolean isSourceSynthetic() { - return in.isSourceSynthetic(); - } - - @Override - public SourceLoader newSourceLoader(@Nullable SourceFilter filter, boolean forceSyntheticSource) { - return in.newSourceLoader(filter, forceSyntheticSource); - } - - @Override - public MappedFieldType buildAnonymousFieldType(String type) { - return in.buildAnonymousFieldType(type); - } - - @Override - public Analyzer getIndexAnalyzer(Function unindexedFieldAnalyzer) { - return in.getIndexAnalyzer(unindexedFieldAnalyzer); - } - - @Override - public void setAllowedFields(Predicate allowedFields) { - in.setAllowedFields(allowedFields); - } - - @Override - public boolean containsBrokenAnalysis(String field) { - return in.containsBrokenAnalysis(field); - } - - @Override - public SearchLookup lookup() { - return in.lookup(); - } - - @Override - public void setLookupProviders( - SourceProvider sourceProvider, - Function fieldLookupProvider - ) { - in.setLookupProviders(sourceProvider, fieldLookupProvider); - } - - @Override - public NestedScope nestedScope() { - return in.nestedScope(); - } - - @Override - public IndexVersion indexVersionCreated() { - return in.indexVersionCreated(); - } - - @Override - public boolean indexSortedOnField(String field) { - return in.indexSortedOnField(field); - } - - @Override - public ParsedQuery toQuery(QueryBuilder queryBuilder) { - return in.toQuery(queryBuilder); - } - - @Override - public Index index() { - return in.index(); - } - - @Override - public FactoryType compile(Script script, ScriptContext context) { - return in.compile(script, context); - } - - @Override - public void disableCache() { - in.disableCache(); - } - - @Override - public void registerAsyncAction(BiConsumer> asyncAction) { - in.registerAsyncAction(asyncAction); - } - - @Override - public void executeAsyncActions(ActionListener listener) { - in.executeAsyncActions(listener); - } - - @Override - public int getShardId() { - return in.getShardId(); - } - - @Override - public int getShardRequestIndex() { - return in.getShardRequestIndex(); - } - - @Override - public long nowInMillis() { - return in.nowInMillis(); - } - - @Override - public Client getClient() { - return in.getClient(); - } - - @Override - public IndexReader getIndexReader() { - return in.getIndexReader(); - } - - @Override - public IndexSearcher searcher() { - return in.searcher(); - } - - @Override - public boolean fieldExistsInIndex(String fieldname) { - return in.fieldExistsInIndex(fieldname); - } - - @Override - public MappingLookup.CacheKey mappingCacheKey() { - return in.mappingCacheKey(); - } - - @Override - public NestedDocuments getNestedDocuments() { - return in.getNestedDocuments(); - } - - @Override - public XContentParserConfiguration getParserConfig() { - return in.getParserConfig(); - } - - @Override - public CoordinatorRewriteContext convertToCoordinatorRewriteContext() { - return in.convertToCoordinatorRewriteContext(); - } - - @Override - public QueryRewriteContext convertToIndexMetadataContext() { - return in.convertToIndexMetadataContext(); - } - - @Override - public DataRewriteContext convertToDataRewriteContext() { - return in.convertToDataRewriteContext(); - } - - @Override - public MappedFieldType getFieldType(String name) { - return in.getFieldType(name); - } - - @Override - protected MappedFieldType fieldType(String name) { - return in.fieldType(name); - } - - @Override - public IndexAnalyzers getIndexAnalyzers() { - return in.getIndexAnalyzers(); - } - - @Override - MappedFieldType failIfFieldMappingNotFound(String name, MappedFieldType fieldMapping) { - return in.failIfFieldMappingNotFound(name, fieldMapping); - } - - @Override - public void setAllowUnmappedFields(boolean allowUnmappedFields) { - in.setAllowUnmappedFields(allowUnmappedFields); - } - - @Override - public void setMapUnmappedFieldAsString(boolean mapUnmappedFieldAsString) { - in.setMapUnmappedFieldAsString(mapUnmappedFieldAsString); - } - - @Override - public NamedWriteableRegistry getWriteableRegistry() { - return in.getWriteableRegistry(); - } - - @Override - public ValuesSourceRegistry getValuesSourceRegistry() { - return in.getValuesSourceRegistry(); - } - - @Override - public boolean allowExpensiveQueries() { - return in.allowExpensiveQueries(); - } - - @Override - public boolean hasAsyncActions() { - return in.hasAsyncActions(); - } - - @Override - public Index getFullyQualifiedIndex() { - return in.getFullyQualifiedIndex(); - } - - @Override - public IndexSettings getIndexSettings() { - return in.getIndexSettings(); - } - - @Override - public boolean indexMatches(String pattern) { - return in.indexMatches(pattern); - } - - @Override - public Set getMatchingFieldNames(String pattern) { - return in.getMatchingFieldNames(pattern); - } - - @Override - public void setRewriteToNamedQueries() { - in.setRewriteToNamedQueries(); - } - - @Override - public boolean rewriteToNamedQuery() { - return in.rewriteToNamedQuery(); - } - - @Override - public AutoPrefilteringScope autoPrefilteringScope() { - return in.autoPrefilteringScope(); - } - - /** - * {@inheritDoc} - */ - @Override - public FilteredSearchExecutionContext copyForConcurrentUse() { - return new FilteredSearchExecutionContext(this); - } -} From 2d17c8da4f3cb4e475b7edf6283f08b8d6930673 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 13:36:03 +0000 Subject: [PATCH 09/12] reverting --- .../index/query/SearchExecutionContext.java | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java index 07e4cbf9e44ed..07b42b667cfbd 100644 --- a/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/SearchExecutionContext.java @@ -94,9 +94,8 @@ * The context is not designed to be thread-safe and is not expected to be * shared between multiple threads. The exception is the Percolator that * runs multiple queries simultaneously with the same context and will mutate - * elements of the context that are not threadsafe. {@link #copyForConcurrentUse()} - * addresses the Percolator use by creating a shallow copy with new instances of the - * mutable elements. + * elements of the context that are not threadsafe. Percolator makes copies of + * the context before executing each query. */ public class SearchExecutionContext extends QueryRewriteContext { @@ -290,19 +289,6 @@ private void reset() { this.autoPrefilteringScope = new AutoPrefilteringScope(); } - /** - * Creates a shallow copy of the context with new instances of the fields - * that are mutable - notably {@code AutoPrefilteringScope} and {@code NestedScope}. - * The mutable elements cannot be shared safely between threads, this function - * returns a copy that is safe to use in a concurrent environment without the - * overhead of a full copy. - * - * @return The shallow copy of the context. - */ - public SearchExecutionContext copyForConcurrentUse() { - return new SearchExecutionContext(this); - } - // Set alias filter, so it can be applied for queries that need it (e.g. knn query) public void setAliasFilter(QueryBuilder aliasFilter) { this.aliasFilter = aliasFilter; From 70ea69a2b012f9947e72077db3ed2121b88482ef Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 14:14:50 +0000 Subject: [PATCH 10/12] merge the 2 wrap methods --- .../percolator/PercolateQueryBuilder.java | 78 ++++++++++++++----- .../percolator/PercolatorFieldMapper.java | 37 +-------- .../percolator/QueryBuilderStoreTests.java | 32 +++++--- .../index/query/QueryRewriteContext.java | 4 + 4 files changed, 87 insertions(+), 64 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index 1c35de9b0938c..b10a27457fc01 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -495,9 +495,7 @@ protected Analyzer getWrappedAnalyzer(String fieldName) { PercolatorFieldMapper.PercolatorFieldType pft = (PercolatorFieldMapper.PercolatorFieldType) fieldType; String queryName = this.name != null ? this.name : pft.name(); - SearchExecutionContext percolateShardContext = wrap(context); - percolateShardContext = PercolatorFieldMapper.configureContext(percolateShardContext, pft.mapUnmappedFieldsAsText); - PercolateQuery.QueryStore queryStore = createStore(pft.queryBuilderField, percolateShardContext); + PercolateQuery.QueryStore queryStore = createStore(pft.queryBuilderField, pft.mapUnmappedFieldsAsText, context); return pft.percolateQuery(queryName, queryStore, documents, docSearcher, excludeNestedDocuments, context.indexVersionCreated()); } @@ -536,7 +534,11 @@ static IndexSearcher createMultiDocumentSearcher(Analyzer analyzer, Collection

{ @@ -547,21 +549,24 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy } return docId -> { if (binaryDocValues.advanceExact(docId)) { + // create a shallow copy and set overrides + var percolateShardContext = newPercolateSearchContext(context, mapUnmappedFieldsAsText); + BytesRef qbSource = binaryDocValues.binaryValue(); QueryBuilder queryBuilder = readQueryBuilder(qbSource, registry, indexVersion, () -> { // query builder is written in an incompatible format, fall-back to reading it from source - if (context.isSourceEnabled() == false) { + if (percolateShardContext.isSourceEnabled() == false) { throw new ElasticsearchException( "Unable to read percolator query. Original transport version is incompatible and source is " + "unavailable on index [{}].", - context.index().getName() + percolateShardContext.index().getName() ); } LOGGER.warn( "Reading percolator query from source. For best performance, reindexing of index [{}] is required.", - context.index().getName() + percolateShardContext.index().getName() ); - SourceProvider sourceProvider = context.createSourceProvider(new SourceFilter(null, null)); + SourceProvider sourceProvider = percolateShardContext.createSourceProvider(new SourceFilter(null, null)); Source source = sourceProvider.getSource(ctx, docId); SourceToParse sourceToParse = new SourceToParse( String.valueOf(docId), @@ -569,7 +574,7 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy source.sourceContentType() ); - return context.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); + return percolateShardContext.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); }); // The QueryBuilder.toQuery() function modifies the search context's @@ -579,10 +584,9 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy // The context's NestedScope is also vulnerable to concurrent modification. // Use a cloned SearchExecutionContext for each thread with new instances of // the mutable fields. - var safeContext = context.copyForConcurrentUse(); - queryBuilder = Rewriteable.rewrite(queryBuilder, safeContext); + queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); // toQuery will access localAutoPrefilteringScope - return queryBuilder.toQuery(safeContext); + return queryBuilder.toQuery(percolateShardContext); } else { return null; } @@ -632,8 +636,18 @@ private static QueryBuilder readQueryBuilder( } } - private static SearchExecutionContext wrap(SearchExecutionContext delegate) { - return new SearchExecutionContext(delegate) { + /** + * Create a shallow copy of the {@code source} context with specific + * overrides for Percolator usage. The shallow copy makes the shared + * elements thread safe + * @param source + * @param mapUnmappedFieldsAsText + * @return + */ + static SearchExecutionContext newPercolateSearchContext(SearchExecutionContext source, boolean mapUnmappedFieldsAsText) { + assert source.getClass().isAnonymousClass() == false + : "source must not be an anonymous class as overridden methods " + "will be lost when a new SearchExecutionContext is created"; + var wrapped = new SearchExecutionContext(source) { @Override public IndexReader getIndexReader() { @@ -667,9 +681,9 @@ public > IFD getForField( ) { IndexFieldData.Builder builder = fieldType.fielddataBuilder( new FieldDataContext( - delegate.getFullyQualifiedIndex().getName(), - delegate.getIndexSettings(), - delegate::lookup, + source.getFullyQualifiedIndex().getName(), + source.getIndexSettings(), + source::lookup, this::sourcePath, fielddataOperation ) @@ -679,11 +693,39 @@ public > IFD getForField( return (IFD) builder.build(cache, circuitBreaker); } + // When expanding wildcard fields for term queries, we don't expand to fields that are empty. + // This is sane behavior for typical usage. But for percolator, the fields for the may not have any terms + // Consequently, we may erroneously skip expanding those term fields. + // This override allows mapped field values to expand via wildcard input, even if the field is empty in the shard. + @Override + public boolean fieldExistsInIndex(String fieldname) { + return true; + } + @Override public void addNamedQuery(String name, Query query) { - delegate.addNamedQuery(name, query); + source.addNamedQuery(name, query); } }; + + // This means that fields in the query need to exist in the mapping prior to registering this query + // The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired. + // + // Even worse when fields mentioned in percolator queries do go added to map after the queries have been registered + // then the percolator queries don't work as expected any more. + // + // Query parsing can't introduce new fields in mappings (which happens when registering a percolator query), + // because field type can't be inferred from queries (like document do) so the best option here is to disallow + // the usage of unmapped fields in percolator queries to avoid unexpected behaviour + // + // if index.percolator.map_unmapped_fields_as_string is set to true, query can contain unmapped fields which will be mapped + // as an analyzed string. + wrapped.setAllowUnmappedFields(false); + wrapped.setMapUnmappedFieldAsString(mapUnmappedFieldsAsText); + // We need to rewrite queries with name to Lucene NamedQuery to find matched sub-queries of percolator query + wrapped.setRewriteToNamedQueries(); + + return wrapped; } @Override diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java index 4d4beeb0b7dfa..1a2e16ff64c35 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java @@ -58,7 +58,6 @@ import org.elasticsearch.index.mapper.RangeType; import org.elasticsearch.index.mapper.SourceValueFetcher; import org.elasticsearch.index.mapper.ValueFetcher; -import org.elasticsearch.index.query.FilteredSearchExecutionContext; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryShardException; import org.elasticsearch.index.query.Rewriteable; @@ -406,7 +405,7 @@ public void parse(DocumentParserContext context) throws IOException { throw new IllegalArgumentException("a document can only contain one percolator query"); } - executionContext = configureContext(executionContext, isMapUnmappedFieldAsText()); + executionContext = PercolateQueryBuilder.newPercolateSearchContext(executionContext, isMapUnmappedFieldAsText()); QueryBuilder queryBuilder = parseQueryBuilder(context); // Fetching of terms, shapes and indexed scripts happen during this rewrite: @@ -508,27 +507,6 @@ void processQuery(Query query, DocumentParserContext context) { doc.add(new NumericDocValuesField(minimumShouldMatchFieldMapper.fullPath(), result.minimumShouldMatch)); } - static SearchExecutionContext configureContext(SearchExecutionContext context, boolean mapUnmappedFieldsAsString) { - SearchExecutionContext wrapped = wrapAllEmptyTextFields(context); - // This means that fields in the query need to exist in the mapping prior to registering this query - // The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired. - // - // Even worse when fields mentioned in percolator queries do go added to map after the queries have been registered - // then the percolator queries don't work as expected any more. - // - // Query parsing can't introduce new fields in mappings (which happens when registering a percolator query), - // because field type can't be inferred from queries (like document do) so the best option here is to disallow - // the usage of unmapped fields in percolator queries to avoid unexpected behaviour - // - // if index.percolator.map_unmapped_fields_as_string is set to true, query can contain unmapped fields which will be mapped - // as an analyzed string. - wrapped.setAllowUnmappedFields(false); - wrapped.setMapUnmappedFieldAsString(mapUnmappedFieldsAsString); - // We need to rewrite queries with name to Lucene NamedQuery to find matched sub-queries of percolator query - wrapped.setRewriteToNamedQueries(); - return wrapped; - } - @Override public Iterator iterator() { return Arrays.asList( @@ -573,17 +551,4 @@ static byte[] encodeRange(String rangeFieldName, byte[] minEncoded, byte[] maxEn System.arraycopy(maxEncoded, 0, bytes, BinaryRange.BYTES + offset, maxEncoded.length); return bytes; } - - // When expanding wildcard fields for term queries, we don't expand to fields that are empty. - // This is sane behavior for typical usage. But for percolator, the fields for the may not have any terms - // Consequently, we may erroneously skip expanding those term fields. - // This override allows mapped field values to expand via wildcard input, even if the field is empty in the shard. - private static SearchExecutionContext wrapAllEmptyTextFields(SearchExecutionContext searchExecutionContext) { - return new FilteredSearchExecutionContext(searchExecutionContext) { - @Override - public boolean fieldExistsInIndex(String fieldname) { - return true; - } - }; - } } diff --git a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java index c8f8e4d6965b2..dc1e72ccb629e 100644 --- a/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java +++ b/modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.CheckedFunction; +import org.elasticsearch.index.IndexMode; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.fielddata.FieldDataContext; @@ -30,13 +31,14 @@ import org.elasticsearch.index.fielddata.plain.BytesBinaryIndexFieldData; import org.elasticsearch.index.mapper.BinaryFieldMapper; import org.elasticsearch.index.mapper.DocumentParserContext; +import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.KeywordFieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.mapper.MapperBuilderContext; import org.elasticsearch.index.mapper.MapperMetrics; +import org.elasticsearch.index.mapper.Mapping; import org.elasticsearch.index.mapper.MappingLookup; import org.elasticsearch.index.mapper.TestDocumentParserContext; -import org.elasticsearch.index.query.FilteredSearchExecutionContext; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.script.field.BinaryDocValuesField; @@ -47,8 +49,11 @@ import org.elasticsearch.xcontent.XContentParserConfiguration; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.function.BiFunction; +import java.util.stream.Collectors; import static org.elasticsearch.index.query.SearchExecutionContextHelper.SHARD_SEARCH_STATS; @@ -104,6 +109,16 @@ public void testStoringQueryBuilders() throws IOException { IndexMetadata.builder("test").settings(indexSettingsSettings).build(), Settings.EMPTY ); + List fieldNames = Arrays.stream(queryBuilders).map(TermQueryBuilder::fieldName).distinct().toList(); + List keywordMappers = fieldNames.stream() + .map(name -> new KeywordFieldMapper.Builder(name, indexSettings).build(MapperBuilderContext.root(false, false))) + .collect(Collectors.toList()); + MappingLookup mappingLookup = MappingLookup.fromMappers( + Mapping.EMPTY, + keywordMappers, + Collections.emptyList(), + IndexMode.STANDARD + ); SearchExecutionContext searchExecutionContext = new SearchExecutionContext( 0, 0, @@ -111,7 +126,7 @@ public void testStoringQueryBuilders() throws IOException { null, indexFieldDataLookup, null, - MappingLookup.EMPTY, + mappingLookup, null, null, parserConfig, @@ -129,14 +144,11 @@ public void testStoringQueryBuilders() throws IOException { SHARD_SEARCH_STATS ); - var filteredSEC = new FilteredSearchExecutionContext(searchExecutionContext) { - @Override - public MappedFieldType getFieldType(String name) { - return new KeywordFieldMapper.KeywordFieldType(name); - } - }; - - PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(fieldMapper.fieldType(), filteredSEC); + PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore( + fieldMapper.fieldType(), + randomBoolean(), + searchExecutionContext + ); try (IndexReader indexReader = DirectoryReader.open(directory)) { LeafReaderContext leafContext = indexReader.leaves().get(0); diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index fb950b00f58f9..801de08f934ba 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -419,6 +419,10 @@ public void setMapUnmappedFieldAsString(boolean mapUnmappedFieldAsString) { this.mapUnmappedFieldAsString = mapUnmappedFieldAsString; } + public boolean isMapUnmappedFieldAsString() { + return mapUnmappedFieldAsString; + } + /** * Returns the CCS minimize round-trips setting. Returns null if the value of the setting is unknown. */ From 8482bd6cf5eb162378acf1d5c926f85d07cc514f Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 14:55:38 +0000 Subject: [PATCH 11/12] tidy --- .../percolator/PercolateQueryBuilder.java | 14 +++----------- .../index/query/QueryRewriteContext.java | 4 ---- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index b10a27457fc01..d4781cc9979d6 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -577,15 +577,7 @@ static PercolateQuery.QueryStore createStore( return percolateShardContext.parseDocument(sourceToParse).rootDoc().getBinaryValue(queryBuilderFieldType.name()); }); - // The QueryBuilder.toQuery() function modifies the search context's - // AutoPrefilteringScope in a way that is not thread safe. For other - // queries this is not a problem, but Percolate will call the returned - // PercolateQuery.QueryStore function from multiple threads. - // The context's NestedScope is also vulnerable to concurrent modification. - // Use a cloned SearchExecutionContext for each thread with new instances of - // the mutable fields. queryBuilder = Rewriteable.rewrite(queryBuilder, percolateShardContext); - // toQuery will access localAutoPrefilteringScope return queryBuilder.toQuery(percolateShardContext); } else { return null; @@ -640,9 +632,9 @@ private static QueryBuilder readQueryBuilder( * Create a shallow copy of the {@code source} context with specific * overrides for Percolator usage. The shallow copy makes the shared * elements thread safe - * @param source - * @param mapUnmappedFieldsAsText - * @return + * @param source The context to copy + * @param mapUnmappedFieldsAsText Controls unmapped fields behavior + * @return A copy of the source context with overrides */ static SearchExecutionContext newPercolateSearchContext(SearchExecutionContext source, boolean mapUnmappedFieldsAsText) { assert source.getClass().isAnonymousClass() == false diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index 801de08f934ba..fb950b00f58f9 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -419,10 +419,6 @@ public void setMapUnmappedFieldAsString(boolean mapUnmappedFieldAsString) { this.mapUnmappedFieldAsString = mapUnmappedFieldAsString; } - public boolean isMapUnmappedFieldAsString() { - return mapUnmappedFieldAsString; - } - /** * Returns the CCS minimize round-trips setting. Returns null if the value of the setting is unknown. */ From 4abcbbe1039e9e0883cb28d356aae54c0f15a745 Mon Sep 17 00:00:00 2001 From: David Kyle Date: Mon, 9 Mar 2026 15:08:30 +0000 Subject: [PATCH 12/12] better format --- .../org/elasticsearch/percolator/PercolateQueryBuilder.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java index d4781cc9979d6..ad5b3076d7d66 100644 --- a/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java +++ b/modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java @@ -638,7 +638,7 @@ private static QueryBuilder readQueryBuilder( */ static SearchExecutionContext newPercolateSearchContext(SearchExecutionContext source, boolean mapUnmappedFieldsAsText) { assert source.getClass().isAnonymousClass() == false - : "source must not be an anonymous class as overridden methods " + "will be lost when a new SearchExecutionContext is created"; + : "source must not be an anonymous class as overridden methods will be lost when a new SearchExecutionContext is created"; var wrapped = new SearchExecutionContext(source) { @Override