Skip to content

Commit 4e16c6c

Browse files
Kubik42Kubik42
authored
[8.19] Fixed match only text block loader not working when a keyword multi field is present (elastic#134582) (elastic#135027)
* Fixed match only text block loader not working when a keyword multi field is present (elastic#134582) * Fixed match only text block loader not working when a keyword multi field is present * Update docs/changelog/134582.yaml * Preemptively mute this test * [CI] Auto commit changes from spotless * Addressed feedback * Update modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java Co-authored-by: Jordan Powers <[email protected]> * Preemptively mute this test * Fixed copyright * Gate tests on feature presence * [CI] Auto commit changes from spotless * Revert muted-tests to main --------- Co-authored-by: elasticsearchmachine <[email protected]> Co-authored-by: Jordan Powers <[email protected]> (cherry picked from commit 86227fb) # Conflicts: # modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldMapper.java # qa/rolling-upgrade/src/javaRestTest/java/org/elasticsearch/upgrades/MatchOnlyTextRollingUpgradeIT.java # server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java * Removed BWC test since its missing way too many dependencies
1 parent 3b35617 commit 4e16c6c

File tree

8 files changed

+237
-294
lines changed

8 files changed

+237
-294
lines changed

‎docs/changelog/134582.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 134582
2+
summary: Fixed match only text block loader not working when a keyword multi field
3+
is present
4+
area: Mapping
5+
type: bug
6+
issues: []

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

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -131,27 +131,28 @@ protected Parameter<?>[] getParameters() {
131131
return new Parameter<?>[] { meta };
132132
}
133133

134-
private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
134+
private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context, MultiFields multiFields) {
135135
NamedAnalyzer searchAnalyzer = analyzers.getSearchAnalyzer();
136136
NamedAnalyzer searchQuoteAnalyzer = analyzers.getSearchQuoteAnalyzer();
137137
NamedAnalyzer indexAnalyzer = analyzers.getIndexAnalyzer();
138138
TextSearchInfo tsi = new TextSearchInfo(Defaults.FIELD_TYPE, null, searchAnalyzer, searchQuoteAnalyzer);
139-
MatchOnlyTextFieldType ft = new MatchOnlyTextFieldType(
139+
return new MatchOnlyTextFieldType(
140140
context.buildFullName(leafName()),
141141
tsi,
142142
indexAnalyzer,
143143
context.isSourceSynthetic(),
144144
meta.getValue(),
145145
withinMultiField,
146-
multiFieldsBuilder.hasSyntheticSourceCompatibleKeywordField(),
147-
storedFieldInBinaryFormat
146+
storedFieldInBinaryFormat,
147+
// match only text fields are not stored by definition
148+
TextFieldMapper.SyntheticSourceHelper.syntheticSourceDelegate(false, multiFields)
148149
);
149-
return ft;
150150
}
151151

152152
@Override
153153
public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
154-
MatchOnlyTextFieldType tft = buildFieldType(context);
154+
BuilderParams builderParams = builderParams(this, context);
155+
MatchOnlyTextFieldType tft = buildFieldType(context, builderParams.multiFields());
155156
final boolean storeSource;
156157
if (indexCreatedVersion.onOrAfter(IndexVersions.MAPPER_TEXT_MATCH_ONLY_MULTI_FIELDS_DEFAULT_NOT_STORED_8_19)) {
157158
storeSource = context.isSourceSynthetic()
@@ -162,6 +163,7 @@ public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
162163
}
163164
return new MatchOnlyTextFieldMapper(leafName(), Defaults.FIELD_TYPE, tft, builderParams(this, context), storeSource, this);
164165
}
166+
165167
}
166168

167169
private static boolean isSyntheticSourceStoredFieldInBinaryFormat(IndexVersion indexCreatedVersion) {
@@ -185,7 +187,6 @@ public static class MatchOnlyTextFieldType extends StringFieldType {
185187
private final String originalName;
186188

187189
private final boolean withinMultiField;
188-
private final boolean hasCompatibleMultiFields;
189190
private final boolean storedFieldInBinaryFormat;
190191

191192
public MatchOnlyTextFieldType(
@@ -195,15 +196,14 @@ public MatchOnlyTextFieldType(
195196
boolean isSyntheticSource,
196197
Map<String, String> meta,
197198
boolean withinMultiField,
198-
boolean hasCompatibleMultiFields,
199-
boolean storedFieldInBinaryFormat
199+
boolean storedFieldInBinaryFormat,
200+
KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate
200201
) {
201202
super(name, true, false, false, tsi, meta);
202203
this.indexAnalyzer = Objects.requireNonNull(indexAnalyzer);
203-
this.textFieldType = new TextFieldType(name, isSyntheticSource);
204-
this.originalName = isSyntheticSource ? name() + "._original" : null;
204+
this.textFieldType = new TextFieldType(name, isSyntheticSource, syntheticSourceDelegate);
205+
this.originalName = isSyntheticSource ? name + "._original" : null;
205206
this.withinMultiField = withinMultiField;
206-
this.hasCompatibleMultiFields = hasCompatibleMultiFields;
207207
this.storedFieldInBinaryFormat = storedFieldInBinaryFormat;
208208
}
209209

@@ -216,7 +216,7 @@ public MatchOnlyTextFieldType(String name) {
216216
Collections.emptyMap(),
217217
false,
218218
false,
219-
false
219+
null
220220
);
221221
}
222222

@@ -264,26 +264,23 @@ private IOFunction<LeafReaderContext, CheckedIntFunction<List<Object>, IOExcepti
264264
} else {
265265
assert false : "parent field should either be stored or have doc values";
266266
}
267-
} else if (searchExecutionContext.isSourceSynthetic() && hasCompatibleMultiFields) {
268-
var mapper = (MatchOnlyTextFieldMapper) searchExecutionContext.getMappingLookup().getMapper(name());
269-
var kwd = TextFieldMapper.SyntheticSourceHelper.getKeywordFieldMapperForSyntheticSource(mapper);
267+
} else if (searchExecutionContext.isSourceSynthetic() && textFieldType.syntheticSourceDelegate() != null) {
268+
var kwd = textFieldType.syntheticSourceDelegate();
270269

271270
if (kwd != null) {
272-
var fieldType = kwd.fieldType();
273-
274-
if (fieldType.ignoreAbove().isSet()) {
275-
if (fieldType.isStored()) {
276-
return storedFieldFetcher(fieldType.name(), fieldType.originalName());
277-
} else if (fieldType.hasDocValues()) {
278-
var ifd = searchExecutionContext.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH);
279-
return combineFieldFetchers(docValuesFieldFetcher(ifd), storedFieldFetcher(fieldType.originalName()));
271+
if (kwd.ignoreAbove().isSet()) {
272+
if (kwd.isStored()) {
273+
return storedFieldFetcher(kwd.name(), kwd.originalName());
274+
} else if (kwd.hasDocValues()) {
275+
var ifd = searchExecutionContext.getForField(kwd, MappedFieldType.FielddataOperation.SEARCH);
276+
return combineFieldFetchers(docValuesFieldFetcher(ifd), storedFieldFetcher(kwd.originalName()));
280277
}
281278
}
282279

283-
if (fieldType.isStored()) {
284-
return storedFieldFetcher(fieldType.name());
285-
} else if (fieldType.hasDocValues()) {
286-
var ifd = searchExecutionContext.getForField(fieldType, MappedFieldType.FielddataOperation.SEARCH);
280+
if (kwd.isStored()) {
281+
return storedFieldFetcher(kwd.name());
282+
} else if (kwd.hasDocValues()) {
283+
var ifd = searchExecutionContext.getForField(kwd, MappedFieldType.FielddataOperation.SEARCH);
287284
return docValuesFieldFetcher(ifd);
288285
} else {
289286
assert false : "multi field should either be stored or have doc values";
@@ -506,7 +503,7 @@ public Query phrasePrefixQuery(TokenStream stream, int slop, int maxExpansions,
506503
return toQuery(query, queryShardContext);
507504
}
508505

509-
private static class BytesFromMixedStringsBytesRefBlockLoader extends BlockStoredFieldsReader.StoredFieldsBlockLoader {
506+
static class BytesFromMixedStringsBytesRefBlockLoader extends BlockStoredFieldsReader.StoredFieldsBlockLoader {
510507
BytesFromMixedStringsBytesRefBlockLoader(String field) {
511508
super(field);
512509
}
@@ -537,12 +534,27 @@ protected BytesRef toBytesRef(Object v) {
537534
@Override
538535
public BlockLoader blockLoader(BlockLoaderContext blContext) {
539536
if (textFieldType.isSyntheticSource()) {
540-
if (storedFieldInBinaryFormat) {
541-
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(storedFieldNameForSyntheticSource());
542-
} else {
543-
return new BytesFromMixedStringsBytesRefBlockLoader(storedFieldNameForSyntheticSource());
537+
// if there is no synthetic source delegate, then this match only text field would've created StoredFields for us to use
538+
if (textFieldType.syntheticSourceDelegate() == null) {
539+
if (storedFieldInBinaryFormat) {
540+
return new BlockStoredFieldsReader.BytesFromBytesRefsBlockLoader(storedFieldNameForSyntheticSource());
541+
} else {
542+
return new BytesFromMixedStringsBytesRefBlockLoader(storedFieldNameForSyntheticSource());
543+
}
544+
}
545+
546+
// otherwise, delegate block loading to the synthetic source delegate if possible
547+
if (textFieldType.canUseSyntheticSourceDelegateForLoading()) {
548+
return new BlockLoader.Delegating(textFieldType.syntheticSourceDelegate().blockLoader(blContext)) {
549+
@Override
550+
protected String delegatingTo() {
551+
return textFieldType.syntheticSourceDelegate().name();
552+
}
553+
};
544554
}
545555
}
556+
557+
// fallback to _source (synthetic or not)
546558
SourceValueFetcher fetcher = SourceValueFetcher.toString(blContext.sourcePaths(name()));
547559
// MatchOnlyText never has norms, so we have to use the field names field
548560
BlockSourceReader.LeafIteratorLookup lookup = BlockSourceReader.lookupFromFieldNames(blContext.fieldNames(), name());

‎modules/mapper-extras/src/test/java/org/elasticsearch/index/mapper/extras/MatchOnlyTextFieldTypeTests.java‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package org.elasticsearch.index.mapper.extras;
1010

1111
import org.apache.lucene.analysis.TokenStream;
12+
import org.apache.lucene.document.FieldType;
1213
import org.apache.lucene.index.Term;
1314
import org.apache.lucene.queries.intervals.Intervals;
1415
import org.apache.lucene.queries.intervals.IntervalsSource;
@@ -27,20 +28,38 @@
2728
import org.apache.lucene.tests.analysis.Token;
2829
import org.apache.lucene.util.BytesRef;
2930
import org.elasticsearch.ElasticsearchException;
31+
import org.elasticsearch.cluster.metadata.IndexMetadata;
3032
import org.elasticsearch.common.lucene.BytesRefs;
33+
import org.elasticsearch.common.lucene.Lucene;
3134
import org.elasticsearch.common.lucene.search.AutomatonQueries;
3235
import org.elasticsearch.common.lucene.search.MultiPhrasePrefixQuery;
36+
import org.elasticsearch.common.settings.Settings;
3337
import org.elasticsearch.common.unit.Fuzziness;
38+
import org.elasticsearch.index.IndexMode;
39+
import org.elasticsearch.index.IndexSettings;
40+
import org.elasticsearch.index.IndexVersion;
41+
import org.elasticsearch.index.analysis.NamedAnalyzer;
42+
import org.elasticsearch.index.mapper.BlockLoader;
43+
import org.elasticsearch.index.mapper.FieldNamesFieldMapper;
3444
import org.elasticsearch.index.mapper.FieldTypeTestCase;
45+
import org.elasticsearch.index.mapper.KeywordFieldMapper;
3546
import org.elasticsearch.index.mapper.MappedFieldType;
47+
import org.elasticsearch.index.mapper.MappingParserContext;
48+
import org.elasticsearch.index.mapper.TextFieldMapper;
49+
import org.elasticsearch.index.mapper.TextSearchInfo;
3650
import org.elasticsearch.index.mapper.extras.MatchOnlyTextFieldMapper.MatchOnlyTextFieldType;
51+
import org.elasticsearch.script.ScriptCompiler;
3752
import org.hamcrest.Matchers;
3853

3954
import java.io.IOException;
4055
import java.util.ArrayList;
4156
import java.util.Arrays;
57+
import java.util.Collections;
4258
import java.util.List;
4359

60+
import static org.mockito.Mockito.doReturn;
61+
import static org.mockito.Mockito.mock;
62+
4463
public class MatchOnlyTextFieldTypeTests extends FieldTypeTestCase {
4564

4665
public void testTermQuery() {
@@ -205,4 +224,149 @@ public void testRangeIntervals() {
205224
((SourceIntervalsSource) rangeIntervals).getIntervalsSource()
206225
);
207226
}
227+
228+
public void test_block_loader_uses_stored_fields_for_loading_when_synthetic_source_delegate_is_absent() {
229+
// given
230+
MatchOnlyTextFieldMapper.MatchOnlyTextFieldType ft = new MatchOnlyTextFieldMapper.MatchOnlyTextFieldType(
231+
"parent",
232+
new TextSearchInfo(TextFieldMapper.Defaults.FIELD_TYPE, null, Lucene.STANDARD_ANALYZER, Lucene.STANDARD_ANALYZER),
233+
mock(NamedAnalyzer.class),
234+
true,
235+
Collections.emptyMap(),
236+
false,
237+
false,
238+
null
239+
);
240+
241+
// when
242+
BlockLoader blockLoader = ft.blockLoader(mock(MappedFieldType.BlockLoaderContext.class));
243+
244+
// then
245+
// verify that we delegate block loading to the synthetic source delegate
246+
assertThat(blockLoader, Matchers.instanceOf(MatchOnlyTextFieldType.BytesFromMixedStringsBytesRefBlockLoader.class));
247+
}
248+
249+
public void test_block_loader_uses_synthetic_source_delegate_when_ignore_above_is_not_set() {
250+
// given
251+
KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate = new KeywordFieldMapper.KeywordFieldType(
252+
"child",
253+
true,
254+
true,
255+
Collections.emptyMap()
256+
);
257+
258+
MatchOnlyTextFieldMapper.MatchOnlyTextFieldType ft = new MatchOnlyTextFieldMapper.MatchOnlyTextFieldType(
259+
"parent",
260+
new TextSearchInfo(TextFieldMapper.Defaults.FIELD_TYPE, null, Lucene.STANDARD_ANALYZER, Lucene.STANDARD_ANALYZER),
261+
mock(NamedAnalyzer.class),
262+
true,
263+
Collections.emptyMap(),
264+
false,
265+
false,
266+
syntheticSourceDelegate
267+
);
268+
269+
// when
270+
BlockLoader blockLoader = ft.blockLoader(mock(MappedFieldType.BlockLoaderContext.class));
271+
272+
// then
273+
// verify that we delegate block loading to the synthetic source delegate
274+
assertThat(blockLoader, Matchers.instanceOf(BlockLoader.Delegating.class));
275+
}
276+
277+
public void test_block_loader_does_not_use_synthetic_source_delegate_when_ignore_above_is_set() {
278+
// given
279+
Settings settings = Settings.builder()
280+
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
281+
.put(IndexSettings.MODE.getKey(), IndexMode.STANDARD)
282+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
283+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
284+
.build();
285+
IndexSettings indexSettings = new IndexSettings(IndexMetadata.builder("index").settings(settings).build(), settings);
286+
MappingParserContext mappingParserContext = mock(MappingParserContext.class);
287+
doReturn(settings).when(mappingParserContext).getSettings();
288+
doReturn(indexSettings).when(mappingParserContext).getIndexSettings();
289+
doReturn(mock(ScriptCompiler.class)).when(mappingParserContext).scriptCompiler();
290+
291+
KeywordFieldMapper.Builder builder = new KeywordFieldMapper.Builder("child", mappingParserContext);
292+
builder.ignoreAbove(123);
293+
294+
KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate = new KeywordFieldMapper.KeywordFieldType(
295+
"child",
296+
mock(FieldType.class),
297+
mock(NamedAnalyzer.class),
298+
mock(NamedAnalyzer.class),
299+
mock(NamedAnalyzer.class),
300+
builder,
301+
true
302+
);
303+
304+
MatchOnlyTextFieldMapper.MatchOnlyTextFieldType ft = new MatchOnlyTextFieldMapper.MatchOnlyTextFieldType(
305+
"parent",
306+
new TextSearchInfo(TextFieldMapper.Defaults.FIELD_TYPE, null, Lucene.STANDARD_ANALYZER, Lucene.STANDARD_ANALYZER),
307+
mock(NamedAnalyzer.class),
308+
true,
309+
Collections.emptyMap(),
310+
false,
311+
false,
312+
syntheticSourceDelegate
313+
);
314+
315+
// when
316+
MappedFieldType.BlockLoaderContext blContext = mock(MappedFieldType.BlockLoaderContext.class);
317+
doReturn(FieldNamesFieldMapper.FieldNamesFieldType.get(false)).when(blContext).fieldNames();
318+
BlockLoader blockLoader = ft.blockLoader(blContext);
319+
320+
// then
321+
// verify that we don't delegate anything
322+
assertThat(blockLoader, Matchers.not(Matchers.instanceOf(BlockLoader.Delegating.class)));
323+
}
324+
325+
public void test_block_loader_does_not_use_synthetic_source_delegate_when_ignore_above_is_set_at_index_level() {
326+
// given
327+
Settings settings = Settings.builder()
328+
.put(IndexMetadata.SETTING_VERSION_CREATED, IndexVersion.current())
329+
.put(IndexSettings.MODE.getKey(), IndexMode.STANDARD)
330+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
331+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 1)
332+
.put(IndexSettings.IGNORE_ABOVE_SETTING.getKey(), 123)
333+
.build();
334+
IndexSettings indexSettings = new IndexSettings(IndexMetadata.builder("index").settings(settings).build(), settings);
335+
MappingParserContext mappingParserContext = mock(MappingParserContext.class);
336+
doReturn(settings).when(mappingParserContext).getSettings();
337+
doReturn(indexSettings).when(mappingParserContext).getIndexSettings();
338+
doReturn(mock(ScriptCompiler.class)).when(mappingParserContext).scriptCompiler();
339+
340+
KeywordFieldMapper.Builder builder = new KeywordFieldMapper.Builder("child", mappingParserContext);
341+
342+
KeywordFieldMapper.KeywordFieldType syntheticSourceDelegate = new KeywordFieldMapper.KeywordFieldType(
343+
"child",
344+
mock(FieldType.class),
345+
mock(NamedAnalyzer.class),
346+
mock(NamedAnalyzer.class),
347+
mock(NamedAnalyzer.class),
348+
builder,
349+
true
350+
);
351+
352+
MatchOnlyTextFieldMapper.MatchOnlyTextFieldType ft = new MatchOnlyTextFieldMapper.MatchOnlyTextFieldType(
353+
"parent",
354+
new TextSearchInfo(TextFieldMapper.Defaults.FIELD_TYPE, null, Lucene.STANDARD_ANALYZER, Lucene.STANDARD_ANALYZER),
355+
mock(NamedAnalyzer.class),
356+
true,
357+
Collections.emptyMap(),
358+
false,
359+
false,
360+
syntheticSourceDelegate
361+
);
362+
363+
// when
364+
MappedFieldType.BlockLoaderContext blContext = mock(MappedFieldType.BlockLoaderContext.class);
365+
doReturn(FieldNamesFieldMapper.FieldNamesFieldType.get(false)).when(blContext).fieldNames();
366+
BlockLoader blockLoader = ft.blockLoader(blContext);
367+
368+
// then
369+
// verify that we don't delegate anything
370+
assertThat(blockLoader, Matchers.not(Matchers.instanceOf(BlockLoader.Delegating.class)));
371+
}
208372
}

‎plugins/mapper-annotated-text/src/main/java/org/elasticsearch/index/mapper/annotatedtext/AnnotatedTextFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ private AnnotatedTextFieldType buildFieldType(FieldType fieldType, MapperBuilder
138138
store.getValue(),
139139
tsi,
140140
context.isSourceSynthetic(),
141-
TextFieldMapper.SyntheticSourceHelper.syntheticSourceDelegate(fieldType, multiFields),
141+
TextFieldMapper.SyntheticSourceHelper.syntheticSourceDelegate(fieldType.stored(), multiFields),
142142
meta.getValue()
143143
);
144144
}

0 commit comments

Comments
 (0)