Skip to content

Commit 96802e5

Browse files
nknizenknize
authored
[Backport 2.0] [Remove] Type from nested fields using new metadata field mapper (#3097)
* [Remove] Type from nested fields using new metadata field mapper (#3004) * [Remove] Type from nested fields using new metadata field mapper types support is removed yet nested documents use the _type field to store the path for nested documents. A new _nested_path metadata field mapper is added to take the place of the _type field in order to remove the type dependency in nested documents. BWC is handled in the new field mapper to ensure compatibility with older versions. Signed-off-by: Nicholas Walter Knize <[email protected]> * spotless apply Signed-off-by: Nicholas Walter Knize <[email protected]> * add back Nested field mapper to IndicesModuleTests Signed-off-by: Nicholas Walter Knize <[email protected]>
1 parent d61aa40 commit 96802e5

21 files changed

+264
-101
lines changed

‎server/src/main/java/org/opensearch/index/mapper/DocumentParser.java

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -455,21 +455,23 @@ private static void innerParseObject(
455455
private static void nested(ParseContext context, ObjectMapper.Nested nested) {
456456
ParseContext.Document nestedDoc = context.doc();
457457
ParseContext.Document parentDoc = nestedDoc.getParent();
458+
Version indexVersion = context.indexSettings().getIndexVersionCreated();
458459
if (nested.isIncludeInParent()) {
459-
addFields(nestedDoc, parentDoc);
460+
addFields(indexVersion, nestedDoc, parentDoc);
460461
}
461462
if (nested.isIncludeInRoot()) {
462463
ParseContext.Document rootDoc = context.rootDoc();
463464
// don't add it twice, if its included in parent, and we are handling the master doc...
464465
if (!nested.isIncludeInParent() || parentDoc != rootDoc) {
465-
addFields(nestedDoc, rootDoc);
466+
addFields(indexVersion, nestedDoc, rootDoc);
466467
}
467468
}
468469
}
469470

470-
private static void addFields(ParseContext.Document nestedDoc, ParseContext.Document rootDoc) {
471+
private static void addFields(Version indexVersion, ParseContext.Document nestedDoc, ParseContext.Document rootDoc) {
472+
String nestedPathFieldName = NestedPathFieldMapper.name(indexVersion);
471473
for (IndexableField field : nestedDoc.getFields()) {
472-
if (!field.name().equals(TypeFieldMapper.NAME)) {
474+
if (field.name().equals(nestedPathFieldName) == false) {
473475
rootDoc.add(field);
474476
}
475477
}
@@ -498,7 +500,7 @@ private static ParseContext nestedContext(ParseContext context, ObjectMapper map
498500
// the type of the nested doc starts with __, so we can identify that its a nested one in filters
499501
// note, we don't prefix it with the type of the doc since it allows us to execute a nested query
500502
// across types (for example, with similar nested objects)
501-
nestedDoc.add(new Field(TypeFieldMapper.NAME, mapper.nestedTypePathAsString(), TypeFieldMapper.Defaults.NESTED_FIELD_TYPE));
503+
nestedDoc.add(NestedPathFieldMapper.field(context.indexSettings().getIndexVersionCreated(), mapper.nestedTypePath()));
502504
return context;
503505
}
504506

‎server/src/main/java/org/opensearch/index/mapper/NestedPathFieldMapper.java

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.index.mapper;
10+
11+
import org.apache.lucene.document.Field;
12+
import org.apache.lucene.document.FieldType;
13+
import org.apache.lucene.index.IndexOptions;
14+
import org.apache.lucene.index.Term;
15+
import org.apache.lucene.search.Query;
16+
import org.apache.lucene.search.TermQuery;
17+
import org.apache.lucene.util.BytesRef;
18+
import org.opensearch.Version;
19+
import org.opensearch.index.query.QueryShardContext;
20+
import org.opensearch.search.lookup.SearchLookup;
21+
22+
import java.util.Collections;
23+
24+
public class NestedPathFieldMapper extends MetadataFieldMapper {
25+
// OpenSearch version 2.0 removed types; this name is used for bwc
26+
public static final String LEGACY_NAME = "_type";
27+
public static final String NAME = "_nested_path";
28+
29+
public static class Defaults {
30+
public static final FieldType FIELD_TYPE = new FieldType();
31+
static {
32+
FIELD_TYPE.setIndexOptions(IndexOptions.DOCS);
33+
FIELD_TYPE.setTokenized(false);
34+
FIELD_TYPE.setStored(false);
35+
FIELD_TYPE.setOmitNorms(true);
36+
FIELD_TYPE.freeze();
37+
}
38+
}
39+
40+
/** private ctor; using SINGLETON to control BWC */
41+
private NestedPathFieldMapper(String name) {
42+
super(new NestedPathFieldType(name));
43+
}
44+
45+
/** returns the field name */
46+
public static String name(Version version) {
47+
if (version.before(Version.V_2_0_0)) {
48+
return LEGACY_NAME;
49+
}
50+
return NAME;
51+
}
52+
53+
@Override
54+
protected String contentType() {
55+
return NAME;
56+
}
57+
58+
private static final NestedPathFieldMapper LEGACY_INSTANCE = new NestedPathFieldMapper(LEGACY_NAME);
59+
private static final NestedPathFieldMapper INSTANCE = new NestedPathFieldMapper(NAME);
60+
61+
public static final TypeParser PARSER = new FixedTypeParser(
62+
c -> c.indexVersionCreated().before(Version.V_2_0_0) ? LEGACY_INSTANCE : INSTANCE
63+
);
64+
65+
/** helper method to create a lucene field based on the opensearch version */
66+
public static Field field(Version version, String path) {
67+
return new Field(name(version), path, Defaults.FIELD_TYPE);
68+
}
69+
70+
/** helper method to create a query based on the opensearch version */
71+
public static Query filter(Version version, String path) {
72+
return new TermQuery(new Term(name(version), new BytesRef(path)));
73+
}
74+
75+
/** field type for the NestPath field */
76+
public static final class NestedPathFieldType extends StringFieldType {
77+
private NestedPathFieldType(String name) {
78+
super(name, true, false, false, TextSearchInfo.SIMPLE_MATCH_ONLY, Collections.emptyMap());
79+
}
80+
81+
@Override
82+
public String typeName() {
83+
return NAME;
84+
}
85+
86+
@Override
87+
public Query existsQuery(QueryShardContext context) {
88+
throw new UnsupportedOperationException("Cannot run exists() query against the nested field path");
89+
}
90+
91+
@Override
92+
public ValueFetcher valueFetcher(QueryShardContext context, SearchLookup searchLookup, String format) {
93+
throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
94+
}
95+
}
96+
}

‎server/src/main/java/org/opensearch/index/mapper/ObjectMapper.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,9 @@
3232

3333
package org.opensearch.index.mapper;
3434

35-
import org.apache.lucene.index.Term;
3635
import org.apache.lucene.search.Query;
37-
import org.apache.lucene.search.TermQuery;
38-
import org.apache.lucene.util.BytesRef;
3936
import org.opensearch.OpenSearchParseException;
37+
import org.opensearch.Version;
4038
import org.opensearch.common.Explicit;
4139
import org.opensearch.common.Nullable;
4240
import org.opensearch.common.collect.CopyOnWriteHashMap;
@@ -388,8 +386,7 @@ protected static void parseProperties(ObjectMapper.Builder objBuilder, Map<Strin
388386

389387
private final Nested nested;
390388

391-
private final String nestedTypePathAsString;
392-
private final BytesRef nestedTypePathAsBytes;
389+
private final String nestedTypePath;
393390

394391
private final Query nestedTypeFilter;
395392

@@ -420,9 +417,13 @@ protected static void parseProperties(ObjectMapper.Builder objBuilder, Map<Strin
420417
} else {
421418
this.mappers = CopyOnWriteHashMap.copyOf(mappers);
422419
}
423-
this.nestedTypePathAsString = "__" + fullPath;
424-
this.nestedTypePathAsBytes = new BytesRef(nestedTypePathAsString);
425-
this.nestedTypeFilter = new TermQuery(new Term(TypeFieldMapper.NAME, nestedTypePathAsBytes));
420+
Version version = Version.indexCreated(settings);
421+
if (version.before(Version.V_2_0_0)) {
422+
this.nestedTypePath = "__" + fullPath;
423+
} else {
424+
this.nestedTypePath = fullPath;
425+
}
426+
this.nestedTypeFilter = NestedPathFieldMapper.filter(version, nestedTypePath);
426427
}
427428

428429
@Override
@@ -486,8 +487,8 @@ public String fullPath() {
486487
return this.fullPath;
487488
}
488489

489-
public String nestedTypePathAsString() {
490-
return nestedTypePathAsString;
490+
public String nestedTypePath() {
491+
return nestedTypePath;
491492
}
492493

493494
public final Dynamic dynamic() {

‎server/src/main/java/org/opensearch/indices/IndicesModule.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
import org.opensearch.index.mapper.KeywordFieldMapper;
5858
import org.opensearch.index.mapper.Mapper;
5959
import org.opensearch.index.mapper.MetadataFieldMapper;
60+
import org.opensearch.index.mapper.NestedPathFieldMapper;
6061
import org.opensearch.index.mapper.NumberFieldMapper;
6162
import org.opensearch.index.mapper.ObjectMapper;
6263
import org.opensearch.index.mapper.RangeType;
@@ -186,6 +187,7 @@ private static Map<String, MetadataFieldMapper.TypeParser> initBuiltInMetadataMa
186187
builtInMetadataMappers.put(DataStreamFieldMapper.NAME, DataStreamFieldMapper.PARSER);
187188
builtInMetadataMappers.put(SourceFieldMapper.NAME, SourceFieldMapper.PARSER);
188189
builtInMetadataMappers.put(TypeFieldMapper.NAME, TypeFieldMapper.PARSER);
190+
builtInMetadataMappers.put(NestedPathFieldMapper.NAME, NestedPathFieldMapper.PARSER);
189191
builtInMetadataMappers.put(VersionFieldMapper.NAME, VersionFieldMapper.PARSER);
190192
builtInMetadataMappers.put(SeqNoFieldMapper.NAME, SeqNoFieldMapper.PARSER);
191193
// _field_names must be added last so that it has a chance to see all the other mappers

‎server/src/main/java/org/opensearch/indices/mapper/MapperRegistry.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import org.opensearch.Version;
3636
import org.opensearch.index.mapper.Mapper;
3737
import org.opensearch.index.mapper.MetadataFieldMapper;
38+
import org.opensearch.index.mapper.NestedPathFieldMapper;
3839
import org.opensearch.plugins.MapperPlugin;
3940

4041
import java.util.Collections;
@@ -50,6 +51,7 @@ public final class MapperRegistry {
5051

5152
private final Map<String, Mapper.TypeParser> mapperParsers;
5253
private final Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsers;
54+
private final Map<String, MetadataFieldMapper.TypeParser> metadataMapperParsersPre20;
5355
private final Function<String, Predicate<String>> fieldFilter;
5456

5557
public MapperRegistry(
@@ -59,6 +61,9 @@ public MapperRegistry(
5961
) {
6062
this.mapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(mapperParsers));
6163
this.metadataMapperParsers = Collections.unmodifiableMap(new LinkedHashMap<>(metadataMapperParsers));
64+
Map<String, MetadataFieldMapper.TypeParser> tempPre20 = new LinkedHashMap<>(metadataMapperParsers);
65+
tempPre20.remove(NestedPathFieldMapper.NAME);
66+
this.metadataMapperParsersPre20 = Collections.unmodifiableMap(tempPre20);
6267
this.fieldFilter = fieldFilter;
6368
}
6469

@@ -75,7 +80,7 @@ public Map<String, Mapper.TypeParser> getMapperParsers() {
7580
* returned map uses the name of the field as a key.
7681
*/
7782
public Map<String, MetadataFieldMapper.TypeParser> getMetadataMapperParsers(Version indexCreatedVersion) {
78-
return metadataMapperParsers;
83+
return indexCreatedVersion.onOrAfter(Version.V_2_0_0) ? metadataMapperParsers : metadataMapperParsersPre20;
7984
}
8085

8186
/**

‎server/src/test/java/org/opensearch/index/mapper/DocumentParserTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -248,14 +248,14 @@ public void testNestedHaveIdAndTypeFields() throws Exception {
248248
assertNotNull(result.docs().get(0).getField(IdFieldMapper.NAME));
249249
assertEquals(Uid.encodeId("1"), result.docs().get(0).getField(IdFieldMapper.NAME).binaryValue());
250250
assertEquals(IdFieldMapper.Defaults.NESTED_FIELD_TYPE, result.docs().get(0).getField(IdFieldMapper.NAME).fieldType());
251-
assertNotNull(result.docs().get(0).getField(TypeFieldMapper.NAME));
252-
assertEquals("__foo", result.docs().get(0).getField(TypeFieldMapper.NAME).stringValue());
251+
assertNotNull(result.docs().get(0).getField(NestedPathFieldMapper.NAME));
252+
assertEquals("foo", result.docs().get(0).getField(NestedPathFieldMapper.NAME).stringValue());
253253
assertEquals("value1", result.docs().get(0).getField("foo.bar").binaryValue().utf8ToString());
254254
// Root document:
255255
assertNotNull(result.docs().get(1).getField(IdFieldMapper.NAME));
256256
assertEquals(Uid.encodeId("1"), result.docs().get(1).getField(IdFieldMapper.NAME).binaryValue());
257257
assertEquals(IdFieldMapper.Defaults.FIELD_TYPE, result.docs().get(1).getField(IdFieldMapper.NAME).fieldType());
258-
assertNull(result.docs().get(1).getField(TypeFieldMapper.NAME));
258+
assertNull(result.docs().get(1).getField(NestedPathFieldMapper.NAME));
259259
assertEquals("value2", result.docs().get(1).getField("baz").binaryValue().utf8ToString());
260260
}
261261

‎server/src/test/java/org/opensearch/index/mapper/FieldAliasMapperValidationTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ private static ObjectMapper createObjectMapper(String name) {
220220
ObjectMapper.Nested.NO,
221221
ObjectMapper.Dynamic.FALSE,
222222
emptyMap(),
223-
Settings.EMPTY
223+
SETTINGS
224224
);
225225
}
226226

@@ -232,7 +232,7 @@ private static ObjectMapper createNestedObjectMapper(String name) {
232232
ObjectMapper.Nested.newNested(),
233233
ObjectMapper.Dynamic.FALSE,
234234
emptyMap(),
235-
Settings.EMPTY
235+
SETTINGS
236236
);
237237
}
238238
}

‎server/src/test/java/org/opensearch/index/mapper/NestedObjectMapperTests.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public void testSingleNested() throws Exception {
149149
);
150150

151151
assertThat(doc.docs().size(), equalTo(2));
152-
assertThat(doc.docs().get(0).get(TypeFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePathAsString()));
152+
assertThat(doc.docs().get(0).get(NestedPathFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePath()));
153153
assertThat(doc.docs().get(0).get("nested1.field1"), equalTo("1"));
154154
assertThat(doc.docs().get(0).get("nested1.field2"), equalTo("2"));
155155

@@ -180,10 +180,10 @@ public void testSingleNested() throws Exception {
180180
);
181181

182182
assertThat(doc.docs().size(), equalTo(3));
183-
assertThat(doc.docs().get(0).get(TypeFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePathAsString()));
183+
assertThat(doc.docs().get(0).get(NestedPathFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePath()));
184184
assertThat(doc.docs().get(0).get("nested1.field1"), equalTo("1"));
185185
assertThat(doc.docs().get(0).get("nested1.field2"), equalTo("2"));
186-
assertThat(doc.docs().get(1).get(TypeFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePathAsString()));
186+
assertThat(doc.docs().get(1).get(NestedPathFieldMapper.NAME), equalTo(nested1Mapper.nestedTypePath()));
187187
assertThat(doc.docs().get(1).get("nested1.field1"), equalTo("3"));
188188
assertThat(doc.docs().get(1).get("nested1.field2"), equalTo("4"));
189189

‎server/src/test/java/org/opensearch/index/mapper/NestedPathFieldMapperTests.java

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.index.mapper;
10+
11+
import org.apache.lucene.index.IndexableField;
12+
import org.opensearch.common.bytes.BytesArray;
13+
import org.opensearch.common.compress.CompressedXContent;
14+
import org.opensearch.common.settings.Settings;
15+
import org.opensearch.common.xcontent.XContentType;
16+
import org.opensearch.test.OpenSearchSingleNodeTestCase;
17+
18+
import java.io.IOException;
19+
import java.util.Arrays;
20+
import java.util.Collections;
21+
22+
/** tests for {@link org.opensearch.index.mapper.NestedPathFieldMapper} */
23+
public class NestedPathFieldMapperTests extends OpenSearchSingleNodeTestCase {
24+
25+
public void testDefaultConfig() throws IOException {
26+
Settings indexSettings = Settings.EMPTY;
27+
MapperService mapperService = createIndex("test", indexSettings).mapperService();
28+
DocumentMapper mapper = mapperService.merge(
29+
MapperService.SINGLE_MAPPING_NAME,
30+
new CompressedXContent("{\"" + MapperService.SINGLE_MAPPING_NAME + "\":{}}"),
31+
MapperService.MergeReason.MAPPING_UPDATE
32+
);
33+
ParsedDocument document = mapper.parse(new SourceToParse("index", "id", new BytesArray("{}"), XContentType.JSON));
34+
assertEquals(Collections.<IndexableField>emptyList(), Arrays.asList(document.rootDoc().getFields(NestedPathFieldMapper.NAME)));
35+
}
36+
37+
public void testUpdatesWithSameMappings() throws IOException {
38+
Settings indexSettings = Settings.EMPTY;
39+
MapperService mapperService = createIndex("test", indexSettings).mapperService();
40+
DocumentMapper mapper = mapperService.merge(
41+
MapperService.SINGLE_MAPPING_NAME,
42+
new CompressedXContent("{\"" + MapperService.SINGLE_MAPPING_NAME + "\":{}}"),
43+
MapperService.MergeReason.MAPPING_UPDATE
44+
);
45+
mapper.merge(mapper.mapping(), MapperService.MergeReason.MAPPING_UPDATE);
46+
}
47+
}

‎server/src/test/java/org/opensearch/index/search/NestedHelperTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.opensearch.common.xcontent.XContentFactory;
4848
import org.opensearch.index.IndexService;
4949
import org.opensearch.index.mapper.MapperService;
50+
import org.opensearch.index.mapper.NestedPathFieldMapper;
5051
import org.opensearch.index.query.MatchAllQueryBuilder;
5152
import org.opensearch.index.query.NestedQueryBuilder;
5253
import org.opensearch.index.query.QueryShardContext;
@@ -324,7 +325,7 @@ public void testNested() throws IOException {
324325

325326
Query expectedChildQuery = new BooleanQuery.Builder().add(new MatchAllDocsQuery(), Occur.MUST)
326327
// we automatically add a filter since the inner query might match non-nested docs
327-
.add(new TermQuery(new Term("_type", "__nested1")), Occur.FILTER)
328+
.add(new TermQuery(new Term(NestedPathFieldMapper.NAME, "nested1")), Occur.FILTER)
328329
.build();
329330
assertEquals(expectedChildQuery, query.getChildQuery());
330331

@@ -352,7 +353,7 @@ public void testNested() throws IOException {
352353

353354
// we need to add the filter again because of include_in_parent
354355
expectedChildQuery = new BooleanQuery.Builder().add(new TermQuery(new Term("nested2.foo", "bar")), Occur.MUST)
355-
.add(new TermQuery(new Term("_type", "__nested2")), Occur.FILTER)
356+
.add(new TermQuery(new Term(NestedPathFieldMapper.NAME, "nested2")), Occur.FILTER)
356357
.build();
357358
assertEquals(expectedChildQuery, query.getChildQuery());
358359

@@ -367,7 +368,7 @@ public void testNested() throws IOException {
367368

368369
// we need to add the filter again because of include_in_root
369370
expectedChildQuery = new BooleanQuery.Builder().add(new TermQuery(new Term("nested3.foo", "bar")), Occur.MUST)
370-
.add(new TermQuery(new Term("_type", "__nested3")), Occur.FILTER)
371+
.add(new TermQuery(new Term(NestedPathFieldMapper.NAME, "nested3")), Occur.FILTER)
371372
.build();
372373
assertEquals(expectedChildQuery, query.getChildQuery());
373374

0 commit comments

Comments
 (0)