Skip to content

Commit 250c32b

Browse files
Counted keyword: inherit source keep mode from index settings (elastic#120678) (elastic#120871)
This patch adds a property to CountedKeywordMapper to track the synthetic_source_keep index setting. This property is then used to properly implement synthetic source support in the counted_keyword field type, with fallback to the ignore_source mechanism when synthetic_source_keep is set in either the field mapping or the index settings.
1 parent f766086 commit 250c32b

File tree

5 files changed

+553
-69
lines changed

5 files changed

+553
-69
lines changed

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,12 +1093,12 @@ public SyntheticSourceExample(
10931093
this(b -> b.value(inputValue), b -> b.value(result), b -> b.value(blockLoaderResults), mapping);
10941094
}
10951095

1096-
private void buildInput(XContentBuilder b) throws IOException {
1096+
public void buildInput(XContentBuilder b) throws IOException {
10971097
b.field("field");
10981098
inputValue.accept(b);
10991099
}
11001100

1101-
private void buildInputArray(XContentBuilder b, int elementCount) throws IOException {
1101+
public void buildInputArray(XContentBuilder b, int elementCount) throws IOException {
11021102
b.startArray("field");
11031103
for (int i = 0; i < elementCount; i++) {
11041104
inputValue.accept(b);
@@ -1385,7 +1385,7 @@ public final void testSyntheticEmptyList() throws IOException {
13851385
assertThat(syntheticSource(mapper, b -> b.startArray("field").endArray()), equalTo(expected));
13861386
}
13871387

1388-
private boolean shouldUseIgnoreMalformed() {
1388+
protected boolean shouldUseIgnoreMalformed() {
13891389
// 5% of test runs use ignore_malformed
13901390
return supportsIgnoreMalformed() && randomDouble() <= 0.05;
13911391
}

x-pack/plugin/mapper-counted-keyword/src/main/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapper.java

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,7 @@
7676
* 2 for each key (one per document), a <code>counted_terms</code> aggregation on a <code>counted_keyword</code> field will consider
7777
* the actual count and report a count of 3 for each key.</p>
7878
*
79-
* <p>Synthetic source is supported, but uses the fallback "ignore source" infrastructure unless the <code>source_keep_mode</code> is
80-
* explicitly set to <code>none</code> in the field mapping parameters.</p>
79+
* <p>Synthetic source is fully supported.</p>
8180
*/
8281
public class CountedKeywordFieldMapper extends FieldMapper {
8382
public static final String CONTENT_TYPE = "counted_keyword";
@@ -277,9 +276,11 @@ private static CountedKeywordFieldMapper toType(FieldMapper in) {
277276
public static class Builder extends FieldMapper.Builder {
278277
private final Parameter<Boolean> indexed = Parameter.indexParam(m -> toType(m).mappedFieldType.isIndexed(), true);
279278
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
279+
private final SourceKeepMode indexSourceKeepMode;
280280

281-
protected Builder(String name) {
281+
protected Builder(String name, SourceKeepMode indexSourceKeepMode) {
282282
super(name);
283+
this.indexSourceKeepMode = indexSourceKeepMode;
283284
}
284285

285286
@Override
@@ -309,7 +310,8 @@ public FieldMapper build(MapperBuilderContext context) {
309310
countFieldMapper.fieldType()
310311
),
311312
builderParams(this, context),
312-
countFieldMapper
313+
countFieldMapper,
314+
indexSourceKeepMode
313315
);
314316
}
315317
}
@@ -389,21 +391,26 @@ public String fieldName() {
389391
}
390392
}
391393

392-
public static TypeParser PARSER = new TypeParser((n, c) -> new CountedKeywordFieldMapper.Builder(n));
394+
public static TypeParser PARSER = new TypeParser(
395+
(n, c) -> new CountedKeywordFieldMapper.Builder(n, c.getIndexSettings().sourceKeepMode())
396+
);
393397

394398
private final FieldType fieldType;
395399
private final BinaryFieldMapper countFieldMapper;
400+
private final SourceKeepMode indexSourceKeepMode;
396401

397402
protected CountedKeywordFieldMapper(
398403
String simpleName,
399404
FieldType fieldType,
400405
MappedFieldType mappedFieldType,
401406
BuilderParams builderParams,
402-
BinaryFieldMapper countFieldMapper
407+
BinaryFieldMapper countFieldMapper,
408+
SourceKeepMode indexSourceKeepMode
403409
) {
404410
super(simpleName, mappedFieldType, builderParams);
405411
this.fieldType = fieldType;
406412
this.countFieldMapper = countFieldMapper;
413+
this.indexSourceKeepMode = indexSourceKeepMode;
407414
}
408415

409416
@Override
@@ -485,7 +492,7 @@ public Iterator<Mapper> iterator() {
485492

486493
@Override
487494
public FieldMapper.Builder getMergeBuilder() {
488-
return new Builder(leafName()).init(this);
495+
return new Builder(leafName(), indexSourceKeepMode).init(this);
489496
}
490497

491498
@Override
@@ -495,8 +502,8 @@ protected String contentType() {
495502

496503
@Override
497504
protected SyntheticSourceSupport syntheticSourceSupport() {
498-
var keepMode = sourceKeepMode();
499-
if (keepMode.isPresent() == false || keepMode.get() != SourceKeepMode.NONE) {
505+
var keepMode = sourceKeepMode().orElse(indexSourceKeepMode);
506+
if (keepMode != SourceKeepMode.NONE) {
500507
return super.syntheticSourceSupport();
501508
}
502509

x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedKeywordFieldMapperTests.java

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
import org.apache.lucene.index.DocValuesType;
1111
import org.apache.lucene.index.IndexOptions;
1212
import org.apache.lucene.index.IndexableField;
13+
import org.elasticsearch.common.Strings;
14+
import org.elasticsearch.common.settings.Settings;
1315
import org.elasticsearch.core.CheckedConsumer;
1416
import org.elasticsearch.core.Tuple;
1517
import org.elasticsearch.index.mapper.DocumentMapper;
@@ -20,12 +22,15 @@
2022
import org.elasticsearch.search.lookup.SourceFilter;
2123
import org.elasticsearch.test.ESTestCase;
2224
import org.elasticsearch.xcontent.XContentBuilder;
25+
import org.elasticsearch.xcontent.XContentFactory;
2326
import org.junit.AssumptionViolatedException;
2427

2528
import java.io.IOException;
2629
import java.util.Collection;
2730
import java.util.Collections;
31+
import java.util.HashSet;
2832
import java.util.List;
33+
import java.util.Set;
2934
import java.util.stream.Stream;
3035

3136
import static org.hamcrest.Matchers.equalTo;
@@ -75,7 +80,6 @@ public void testSyntheticSourceSingleNullValue() throws IOException {
7580
DocumentMapper mapper = createSytheticSourceMapperService(mapping(b -> {
7681
b.startObject("field");
7782
minimalMapping(b);
78-
b.field("synthetic_source_keep", "none");
7983
b.endObject();
8084
})).documentMapper();
8185

@@ -94,7 +98,6 @@ public void testSyntheticSourceManyNullValue() throws IOException {
9498
DocumentMapper mapper = createSytheticSourceMapperService(mapping(b -> {
9599
b.startObject("field");
96100
minimalMapping(b);
97-
b.field("synthetic_source_keep", "none");
98101
b.endObject();
99102
})).documentMapper();
100103

@@ -114,19 +117,32 @@ public void testSyntheticSourceManyNullValue() throws IOException {
114117
assertThat(syntheticSource(mapper, new SourceFilter(null, new String[] { "field" }), buildInput), equalTo("{}"));
115118
}
116119

117-
@Override
118-
public void testSyntheticSourceKeepAll() throws IOException {
119-
// For now, native synthetic source is only supported when "synthetic_source_keep" mapping attribute is "none"
120-
}
120+
public void testSyntheticSourceIndexLevelKeepArrays() throws IOException {
121+
SyntheticSourceExample example = syntheticSourceSupportForKeepTests(shouldUseIgnoreMalformed()).example(1);
122+
XContentBuilder mappings = mapping(b -> {
123+
b.startObject("field");
124+
example.mapping().accept(b);
125+
b.endObject();
126+
});
121127

122-
@Override
123-
public void testSyntheticSourceKeepArrays() throws IOException {
124-
// For now, native synthetic source is only supported when "synthetic_source_keep" mapping attribute is "none"
125-
}
128+
var settings = Settings.builder()
129+
.put("index.mapping.source.mode", "synthetic")
130+
.put("index.mapping.synthetic_source_keep", "arrays")
131+
.build();
132+
DocumentMapper mapperAll = createMapperService(getVersion(), settings, () -> true, mappings).documentMapper();
126133

127-
@Override
128-
public void testSyntheticSourceKeepNone() throws IOException {
129-
// For now, native synthetic source is only supported when "synthetic_source_keep" mapping attribute is "none"
134+
int elementCount = randomIntBetween(2, 5);
135+
CheckedConsumer<XContentBuilder, IOException> buildInput = (XContentBuilder builder) -> {
136+
example.buildInputArray(builder, elementCount);
137+
};
138+
139+
var builder = XContentFactory.jsonBuilder();
140+
builder.startObject();
141+
buildInput.accept(builder);
142+
builder.endObject();
143+
String expected = Strings.toString(builder);
144+
String actual = syntheticSource(mapperAll, buildInput);
145+
assertThat(actual, equalTo(expected));
130146
}
131147

132148
@Override
@@ -151,16 +167,21 @@ public SyntheticSourceExample example(int maxValues) throws IOException {
151167
return new SyntheticSourceExample(in, out, this::mapping);
152168
}
153169

170+
private final Set<String> previousValues = new HashSet<>();
171+
154172
private Tuple<String, String> generateValue() {
155-
String v = ESTestCase.randomAlphaOfLength(5);
173+
String v;
174+
if (previousValues.size() > 0 && randomBoolean()) {
175+
v = randomFrom(previousValues);
176+
} else {
177+
v = ESTestCase.randomAlphaOfLength(5);
178+
previousValues.add(v);
179+
}
156180
return Tuple.tuple(v, v);
157181
}
158182

159183
private void mapping(XContentBuilder b) throws IOException {
160184
minimalMapping(b);
161-
// For now, synthetic source is only supported when "synthetic_source_keep" is "none".
162-
// Once we implement true synthetic source support, we should remove this.
163-
b.field("synthetic_source_keep", "none");
164185
}
165186

166187
@Override

x-pack/plugin/mapper-counted-keyword/src/test/java/org/elasticsearch/xpack/countedkeyword/CountedTermsAggregatorTests.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.elasticsearch.common.bytes.BytesArray;
1313
import org.elasticsearch.index.mapper.FieldMapper;
1414
import org.elasticsearch.index.mapper.MappedFieldType;
15+
import org.elasticsearch.index.mapper.Mapper;
1516
import org.elasticsearch.index.mapper.MapperBuilderContext;
1617
import org.elasticsearch.index.mapper.MappingLookup;
1718
import org.elasticsearch.index.mapper.SourceToParse;
@@ -40,7 +41,9 @@ protected List<SearchPlugin> getSearchPlugins() {
4041
}
4142

4243
public void testAggregatesCountedKeywords() throws Exception {
43-
FieldMapper mapper = new CountedKeywordFieldMapper.Builder("stacktraces").build(MapperBuilderContext.root(false, false));
44+
FieldMapper mapper = new CountedKeywordFieldMapper.Builder("stacktraces", Mapper.SourceKeepMode.NONE).build(
45+
MapperBuilderContext.root(false, false)
46+
);
4447
MappedFieldType fieldType = mapper.fieldType();
4548

4649
CountedTermsAggregationBuilder aggregationBuilder = new CountedTermsAggregationBuilder("st").field("stacktraces");

0 commit comments

Comments
 (0)