Skip to content

Fix match_only_text for keyword multi-fields with ignore_above #131314

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 15, 2025
Merged

Fix match_only_text for keyword multi-fields with ignore_above #131314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 18 additions & 4 deletions ...-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.elasticsearch.index.mapper.BlockStoredFieldsReader;
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.SourceValueFetcher;
Expand All @@ -66,6 +67,7 @@
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -252,6 +254,10 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
String parentField = searchExecutionContext.parentPath(name());
var parent = searchExecutionContext.lookup().fieldType(parentField);
if (parent.isStored()) {
if (parent instanceof KeywordFieldMapper.KeywordFieldType keywordParent
&& keywordParent.ignoreAbove() != Integer.MAX_VALUE) {
return storedFieldFetcher(parentField, keywordParent.originalName());
}
return storedFieldFetcher(parentField);
} else if (parent.hasDocValues()) {
var ifd = searchExecutionContext.getForField(parent, MappedFieldType.FielddataOperation.SEARCH);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like there can be a similar problem here because doc values won't include ignored values? Not sure if that matters though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot you're right. If the keyword field is store: false, doc_values: true, we don't error, but we completely omit the value from the match_only_text field results.

I'll work on a follow-up PR to address this.

Expand All @@ -265,7 +271,11 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
if (kwd != null) {
var fieldType = kwd.fieldType();
if (fieldType.isStored()) {
return storedFieldFetcher(fieldType.name());
if (fieldType.ignoreAbove() != Integer.MAX_VALUE) {
return storedFieldFetcher(fieldType.name(), fieldType.originalName());
} else {
return storedFieldFetcher(fieldType.name());
}
} else if (fieldType.hasDocValues()) {
var ifd = searchExecutionContext.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH);
return docValuesFieldFetcher(ifd);
Expand Down Expand Up @@ -312,13 +322,17 @@ private static IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IO
};
}

private static IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> storedFieldFetcher(String name) {
var loader = StoredFieldLoader.create(false, Set.of(name));
private static IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOException>> storedFieldFetcher(String... names) {
var loader = StoredFieldLoader.create(false, Set.of(names));
return context -> {
var leafLoader = loader.getLoader(context, null);
return docId -> {
leafLoader.advanceTo(docId);
return leafLoader.storedFields().get(name);
var storedFields = leafLoader.storedFields();
if (names.length == 1) {
return storedFields.get(names[0]);
}
return Arrays.stream(names).map(storedFields::get).filter(Objects::nonNull).flatMap(List::stream).toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: streams are not very efficient, it's better to avoid them for code that runs per document.

};
};
}
Expand Down
87 changes: 87 additions & 0 deletions .../mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/match_only_text/10_basic.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,49 @@ synthetic_source match_only_text as multi-field with stored keyword as parent:
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"

---
synthetic_source match_only_text as multi-field with ignored stored keyword as parent:
- requires:
cluster_features: [ "mapper.source.mode_from_index_setting" ]
reason: "Source mode configured through index setting"

- do:
indices.create:
index: synthetic_source_test
body:
settings:
index:
mapping.source.mode: synthetic
mappings:
properties:
foo:
type: keyword
store: true
doc_values: false
ignore_above: 10
fields:
text:
type: match_only_text

- do:
index:
index: synthetic_source_test
id: "1"
refresh: true
body:
foo: "Apache Lucene powers Elasticsearch"

- do:
search:
index: synthetic_source_test
body:
query:
match_phrase:
foo.text: apache lucene

- match: { "hits.total.value": 1 }
- match:
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"
---
synthetic_source match_only_text with multi-field:
- requires:
cluster_features: [ "mapper.source.mode_from_index_setting" ]
Expand Down Expand Up @@ -561,3 +604,47 @@ synthetic_source match_only_text with stored multi-field:
- match: { "hits.total.value": 1 }
- match:
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"

---
synthetic_source match_only_text with ignored stored multi-field:
- requires:
cluster_features: [ "mapper.source.mode_from_index_setting" ]
reason: "Source mode configured through index setting"

- do:
indices.create:
index: synthetic_source_test
body:
settings:
index:
mapping.source.mode: synthetic
mappings:
properties:
foo:
type: match_only_text
fields:
raw:
type: keyword
store: true
doc_values: false
ignore_above: 10

- do:
index:
index: synthetic_source_test
id: "1"
refresh: true
body:
foo: "Apache Lucene powers Elasticsearch"

- do:
search:
index: synthetic_source_test
body:
query:
match_phrase:
foo: apache lucene

- match: { "hits.total.value": 1 }
- match:
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"
29 changes: 17 additions & 12 deletions server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ public static final class KeywordFieldType extends StringFieldType {
private final IndexMode indexMode;
private final IndexSortConfig indexSortConfig;
private final boolean hasDocValuesSkipper;
private final String originalName;

public KeywordFieldType(
String name,
Expand Down Expand Up @@ -541,6 +542,7 @@ public KeywordFieldType(
this.indexMode = builder.indexMode;
this.indexSortConfig = builder.indexSortConfig;
this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false;
this.originalName = isSyntheticSource ? name + "._original" : null;
}

public KeywordFieldType(String name, boolean isIndexed, boolean hasDocValues, Map<String, String> meta) {
Expand All @@ -555,6 +557,7 @@ public KeywordFieldType(String name, boolean isIndexed, boolean hasDocValues, Ma
this.indexMode = IndexMode.STANDARD;
this.indexSortConfig = null;
this.hasDocValuesSkipper = false;
this.originalName = null;
}

public KeywordFieldType(String name) {
Expand All @@ -580,6 +583,7 @@ public KeywordFieldType(String name, FieldType fieldType) {
this.indexMode = IndexMode.STANDARD;
this.indexSortConfig = null;
this.hasDocValuesSkipper = DocValuesSkipIndexType.NONE.equals(fieldType.docValuesSkipIndexType()) == false;
this.originalName = null;
}

public KeywordFieldType(String name, NamedAnalyzer analyzer) {
Expand All @@ -594,6 +598,7 @@ public KeywordFieldType(String name, NamedAnalyzer analyzer) {
this.indexMode = IndexMode.STANDARD;
this.indexSortConfig = null;
this.hasDocValuesSkipper = false;
this.originalName = null;
}

@Override
Expand Down Expand Up @@ -1057,6 +1062,15 @@ public Query automatonQuery(
) {
return new AutomatonQueryWithDescription(new Term(name()), automatonSupplier.get(), description);
}

/**
* The name used to store "original" that have been ignored
* by {@link KeywordFieldType#ignoreAbove()} so that they can be rebuilt
* for synthetic source.
*/
public String originalName() {
return originalName;
}
}

private final boolean indexed;
Expand Down Expand Up @@ -1109,7 +1123,7 @@ private KeywordFieldMapper(
this.useDocValuesSkipper = useDocValuesSkipper;
this.offsetsFieldName = offsetsFieldName;
this.indexSourceKeepMode = indexSourceKeepMode;
this.originalName = isSyntheticSource ? fullPath() + "._original" : null;
this.originalName = mappedFieldType.originalName();
}

@Override
Expand Down Expand Up @@ -1169,7 +1183,7 @@ private boolean indexValue(DocumentParserContext context, XContentString value)
// Save a copy of the field so synthetic source can load it
var utfBytes = value.bytes();
var bytesRef = new BytesRef(utfBytes.bytes(), utfBytes.offset(), utfBytes.length());
context.doc().add(new StoredField(originalName(), bytesRef));
context.doc().add(new StoredField(originalName, bytesRef));
}
return false;
}
Expand Down Expand Up @@ -1280,15 +1294,6 @@ boolean hasNormalizer() {
return normalizerName != null;
}

/**
* The name used to store "original" that have been ignored
* by {@link KeywordFieldType#ignoreAbove()} so that they can be rebuilt
* for synthetic source.
*/
private String originalName() {
return originalName;
}

@Override
protected SyntheticSourceSupport syntheticSourceSupport() {
if (hasNormalizer()) {
Expand Down Expand Up @@ -1337,7 +1342,7 @@ protected BytesRef preserve(BytesRef value) {
}

if (fieldType().ignoreAbove != Integer.MAX_VALUE) {
layers.add(new CompositeSyntheticFieldLoader.StoredFieldLayer(originalName()) {
layers.add(new CompositeSyntheticFieldLoader.StoredFieldLayer(originalName) {
@Override
protected void writeValue(Object value, XContentBuilder b) throws IOException {
BytesRef ref = (BytesRef) value;
Expand Down