Skip to content

Commit 6ae7049

Browse files
committed
Avoid storing multi fields as type text and match_only_text.
Don't store text and match_only_text field by default when source mode is synthetic and a field is a multi field or when there is a suitable multi field. Without this change, ES would store field otherwise twice in a multi-field configuration. For example: ``` ... "os": { "properties": { "name": { "ignore_above": 1024, "type": "keyword", "fields": { "text": { "type": "match_only_text" } } } ... ``` In this case, two stored fields were added, one in case for the `name` field and one for `name.text` multi-field. This change prevents this, and would never store a stored field when text or match_only_text field is a multi-field.
1 parent 53f3ab2 commit 6ae7049

File tree

5 files changed

+167
-9
lines changed

5 files changed

+167
-9
lines changed

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,10 @@ private MatchOnlyTextFieldType buildFieldType(MapperBuilderContext context) {
140140
@Override
141141
public MatchOnlyTextFieldMapper build(MapperBuilderContext context) {
142142
MatchOnlyTextFieldType tft = buildFieldType(context);
143-
return new MatchOnlyTextFieldMapper(
144-
leafName(),
145-
Defaults.FIELD_TYPE,
146-
tft,
147-
builderParams(this, context),
148-
context.isSourceSynthetic(),
149-
this
150-
);
143+
boolean storeSource = context.isSourceSynthetic()
144+
&& currentFieldIsAMultiField == false
145+
&& multiFieldsBuilder.hasSyntheticSourceCompatibleKeywordField() == false;
146+
return new MatchOnlyTextFieldMapper(leafName(), Defaults.FIELD_TYPE, tft, builderParams(this, context), storeSource, this);
151147
}
152148
}
153149

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.lucene.tests.index.RandomIndexWriter;
2424
import org.elasticsearch.common.Strings;
2525
import org.elasticsearch.core.Tuple;
26+
import org.elasticsearch.index.IndexSettings;
2627
import org.elasticsearch.index.mapper.DocumentMapper;
2728
import org.elasticsearch.index.mapper.KeywordFieldMapper;
2829
import org.elasticsearch.index.mapper.LuceneDocument;
@@ -46,8 +47,10 @@
4647
import java.util.stream.Collectors;
4748

4849
import static org.hamcrest.Matchers.containsString;
50+
import static org.hamcrest.Matchers.empty;
4951
import static org.hamcrest.Matchers.equalTo;
5052
import static org.hamcrest.Matchers.instanceOf;
53+
import static org.hamcrest.core.Is.is;
5154

5255
public class MatchOnlyTextFieldMapperTests extends MapperTestCase {
5356

@@ -255,4 +258,91 @@ public void testDocValuesLoadedFromSynthetic() throws IOException {
255258
protected IngestScriptSupport ingestScriptSupport() {
256259
throw new AssumptionViolatedException("not supported");
257260
}
261+
262+
public void testStoreParameterDefaultsSyntheticSource() throws IOException {
263+
var indexSettingsBuilder = getIndexSettingsBuilder();
264+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
265+
var indexSettings = indexSettingsBuilder.build();
266+
267+
var mapping = mapping(b -> {
268+
b.startObject("name");
269+
b.field("type", "match_only_text");
270+
b.endObject();
271+
});
272+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
273+
274+
var source = source(b -> b.field("name", "quick brown fox"));
275+
ParsedDocument doc = mapper.parse(source);
276+
277+
{
278+
List<IndexableField> fields = doc.rootDoc().getFields("name");
279+
IndexableFieldType fieldType = fields.get(0).fieldType();
280+
assertThat(fieldType.stored(), is(false));
281+
}
282+
{
283+
List<IndexableField> fields = doc.rootDoc().getFields("name._original");
284+
IndexableFieldType fieldType = fields.get(0).fieldType();
285+
assertThat(fieldType.stored(), is(true));
286+
}
287+
}
288+
289+
public void testStoreParameterDefaultsSyntheticSourceWithKeywordMultiField() throws IOException {
290+
var indexSettingsBuilder = getIndexSettingsBuilder();
291+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
292+
var indexSettings = indexSettingsBuilder.build();
293+
294+
var mapping = mapping(b -> {
295+
b.startObject("name");
296+
b.field("type", "match_only_text");
297+
b.startObject("fields");
298+
b.startObject("keyword");
299+
b.field("type", "keyword");
300+
b.endObject();
301+
b.endObject();
302+
b.endObject();
303+
});
304+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
305+
306+
var source = source(b -> b.field("name", "quick brown fox"));
307+
ParsedDocument doc = mapper.parse(source);
308+
{
309+
List<IndexableField> fields = doc.rootDoc().getFields("name");
310+
IndexableFieldType fieldType = fields.get(0).fieldType();
311+
assertThat(fieldType.stored(), is(false));
312+
}
313+
{
314+
List<IndexableField> fields = doc.rootDoc().getFields("name._original");
315+
assertThat(fields, empty());
316+
}
317+
}
318+
319+
public void testStoreParameterDefaultsSyntheticSourceTextFieldIsMultiField() throws IOException {
320+
var indexSettingsBuilder = getIndexSettingsBuilder();
321+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
322+
var indexSettings = indexSettingsBuilder.build();
323+
324+
var mapping = mapping(b -> {
325+
b.startObject("name");
326+
b.field("type", "keyword");
327+
b.startObject("fields");
328+
b.startObject("text");
329+
b.field("type", "match_only_text");
330+
b.endObject();
331+
b.endObject();
332+
b.endObject();
333+
});
334+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
335+
336+
var source = source(b -> b.field("name", "quick brown fox"));
337+
ParsedDocument doc = mapper.parse(source);
338+
{
339+
List<IndexableField> fields = doc.rootDoc().getFields("name.text");
340+
IndexableFieldType fieldType = fields.get(0).fieldType();
341+
assertThat(fieldType.stored(), is(false));
342+
}
343+
{
344+
List<IndexableField> fields = doc.rootDoc().getFields("name.text._original");
345+
assertThat(fields, empty());
346+
}
347+
}
258348
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -602,6 +602,7 @@ public static class Builder {
602602
private boolean hasSyntheticSourceCompatibleKeywordField;
603603

604604
public Builder add(FieldMapper.Builder builder) {
605+
builder.currentFieldIsAMultiField = true;
605606
mapperBuilders.put(builder.leafName(), builder::build);
606607

607608
if (builder instanceof KeywordFieldMapper.Builder kwd) {
@@ -1384,6 +1385,7 @@ public abstract static class Builder extends Mapper.Builder implements ToXConten
13841385
protected Optional<SourceKeepMode> sourceKeepMode = Optional.empty();
13851386
protected boolean hasScript = false;
13861387
protected OnScriptError onScriptError = null;
1388+
protected boolean currentFieldIsAMultiField = false;
13871389

13881390
/**
13891391
* Creates a new Builder with a field name

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,9 +300,12 @@ public Builder(String name, IndexVersion indexCreatedVersion, IndexAnalyzers ind
300300
// storing the field without requiring users to explicitly set 'store'.
301301
//
302302
// If 'store' parameter was explicitly provided we'll reject the request.
303+
// Note that if current builder is a multi field, then we don't need to store, given that responsibility lies with parent field
303304
this.store = Parameter.storeParam(
304305
m -> ((TextFieldMapper) m).store,
305-
() -> isSyntheticSourceEnabled && multiFieldsBuilder.hasSyntheticSourceCompatibleKeywordField() == false
306+
() -> isSyntheticSourceEnabled
307+
&& currentFieldIsAMultiField == false
308+
&& multiFieldsBuilder.hasSyntheticSourceCompatibleKeywordField() == false
306309
);
307310
this.indexCreatedVersion = indexCreatedVersion;
308311
this.analyzers = new TextParams.Analyzers(

server/src/test/java/org/elasticsearch/index/mapper/TextFieldMapperTests.java

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,73 @@ public void testStoreParameterDefaults() throws IOException {
307307
}
308308
}
309309

310+
public void testStoreParameterDefaultsSyntheticSource() throws IOException {
311+
var indexSettingsBuilder = getIndexSettingsBuilder();
312+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
313+
var indexSettings = indexSettingsBuilder.build();
314+
315+
var mapping = mapping(b -> {
316+
b.startObject("name");
317+
b.field("type", "text");
318+
b.endObject();
319+
});
320+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
321+
322+
var source = source(b -> b.field("name", "quick brown fox"));
323+
ParsedDocument doc = mapper.parse(source);
324+
List<IndexableField> fields = doc.rootDoc().getFields("name");
325+
IndexableFieldType fieldType = fields.get(0).fieldType();
326+
assertThat(fieldType.stored(), is(true));
327+
}
328+
329+
public void testStoreParameterDefaultsSyntheticSourceWithKeywordMultiField() throws IOException {
330+
var indexSettingsBuilder = getIndexSettingsBuilder();
331+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
332+
var indexSettings = indexSettingsBuilder.build();
333+
334+
var mapping = mapping(b -> {
335+
b.startObject("name");
336+
b.field("type", "text");
337+
b.startObject("fields");
338+
b.startObject("keyword");
339+
b.field("type", "keyword");
340+
b.endObject();
341+
b.endObject();
342+
b.endObject();
343+
});
344+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
345+
346+
var source = source(b -> b.field("name", "quick brown fox"));
347+
ParsedDocument doc = mapper.parse(source);
348+
List<IndexableField> fields = doc.rootDoc().getFields("name");
349+
IndexableFieldType fieldType = fields.get(0).fieldType();
350+
assertThat(fieldType.stored(), is(false));
351+
}
352+
353+
public void testStoreParameterDefaultsSyntheticSourceTextFieldIsMultiField() throws IOException {
354+
var indexSettingsBuilder = getIndexSettingsBuilder();
355+
indexSettingsBuilder.put(IndexSettings.INDEX_MAPPER_SOURCE_MODE_SETTING.getKey(), "synthetic");
356+
var indexSettings = indexSettingsBuilder.build();
357+
358+
var mapping = mapping(b -> {
359+
b.startObject("name");
360+
b.field("type", "keyword");
361+
b.startObject("fields");
362+
b.startObject("text");
363+
b.field("type", "text");
364+
b.endObject();
365+
b.endObject();
366+
b.endObject();
367+
});
368+
DocumentMapper mapper = createMapperService(indexSettings, mapping).documentMapper();
369+
370+
var source = source(b -> b.field("name", "quick brown fox"));
371+
ParsedDocument doc = mapper.parse(source);
372+
List<IndexableField> fields = doc.rootDoc().getFields("name.text");
373+
IndexableFieldType fieldType = fields.get(0).fieldType();
374+
assertThat(fieldType.stored(), is(false));
375+
}
376+
310377
public void testBWCSerialization() throws IOException {
311378
MapperService mapperService = createMapperService(fieldMapping(b -> {
312379
b.field("type", "text");

0 commit comments

Comments
 (0)