Skip to content
Open
Show file tree
Hide file tree
Changes from 10 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
6 changes: 6 additions & 0 deletions docs/changelog/132987.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 132987
summary: Rewrite more queries to `match_none` when they target an unmapped field
area: Search
type: enhancement
issues:
- 97129
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,9 @@ public void parse(DocumentParserContext context) throws IOException {
IndexVersion indexVersion = context.indexSettings().getIndexVersionCreated();
createQueryBuilderField(indexVersion, clusterTransportVersion.get(), queryBuilderField, queryBuilder, context);

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.

QueryBuilder queryBuilderForProcessing = queryBuilder.rewrite(copy);
Query query = queryBuilderForProcessing.toQuery(executionContext);
processQuery(query, context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ public void testTermQueryBigInt() throws Exception {

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.


int defaultMaxRegexLength = IndexSettings.MAX_REGEX_LENGTH_SETTING.get(Settings.EMPTY);
StringBuilder regexp = new StringBuilder(defaultMaxRegexLength);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,15 @@ protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws
});
return newShapeQueryBuilder(this.fieldName, supplier::get, this.indexedShapeId).relation(relation);
}
return super.doRewrite(queryRewriteContext);
}

@Override
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;
}
Comment on lines +467 to 473
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).


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.XContentBuilder;

Expand Down Expand Up @@ -161,4 +162,17 @@ protected int doHashCode() {
protected boolean doEquals(QB other) {
return Objects.equals(fieldName, other.fieldName) && Objects.equals(value, other.value);
}

@Override
protected final QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
}
return doIndexMetadataRewrite(context, fieldType);
}

protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context, MappedFieldType fieldType) throws IOException {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -253,15 +253,12 @@ public String getWriteableName() {
}

@Override
protected QueryBuilder doRewrite(QueryRewriteContext queryRewriteContext) throws IOException {
SearchExecutionContext context = queryRewriteContext.convertToSearchExecutionContext();
if (context != null) {
MappedFieldType fieldType = context.getFieldType(fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
}
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 super.doRewrite(context);
return this;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,4 +384,13 @@ public String getWriteableName() {
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ZERO;
}

@Override
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -387,4 +387,13 @@ public String getWriteableName() {
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ZERO;
}

@Override
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -321,4 +321,13 @@ public String getWriteableName() {
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ZERO;
}

@Override
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.search.MatchQueryParser;
import org.elasticsearch.xcontent.ParseField;
import org.elasticsearch.xcontent.XContentBuilder;
Expand Down Expand Up @@ -288,4 +289,13 @@ public static MatchPhrasePrefixQueryBuilder fromXContent(XContentParser parser)
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ZERO;
}

@Override
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep

@Override
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
}
// If we're using the default keyword analyzer then we can rewrite this to a TermQueryBuilder
// and possibly shortcut
// If we're using a keyword analyzer then we can rewrite this to a TermQueryBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,10 @@ public void doXContent(XContentBuilder builder, Params params) throws IOExceptio

@Override
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) throws IOException {
MappedFieldType fieldType = context.getFieldType(fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
}
if (fuzziness != null || lenient) {
// Term queries can be neither fuzzy nor lenient, so don't rewrite under these conditions
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,15 @@ protected MappedFieldType.Relation getRelation(final SearchExecutionContext sear
);
}

@Override
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;
}

@Override
protected QueryBuilder doCoordinatorRewrite(final CoordinatorRewriteContext coordinatorRewriteContext) {
return toQueryBuilder(getRelation(coordinatorRewriteContext));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -323,4 +323,13 @@ protected boolean doEquals(RegexpQueryBuilder other) {
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.ZERO;
}

@Override
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,7 @@ protected void addExtraXContent(XContentBuilder builder, Params params) throws I
}

@Override
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) {
MappedFieldType fieldType = context.getFieldType(this.fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query is against a field that does not exist");
}
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context, MappedFieldType fieldType) throws IOException {
return maybeRewriteBasedOnConstantFields(fieldType, context);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,15 @@ public static TermsSetQueryBuilder fromXContent(XContentParser parser) throws IO
return queryBuilder;
}

@Override
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;
}

@Override
protected Query doToQuery(SearchExecutionContext context) {
if (values.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
/**
* A {@link SpanQuery} that matches no documents.
*/
// TODO is this still needed?
public class SpanMatchNoDocsQuery extends SpanQuery {
private final String field;
private final String reason;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@

package org.elasticsearch.index.query;

import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.RegexpQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.core.Strings;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.test.AbstractQueryTestCase;

import java.io.IOException;
Expand Down Expand Up @@ -72,11 +74,15 @@ private static RegexpQueryBuilder randomRegexpQuery() {

@Override
protected void doAssertLuceneQuery(RegexpQueryBuilder queryBuilder, Query query, SearchExecutionContext context) throws IOException {
assertThat(query, instanceOf(RegexpQuery.class));
RegexpQuery regexpQuery = (RegexpQuery) query;

String expectedFieldName = expectedFieldName(queryBuilder.fieldName());
assertThat(regexpQuery.getField(), equalTo(expectedFieldName));
MappedFieldType fieldType = context.getFieldType(queryBuilder.fieldName());
if (fieldType == null) {
assertThat(query, instanceOf(MatchNoDocsQuery.class));
} else {
assertThat(query, instanceOf(RegexpQuery.class));
RegexpQuery regexpQuery = (RegexpQuery) query;
String expectedFieldName = expectedFieldName(queryBuilder.fieldName());
assertThat(regexpQuery.getField(), equalTo(expectedFieldName));
}
}

public void testIllegalArguments() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@
import org.apache.lucene.index.Term;
import org.apache.lucene.queries.spans.SpanTermQuery;
import org.apache.lucene.search.BoostQuery;
import org.apache.lucene.search.MatchNoDocsQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.TermQuery;
import org.elasticsearch.common.ParsingException;
import org.elasticsearch.index.mapper.IdFieldMapper;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.lucene.queries.SpanMatchNoDocsQuery;
import org.elasticsearch.xcontent.json.JsonStringEncoder;

import java.io.IOException;
Expand Down Expand Up @@ -58,7 +58,7 @@ protected void doAssertLuceneQuery(SpanTermQueryBuilder queryBuilder, Query quer
Term term = ((TermQuery) mapper.termQuery(queryBuilder.value(), null)).getTerm();
assertThat(spanTermQuery.getTerm(), equalTo(term));
} else {
assertThat(query, instanceOf(SpanMatchNoDocsQuery.class));
assertThat(query, instanceOf(MatchNoDocsQuery.class));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
import org.elasticsearch.index.mapper.GeoShapeQueryable;
import org.elasticsearch.index.mapper.MappedFieldType;
import org.elasticsearch.index.query.AbstractQueryBuilder;
import org.elasticsearch.index.query.MatchNoneQueryBuilder;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryRewriteContext;
import org.elasticsearch.index.query.QueryShardException;
import org.elasticsearch.index.query.SearchExecutionContext;
import org.elasticsearch.search.aggregations.bucket.geogrid.GeoTileUtils;
Expand Down Expand Up @@ -402,4 +405,13 @@ public String getWriteableName() {
public TransportVersion getMinimalSupportedVersion() {
return TransportVersions.V_8_3_0;
}

@Override
protected QueryBuilder doIndexMetadataRewrite(QueryRewriteContext context) {
MappedFieldType fieldType = context.getFieldType(fieldName);
if (fieldType == null) {
return new MatchNoneQueryBuilder("The \"" + getName() + "\" query was rewritten to a \"match_none\" query.");
}
return this;
}
}