Skip to content

Conversation

@javanna
Copy link
Member

@javanna javanna commented Aug 15, 2025

We have a specific metadata rewrite that allows us to optionally rewrite a query builder based on mappings. It is used in some places to handle unmapped fields, but not consistently. This PR adds the rewrite to match_none to more queries when they target an unmapped field.

Closes #97129
Closes #131037

@javanna javanna force-pushed the enhancement/unmapped_fields_rewrite branch from dfd6605 to 51b6f9f Compare August 18, 2025 07:39
@javanna javanna added :Search Relevance/Search Catch all for Search Relevance >enhancement labels Aug 18, 2025
@javanna javanna marked this pull request as ready for review August 18, 2025 18:15
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Aug 18, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@elasticsearchmachine
Copy link
Collaborator

Hi @javanna, I've created a changelog YAML for you.


QueryBuilder queryBuilderForProcessing = queryBuilder.rewrite(new SearchExecutionContext(executionContext));
SearchExecutionContext copy = new SearchExecutionContext(executionContext);
configureContext(copy, isMapUnmappedFieldAsText());
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configuring the context was missing, hence the configuration values were not available to the underlying query rewrite. This fixes it.

public void testTooLongRegexInRegexpQuery() throws Exception {
createIndex("idx");
indexRandom(true, prepareIndex("idx").setSource("{}", XContentType.JSON));
indexRandom(true, prepareIndex("idx").setSource("num", "value"));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no more failure when targeting an unmapped field, the test needs fixing to target a mapped field.

Copy link
Contributor

@john-wagster john-wagster left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super familiar with this rewrite logic but I did some spot checking and looked through the original ask and PR; this lgtm

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What makes this confusing is that we have a default index.query.parse.allow_unmapped_fields which defaults to true, and then individual queries have ignore_unmapped (are all of them deprecated?).

With this change, I think ignore_unmapped: false is broken for every where that accepts an ignore_unmapped.

@elasticsearchmachine
Copy link
Collaborator

Hi @javanna, I've updated the changelog YAML for you.

Comment on lines +467 to 473
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
}
return this;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignoreUnmapped defined in the query should take precedence right? It has a default value of false for this query type.

There are a bunch of these (HasChildQueryBuilder, HasParentQueryBuilder, NestedQueryBuilder, GeoGridQueryBuilder,ParentIdQueryBuilder, and maybe more..but these are the ones I found quickly).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Search Relevance/Search Catch all for Search Relevance Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.3.0

Projects

None yet

4 participants