Skip to content

Commit 28eeb22

Browse files
gaobinlonggaobinlong
committed
Optimize the code of couting field count
x
1 parent a8998bc commit 28eeb22

5 files changed

Lines changed: 114 additions & 45 deletions

File tree

‎rest-api-spec/src/main/resources/rest-api-spec/test/bulk/100_unmap_fields_beyond_limit.yml‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,15 @@
4242
field3: {
4343
"field4": "field4"
4444
},
45-
"field5": 100
45+
"field5": 100,
46+
"dateField": "2024-07-25T05:11:51.243Z",
47+
"booleanField": true
4648
}
4749
- do:
4850
get:
4951
index: test_index_1
5052
id: 2
51-
- match: { _source: { field3: { field4: "field4" }, field5: 100 }}
53+
- match: { _source: { field3: { field4: "field4" }, field5: 100, "dateField": "2024-07-25T05:11:51.243Z", "booleanField": true }}
5254

5355
- do:
5456
indices.get_mapping:
@@ -58,6 +60,8 @@
5860
- match: {test_index_1.mappings.properties.field2.type: long}
5961
- match: {test_index_1.mappings.properties.field3: null}
6062
- match: {test_index_1.mappings.properties.field5: null}
63+
- match: {test_index_1.mappings.properties.dateField.type: null}
64+
- match: {test_index_1.mappings.properties.booleanField.type: null}
6165

6266
- do:
6367
indices.put_settings:
@@ -80,13 +84,15 @@
8084
field3: {
8185
"field4": "field4"
8286
},
83-
"field5": 100
87+
"field5": 100,
88+
"dateField": "2024-07-25T05:11:51.243Z",
89+
"booleanField": true
8490
}
8591
- do:
8692
get:
8793
index: test_index_1
8894
id: 2
89-
- match: { _source: { field1: "field1", field2: [1,2,3], field3: { field4: "field4" }, field5: 100 }}
95+
- match: { _source: { field1: "field1", field2: [1,2,3], field3: { field4: "field4" }, field5: 100, "dateField": "2024-07-25T05:11:51.243Z", "booleanField": true }}
9096

9197
- do:
9298
indices.get_mapping:
@@ -97,3 +103,5 @@
97103
- match: {test_index_1.mappings.properties.field3.properties.field4.type: text}
98104
- match: {test_index_1.mappings.properties.field3.properties.field4.fields.keyword.type: keyword}
99105
- match: {test_index_1.mappings.properties.field5.type: long}
106+
- match: {test_index_1.mappings.properties.dateField.type: date}
107+
- match: {test_index_1.mappings.properties.booleanField.type: boolean}

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

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,7 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
571571
Mapper.BuilderContext builderContext = new Mapper.BuilderContext(context.indexSettings().getSettings(), context.path());
572572
objectMapper = builder.build(builderContext);
573573
context.addDynamicMapper(objectMapper);
574+
increaseDynamicFieldCountIfNeed(context);
574575
context.path().add(currentFieldName);
575576
parseObjectOrField(context, objectMapper);
576577
context.path().remove();
@@ -587,17 +588,19 @@ private static void parseObject(final ParseContext context, ObjectMapper mapper,
587588
}
588589

589590
private static boolean checkIfUnmapFieldsBeyondTotalFieldsLimit(ParseContext context) {
590-
return context.indexSettings().getUnmapFieldsBeyondTotalFieldsLimit()
591-
&& context.docMapper()
592-
.mappers()
593-
.exceedTotalFieldsLimit(context.indexSettings().getMappingTotalFieldsLimit(), context.getDynamicMappers());
591+
return context.getUnmapFieldsBeyondLimit() && context.docMapper().mappers().exceedTotalFieldsLimit(context.getTotalFieldsLimit());
594592
}
595593

596-
private static boolean checkUnmapFieldsOrNotIfAddNewField(ParseContext context, Mapper mapper) {
597-
return context.indexSettings().getUnmapFieldsBeyondTotalFieldsLimit()
598-
&& context.docMapper()
599-
.mappers()
600-
.exceedTotalFieldsLimitIfAddNewField(context.indexSettings().getMappingTotalFieldsLimit(), mapper);
594+
private static void increaseDynamicFieldCountIfNeed(ParseContext context) {
595+
if (context.getUnmapFieldsBeyondLimit()) {
596+
context.docMapper().mappers().increaseDynamicFieldCount();
597+
}
598+
}
599+
600+
private static void increaseDynamicFieldCountIfNeed(ParseContext context, long fieldCount) {
601+
if (context.getUnmapFieldsBeyondLimit()) {
602+
context.docMapper().mappers().increaseDynamicFieldCount(fieldCount);
603+
}
601604
}
602605

603606
private static void parseArray(ParseContext context, ObjectMapper parentMapper, String lastFieldName, String[] paths)
@@ -650,6 +653,7 @@ private static void parseArray(ParseContext context, ObjectMapper parentMapper,
650653
assert mapper != null;
651654
if (parsesArrayValue(mapper)) {
652655
context.addDynamicMapper(mapper);
656+
increaseDynamicFieldCountIfNeed(context);
653657
context.path().add(arrayFieldName);
654658
parseObjectOrField(context, mapper);
655659
context.path().remove();
@@ -891,12 +895,18 @@ private static void parseDynamicValue(
891895
final Mapper.Builder<?> builder = createBuilderFromDynamicValue(context, token, currentFieldName, dynamic, parentMapper.fullPath());
892896
Mapper mapper = builder.build(builderContext);
893897

894-
// edge case, if index.mapping.total_fields.unmap_fields_beyond_limit is true,
895-
// then we check if adding a new field will cause the field count to exceed the total fields limit, if so we don't add it
896-
if (checkUnmapFieldsOrNotIfAddNewField(context, mapper)) {
897-
return;
898+
// edge case, adding a new field may increase the dynamic field count by 2 or more,
899+
// so we check if adding a new field will cause the total field count to exceed the total fields limit, if so we don't add it
900+
long fieldCount = 0;
901+
if (context.getUnmapFieldsBeyondLimit()) {
902+
fieldCount = context.docMapper().mappers().countFields(mapper);
903+
if (context.docMapper().mappers().exceedTotalFieldsLimitIfAddNewField(context.getTotalFieldsLimit(), fieldCount)) {
904+
return;
905+
}
898906
}
907+
899908
context.addDynamicMapper(mapper);
909+
increaseDynamicFieldCountIfNeed(context, fieldCount);
900910

901911
parseObjectOrField(context, mapper);
902912
}
@@ -1013,6 +1023,7 @@ private static Tuple<Integer, ObjectMapper> getDynamicParentMapper(
10131023
);
10141024
}
10151025
context.addDynamicMapper(mapper);
1026+
increaseDynamicFieldCountIfNeed(context);
10161027
break;
10171028
case FALSE:
10181029
// Should not dynamically create any more mappers so return the last mapper

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

Lines changed: 23 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,9 @@ public final class MappingLookup implements Iterable<Mapper> {
6060
private final Map<String, ObjectMapper> objectMappers;
6161
private final boolean hasNested;
6262
private final FieldTypeLookup fieldTypeLookup;
63-
private final int metadataFieldCount;
6463
private final FieldNameAnalyzer indexAnalyzer;
6564
private final int nonMetadataFieldCount;
6665
private int dynamicFieldCount;
67-
private int dynamicObjectFieldCount;
6866

6967
private static void put(Map<String, Analyzer> analyzers, String key, Analyzer value, Analyzer defaultValue) {
7068
if (value == null) {
@@ -141,7 +139,6 @@ public MappingLookup(
141139
MappedFieldType fieldType = mapper.fieldType();
142140
put(indexAnalyzers, fieldType.name(), fieldType.indexAnalyzer(), defaultIndex);
143141
}
144-
this.metadataFieldCount = metadataFieldCount;
145142

146143
for (FieldAliasMapper aliasMapper : aliasMappers) {
147144
if (objects.containsKey(aliasMapper.name())) {
@@ -158,6 +155,7 @@ public MappingLookup(
158155
this.indexAnalyzer = new FieldNameAnalyzer(indexAnalyzers);
159156
this.objectMappers = Collections.unmodifiableMap(objects);
160157
this.nonMetadataFieldCount = fieldMappers.size() + objectMappers.size() - metadataFieldCount;
158+
this.dynamicFieldCount = this.nonMetadataFieldCount;
161159
}
162160

163161
/**
@@ -197,42 +195,39 @@ public void checkLimits(IndexSettings settings) {
197195
/**
198196
*
199197
* @param limit the value of the setting index.mapping.total_fields_limit
200-
* @param dynamicMappers mappers of the new fields detected by dynamic mapping
201198
* @return true if adding new fields will cause to exceed the total fields limit
202199
*/
203-
public boolean exceedTotalFieldsLimit(long limit, List<Mapper> dynamicMappers) {
204-
if (nonMetadataFieldCount > limit) {
205-
return true;
206-
}
207-
208-
List<ObjectMapper> newObjectMappers = new ArrayList<>();
209-
List<FieldMapper> newFieldMappers = new ArrayList<>();
210-
List<FieldAliasMapper> newFieldAliasMappers = new ArrayList<>();
211-
dynamicMappers.forEach(dynamicMapper -> collect(dynamicMapper, newObjectMappers, newFieldMappers, newFieldAliasMappers));
212-
this.dynamicFieldCount = newFieldMappers.size();
213-
this.dynamicObjectFieldCount = newObjectMappers.size();
214-
215-
return nonMetadataFieldCount + dynamicFieldCount + dynamicObjectFieldCount >= limit;
200+
public boolean exceedTotalFieldsLimit(long limit) {
201+
return nonMetadataFieldCount + dynamicFieldCount >= limit;
216202
}
217203

218204
/**
219205
*
220206
* @param limit the value of the setting index.mapping.total_fields_limit
221-
* @param mapper mapper of the new field detected by dynamic mapping
207+
*
222208
* @return true if adding a new field will cause to exceed the total fields limit
223209
*/
224-
public boolean exceedTotalFieldsLimitIfAddNewField(long limit, Mapper mapper) {
225-
if (nonMetadataFieldCount > limit) {
226-
return true;
227-
}
210+
public boolean exceedTotalFieldsLimitIfAddNewField(long limit, long fieldCount) {
211+
return nonMetadataFieldCount + dynamicFieldCount + fieldCount > limit;
212+
}
228213

229-
List<ObjectMapper> newObjectMappers = new ArrayList<>();
230-
List<FieldMapper> newFieldMappers = new ArrayList<>();
231-
List<FieldAliasMapper> newFieldAliasMappers = new ArrayList<>();
232-
collect(mapper, newObjectMappers, newFieldMappers, newFieldAliasMappers);
214+
public void increaseDynamicFieldCount() {
215+
this.dynamicFieldCount++;
216+
}
233217

234-
return nonMetadataFieldCount + dynamicFieldCount + dynamicObjectFieldCount + newObjectMappers.size() + newFieldMappers
235-
.size() > limit;
218+
public void increaseDynamicFieldCount(long fieldCount) {
219+
this.dynamicFieldCount += fieldCount;
220+
}
221+
222+
public long countFields(Mapper mapper) {
223+
long count = 0;
224+
if (mapper instanceof ObjectMapper || mapper instanceof FieldMapper) {
225+
count++;
226+
}
227+
for (Mapper child : mapper) {
228+
count += countFields(child);
229+
}
230+
return count;
236231
}
237232

238233
private void checkFieldLimit(long limit) {

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@
5959
@PublicApi(since = "1.0.0")
6060
public abstract class ParseContext implements Iterable<ParseContext.Document> {
6161

62+
public abstract boolean getUnmapFieldsBeyondLimit();
63+
64+
public abstract long getTotalFieldsLimit();
65+
6266
/**
6367
* Fork of {@link org.apache.lucene.document.Document} with additional functionality.
6468
*
@@ -346,6 +350,16 @@ public void decrementFieldArrayDepth() {
346350
public void checkFieldArrayDepthLimit() {
347351
in.checkFieldArrayDepthLimit();
348352
}
353+
354+
@Override
355+
public boolean getUnmapFieldsBeyondLimit() {
356+
return in.getUnmapFieldsBeyondLimit();
357+
}
358+
359+
@Override
360+
public long getTotalFieldsLimit() {
361+
return in.getTotalFieldsLimit();
362+
}
349363
}
350364

351365
/**
@@ -392,6 +406,8 @@ public static class InternalParseContext extends ParseContext {
392406
private boolean docsReversed = false;
393407

394408
private final Set<String> ignoredFields = new HashSet<>();
409+
private final boolean unmapFieldsBeyondLimit;
410+
private final long totalFieldsLimit;
395411

396412
public InternalParseContext(
397413
IndexSettings indexSettings,
@@ -417,6 +433,8 @@ public InternalParseContext(
417433
this.currentArrayDepth = 0L;
418434
this.maxAllowedFieldDepth = indexSettings.getMappingDepthLimit();
419435
this.maxAllowedArrayDepth = indexSettings.getMappingDepthLimit();
436+
this.unmapFieldsBeyondLimit = indexSettings.getUnmapFieldsBeyondTotalFieldsLimit();
437+
this.totalFieldsLimit = indexSettings.getMappingTotalFieldsLimit();
420438
}
421439

422440
@Override
@@ -622,6 +640,16 @@ public void checkFieldArrayDepthLimit() {
622640
);
623641
}
624642
}
643+
644+
@Override
645+
public boolean getUnmapFieldsBeyondLimit() {
646+
return this.unmapFieldsBeyondLimit;
647+
}
648+
649+
@Override
650+
public long getTotalFieldsLimit() {
651+
return this.totalFieldsLimit;
652+
}
625653
}
626654

627655
/**

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,33 @@ public void testDynamicStrictAllowTemplatesObjectWithUnmapFieldsBeyondTotalLimit
14081408
assertEquals(0, doc.rootDoc().getFields("test.test1").length);
14091409
}
14101410

1411+
public void testCannotAddNewFieldWithUnmapFieldsBeyondTotalLimit() throws Exception {
1412+
Settings settings = Settings.builder()
1413+
.put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_LIMIT_SETTING.getKey(), 3)
1414+
.put(MapperService.INDEX_MAPPING_TOTAL_FIELDS_UNMAP_FIELDS_BEYONGD_LIMIT_SETTING.getKey(), true)
1415+
.build();
1416+
DocumentMapper mapper = createDocumentMapper(topMapping(b -> {
1417+
b.field("dynamic", "true");
1418+
b.startObject("properties");
1419+
{
1420+
b.startObject("foo");
1421+
b.field("type", "keyword");
1422+
b.endObject();
1423+
b.startObject("bar");
1424+
b.field("type", "keyword");
1425+
b.endObject();
1426+
}
1427+
b.endObject();
1428+
}
1429+
1430+
), settings);
1431+
1432+
// Add a string type field will add two fields into the mapping(text+keyword), so the field `test`
1433+
// will not be added to the mapping because of the total fields limit
1434+
ParsedDocument doc = mapper.parse(source(b -> b.field("test", "baz")));
1435+
assertEquals(0, doc.rootDoc().getFields("test").length);
1436+
}
1437+
14111438
public void testDynamicStrictAllowTemplatesNull() throws Exception {
14121439
DocumentMapper mapper = createDocumentMapper(topMapping(b -> b.field("dynamic", "strict_allow_templates")));
14131440
StrictDynamicMappingException exception = expectThrows(

0 commit comments

Comments
 (0)