Skip to content

Commit b88f1ed

Browse files
Counted keyword: inherit source keep mode from index settings (#120678)
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 36292d3 commit b88f1ed

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
@@ -1077,12 +1077,12 @@ public SyntheticSourceExample(
10771077
this(b -> b.value(inputValue), b -> b.value(result), b -> b.value(blockLoaderResults), mapping);
10781078
}
10791079

1080-
private void buildInput(XContentBuilder b) throws IOException {
1080+
public void buildInput(XContentBuilder b) throws IOException {
10811081
b.field("field");
10821082
inputValue.accept(b);
10831083
}
10841084

1085-
private void buildInputArray(XContentBuilder b, int elementCount) throws IOException {
1085+
public void buildInputArray(XContentBuilder b, int elementCount) throws IOException {
10861086
b.startArray("field");
10871087
for (int i = 0; i < elementCount; i++) {
10881088
inputValue.accept(b);
@@ -1369,7 +1369,7 @@ public final void testSyntheticEmptyList() throws IOException {
13691369
assertThat(syntheticSource(mapper, b -> b.startArray("field").endArray()), equalTo(expected));
13701370
}
13711371

1372-
private boolean shouldUseIgnoreMalformed() {
1372+
protected boolean shouldUseIgnoreMalformed() {
13731373
// 5% of test runs use ignore_malformed
13741374
return supportsIgnoreMalformed() && randomDouble() <= 0.05;
13751375
}

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";
@@ -274,9 +273,11 @@ private static CountedKeywordFieldMapper toType(FieldMapper in) {
274273
public static class Builder extends FieldMapper.Builder {
275274
private final Parameter<Boolean> indexed = Parameter.indexParam(m -> toType(m).mappedFieldType.isIndexed(), true);
276275
private final Parameter<Map<String, String>> meta = Parameter.metaParam();
276+
private final SourceKeepMode indexSourceKeepMode;
277277

278-
protected Builder(String name) {
278+
protected Builder(String name, SourceKeepMode indexSourceKeepMode) {
279279
super(name);
280+
this.indexSourceKeepMode = indexSourceKeepMode;
280281
}
281282

282283
@Override
@@ -306,7 +307,8 @@ public FieldMapper build(MapperBuilderContext context) {
306307
countFieldMapper.fieldType()
307308
),
308309
builderParams(this, context),
309-
countFieldMapper
310+
countFieldMapper,
311+
indexSourceKeepMode
310312
);
311313
}
312314
}
@@ -386,21 +388,26 @@ public String fieldName() {
386388
}
387389
}
388390

389-
public static TypeParser PARSER = new TypeParser((n, c) -> new CountedKeywordFieldMapper.Builder(n));
391+
public static TypeParser PARSER = new TypeParser(
392+
(n, c) -> new CountedKeywordFieldMapper.Builder(n, c.getIndexSettings().sourceKeepMode())
393+
);
390394

391395
private final FieldType fieldType;
392396
private final BinaryFieldMapper countFieldMapper;
397+
private final SourceKeepMode indexSourceKeepMode;
393398

394399
protected CountedKeywordFieldMapper(
395400
String simpleName,
396401
FieldType fieldType,
397402
MappedFieldType mappedFieldType,
398403
BuilderParams builderParams,
399-
BinaryFieldMapper countFieldMapper
404+
BinaryFieldMapper countFieldMapper,
405+
SourceKeepMode indexSourceKeepMode
400406
) {
401407
super(simpleName, mappedFieldType, builderParams);
402408
this.fieldType = fieldType;
403409
this.countFieldMapper = countFieldMapper;
410+
this.indexSourceKeepMode = indexSourceKeepMode;
404411
}
405412

406413
@Override
@@ -482,7 +489,7 @@ public Iterator<Mapper> iterator() {
482489

483490
@Override
484491
public FieldMapper.Builder getMergeBuilder() {
485-
return new Builder(leafName()).init(this);
492+
return new Builder(leafName(), indexSourceKeepMode).init(this);
486493
}
487494

488495
@Override
@@ -492,8 +499,8 @@ protected String contentType() {
492499

493500
@Override
494501
protected SyntheticSourceSupport syntheticSourceSupport() {
495-
var keepMode = sourceKeepMode();
496-
if (keepMode.isPresent() == false || keepMode.get() != SourceKeepMode.NONE) {
502+
var keepMode = sourceKeepMode().orElse(indexSourceKeepMode);
503+
if (keepMode != SourceKeepMode.NONE) {
497504
return super.syntheticSourceSupport();
498505
}
499506

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)