Skip to content

Commit 9cb1c51

Browse files
committed
Fix match_only_text bugs if defined as multi-field
Bugs starting to occur when elastic#129126 was merged. Closes elastic#129737
1 parent ff65fd1 commit 9cb1c51

File tree

3 files changed

+112
-22
lines changed

3 files changed

+112
-22
lines changed

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java

Lines changed: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.elasticsearch.index.mapper.BlockStoredFieldsReader;
4848
import org.elasticsearch.index.mapper.DocumentParserContext;
4949
import org.elasticsearch.index.mapper.FieldMapper;
50+
import org.elasticsearch.index.mapper.KeywordFieldMapper;
5051
import org.elasticsearch.index.mapper.MapperBuilderContext;
5152
import org.elasticsearch.index.mapper.SourceValueFetcher;
5253
import org.elasticsearch.index.mapper.StringFieldType;
@@ -123,7 +124,7 @@ protected Parameter<?>[] getParameters() {
123124
return new Parameter<?>[] { meta };
124125
}
125126

126-
private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
127+
private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context, boolean storeSource) {
127128
NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer();
128129
NamedAnalyzer searchQuoteAnalyzer = analyzers.getSearchQuoteAnalyzer();
129130
NamedAnalyzer indexAnalyzer = analyzers.getIndexAnalyzer();
@@ -132,15 +133,14 @@ private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
132133
context.buildFullName(leafName()),
133134
tsi,
134135
indexAnalyzer,
135-
context.isSourceSynthetic(),
136+
storeSource,
136137
meta.getValue()
137138
);
138139
return ft;
139140
}
140141

141142
@Override
142143
public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
143-
MatchOnlyTextFieldType tft = buildFieldType(context);
144144
final boolean storeSource;
145145
if (multiFieldsNotStoredByDefaultIndexVersionCheck(indexCreatedVersion)) {
146146
storeSource = context.isSourceSynthetic()
@@ -149,6 +149,7 @@ public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
149149
} else {
150150
storeSource = context.isSourceSynthetic();
151151
}
152+
MatchOnlyTextFieldType tft = buildFieldType(context, storeSource);
152153
return new MatchOnlyTextFieldMapper(leafName(), Defaults.FIELD_TYPE, tft, builderParams(this, context), storeSource, this);
153154
}
154155
}
@@ -211,14 +212,18 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
211212
}
212213
if (searchExecutionContext.isSourceSynthetic()) {
213214
String name = storedFieldNameForSyntheticSource();
214-
StoredFieldLoader loader = StoredFieldLoader.create(false, Set.of(name));
215-
return context -> {
216-
LeafStoredFieldLoader leafLoader = loader.getLoader(context, null);
217-
return docId -> {
218-
leafLoader.advanceTo(docId);
219-
return leafLoader.storedFields().get(name);
215+
// TODO: go the parent field and load either via stored fields or doc values the values instead synthesizing complete source
216+
// (in case of synthetic source and if this field is a multi field, then it will not have a stored field.)
217+
if (name != null) {
218+
StoredFieldLoader loader = StoredFieldLoader.create(false, Set.of(name));
219+
return context -> {
220+
LeafStoredFieldLoader leafLoader = loader.getLoader(context, null);
221+
return docId -> {
222+
leafLoader.advanceTo(docId);
223+
return leafLoader.storedFields().get(name);
224+
};
220225
};
221-
};
226+
}
222227
}
223228
return context -> {
224229
ValueFetcher valueFetcher = valueFetcher(searchExecutionContext, null);
@@ -506,18 +511,30 @@ public MatchOnlyTextFieldType fieldType() {
506511

507512
@Override
508513
protected SyntheticSourceSupport syntheticSourceSupport() {
509-
return new SyntheticSourceSupport.Native(
510-
() -> new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), fieldType().name(), leafName()) {
511-
@Override
512-
protected void write(XContentBuilder b, Object value) throws IOException {
513-
if (value instanceof BytesRef valueBytes) {
514-
b.value(valueBytes.utf8ToString());
515-
} else {
516-
assert value instanceof String;
517-
b.value(value.toString());
514+
if (storeSource) {
515+
return new SyntheticSourceSupport.Native(
516+
() -> new StringStoredFieldFieldLoader(fieldType().storedFieldNameForSyntheticSource(), fieldType().name(), leafName()) {
517+
@Override
518+
protected void write(XContentBuilder b, Object value) throws IOException {
519+
if (value instanceof BytesRef valueBytes) {
520+
b.value(valueBytes.utf8ToString());
521+
} else {
522+
assert value instanceof String;
523+
b.value(value.toString());
524+
}
525+
}
526+
}
527+
);
528+
} else {
529+
for (FieldMapper multiField : multiFields()) {
530+
if (multiField instanceof KeywordFieldMapper kwd) {
531+
if (kwd.hasNormalizer() == false && (kwd.fieldType().hasDocValues() || kwd.fieldType().isStored())) {
532+
return kwd.syntheticSourceSupport();
518533
}
519534
}
520535
}
521-
);
536+
assert false : "there should be a suite field mapper with native synthetic source support";
537+
return null;
538+
}
522539
}
523540
}

modules/mapper-extras/src/yamlRestTest/resources/rest-api-spec/test/match_only_text/10_basic.yml

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,4 +394,77 @@ synthetic_source with copy_to:
394394
- match:
395395
hits.hits.0.fields.copy.0: "Apache Lucene powers Elasticsearch"
396396

397+
---
398+
synthetic_source match_only_text as multi-field:
399+
- do:
400+
indices.create:
401+
index: synthetic_source_test
402+
body:
403+
settings:
404+
index:
405+
mapping.source.mode: synthetic
406+
mappings:
407+
properties:
408+
foo:
409+
type: keyword
410+
fields:
411+
text:
412+
type: match_only_text
413+
414+
- do:
415+
index:
416+
index: synthetic_source_test
417+
id: "1"
418+
refresh: true
419+
body:
420+
foo: "Apache Lucene powers Elasticsearch"
421+
422+
- do:
423+
search:
424+
index: synthetic_source_test
425+
body:
426+
query:
427+
match_phrase:
428+
foo.text: apache lucene
429+
430+
- match: { "hits.total.value": 1 }
431+
- match:
432+
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"
433+
434+
---
435+
synthetic_source match_only_text with multi-field:
436+
- do:
437+
indices.create:
438+
index: synthetic_source_test
439+
body:
440+
settings:
441+
index:
442+
mapping.source.mode: synthetic
443+
mappings:
444+
properties:
445+
foo:
446+
type: match_only_text
447+
fields:
448+
raw:
449+
type: keyword
450+
451+
- do:
452+
index:
453+
index: synthetic_source_test
454+
id: "1"
455+
refresh: true
456+
body:
457+
foo: "Apache Lucene powers Elasticsearch"
458+
459+
- do:
460+
search:
461+
index: synthetic_source_test
462+
body:
463+
query:
464+
match_phrase:
465+
foo: apache lucene
466+
467+
- match: { "hits.total.value": 1 }
468+
- match:
469+
hits.hits.0._source.foo: "Apache Lucene powers Elasticsearch"
397470

server/src/main/java/org/elasticsearch/index/mapper/KeywordFieldMapper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,7 +1261,7 @@ public void doValidate(MappingLookup lookup) {
12611261
}
12621262
}
12631263

1264-
boolean hasNormalizer() {
1264+
public boolean hasNormalizer() {
12651265
return normalizerName != null;
12661266
}
12671267

@@ -1275,7 +1275,7 @@ private String originalName() {
12751275
}
12761276

12771277
@Override
1278-
protected SyntheticSourceSupport syntheticSourceSupport() {
1278+
public SyntheticSourceSupport syntheticSourceSupport() {
12791279
if (hasNormalizer()) {
12801280
// NOTE: no matter if we have doc values or not we use fallback synthetic source
12811281
// to store the original value whose doc values would be altered by the normalizer

0 commit comments

Comments
 (0)