Skip to content

Commit 116f32e

Browse files
tanik98tanik98
committed
Using index setting instead of mapping field in _source
Signed-off-by: Tanik Pansuriya <[email protected]>
1 parent f45bedf commit 116f32e

File tree

16 files changed

+136
-234
lines changed

16 files changed

+136
-234
lines changed

‎modules/reindex/src/test/java/org/opensearch/index/reindex/ReindexBasicTests.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -181,20 +181,20 @@ public void testMissingSources() {
181181
}
182182

183183
public void testReindexWithDerivedSource() throws Exception {
184-
// Create source index with _source option set as derived
184+
// Create source index with derived source setting enabled
185185
String sourceIndexMapping = """
186186
{
187187
"settings": {
188188
"index": {
189189
"number_of_shards": 1,
190-
"number_of_replicas": 0
190+
"number_of_replicas": 0,
191+
"derived_source": {
192+
"enabled": true
193+
}
191194
}
192195
},
193196
"mappings": {
194197
"_doc": {
195-
"_source": {
196-
"enabled": "derived"
197-
},
198198
"properties": {
199199
"foo": {
200200
"type": "keyword",

‎server/src/internalClusterTest/java/org/opensearch/get/GetActionIT.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -790,20 +790,20 @@ public void testGeneratedStringFieldsStored() throws IOException {
790790
}
791791

792792
public void testDerivedSourceSimple() throws IOException {
793-
// Create index with _source option as derived source
793+
// Create index with derived source index setting enabled
794794
String createIndexSource = """
795795
{
796796
"settings": {
797797
"index": {
798798
"number_of_shards": 2,
799-
"number_of_replicas": 0
799+
"number_of_replicas": 0,
800+
"derived_source": {
801+
"enabled": true
802+
}
800803
}
801804
},
802805
"mappings": {
803806
"_doc": {
804-
"_source": {
805-
"enabled": "derived"
806-
},
807807
"properties": {
808808
"geopoint_field": {
809809
"type": "geo_point"
@@ -895,9 +895,6 @@ public void testDerivedSource_MultiValuesAndComplexField() throws Exception {
895895
// Create mapping with properly closed objects
896896
String mapping = XContentFactory.jsonBuilder()
897897
.startObject()
898-
.startObject("_source")
899-
.field("enabled", "derived")
900-
.endObject()
901898
.startObject("properties")
902899
.startObject("level1")
903900
.startObject("properties")
@@ -923,8 +920,12 @@ public void testDerivedSource_MultiValuesAndComplexField() throws Exception {
923920

924921
// Create index with settings and mapping
925922
assertAcked(
926-
prepareCreate("test_derive").setSettings(Settings.builder().put("index.number_of_shards", 1).put("index.number_of_replicas", 0))
927-
.setMapping(mapping)
923+
prepareCreate("test_derive").setSettings(
924+
Settings.builder()
925+
.put("index.number_of_shards", 1)
926+
.put("index.number_of_replicas", 0)
927+
.put("index.derived_source.enabled", true)
928+
).setMapping(mapping)
928929
);
929930
ensureGreen();
930931

‎server/src/internalClusterTest/java/org/opensearch/indexing/IndexActionIT.java

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -310,13 +310,10 @@ public void testDocumentWithBlankFieldName() {
310310
assertThat(e.getRootCause().getMessage(), containsString("field name cannot be an empty string"));
311311
}
312312

313-
public void testDeriveSourceMapperValidation() throws Exception {
313+
public void testDeriveSourceMapperValidation() {
314314
// Test 1: Validate basic derive source mapping
315315
String basicMapping = """
316316
{
317-
"_source": {
318-
"enabled": "derived"
319-
},
320317
"properties": {
321318
"numeric_field": {
322319
"type": "long"
@@ -328,14 +325,14 @@ public void testDeriveSourceMapperValidation() throws Exception {
328325
}""";
329326

330327
// Should succeed with derive source enabled and doc values enabled (default)
331-
assertAcked(prepareCreate("test_derive_1").setMapping(basicMapping));
328+
assertAcked(
329+
prepareCreate("test_derive_1").setSettings(Settings.builder().put("index.derived_source.enabled", true))
330+
.setMapping(basicMapping)
331+
);
332332

333333
// Test 2: Validate mapping with doc values disabled
334334
String docValuesDisabledMapping = """
335335
{
336-
"_source": {
337-
"enabled": "derived"
338-
},
339336
"properties": {
340337
"numeric_field": {
341338
"type": "long",
@@ -345,14 +342,16 @@ public void testDeriveSourceMapperValidation() throws Exception {
345342
}""";
346343

347344
// Should fail because doc values and stored are both disabled
348-
expectThrows(MapperParsingException.class, () -> prepareCreate("test_derive_2").setMapping(docValuesDisabledMapping).get());
345+
expectThrows(
346+
MapperParsingException.class,
347+
() -> prepareCreate("test_derive_2").setSettings(Settings.builder().put("index.derived_source.enabled", true))
348+
.setMapping(docValuesDisabledMapping)
349+
.get()
350+
);
349351

350352
// Test 3: Validate mapping with stored enabled but doc values disabled
351353
String storedEnabledMapping = """
352354
{
353-
"_source": {
354-
"enabled": "derived"
355-
},
356355
"properties": {
357356
"numeric_field": {
358357
"type": "long",
@@ -363,14 +362,14 @@ public void testDeriveSourceMapperValidation() throws Exception {
363362
}""";
364363

365364
// Should succeed because stored is enabled
366-
assertAcked(prepareCreate("test_derive_3").setMapping(storedEnabledMapping));
365+
assertAcked(
366+
prepareCreate("test_derive_3").setSettings(Settings.builder().put("index.derived_source.enabled", true))
367+
.setMapping(storedEnabledMapping)
368+
);
367369

368370
// Test 4: Validate keyword field with normalizer
369371
String normalizerMapping = """
370372
{
371-
"_source": {
372-
"enabled": "derived"
373-
},
374373
"properties": {
375374
"keyword_field": {
376375
"type": "keyword",
@@ -385,16 +384,14 @@ public void testDeriveSourceMapperValidation() throws Exception {
385384
() -> prepareCreate("test_derive_4").setSettings(
386385
Settings.builder()
387386
.put("analysis.normalizer.lowercase.type", "custom")
387+
.put("index.derived_source.enabled", true)
388388
.putList("analysis.normalizer.lowercase.filter", "lowercase")
389389
).setMapping(normalizerMapping).get()
390390
);
391391

392392
// Test 5: Validate keyword field with ignore_above
393393
String ignoreAboveMapping = """
394394
{
395-
"_source": {
396-
"enabled": "derived"
397-
},
398395
"properties": {
399396
"keyword_field": {
400397
"type": "keyword",
@@ -404,14 +401,16 @@ public void testDeriveSourceMapperValidation() throws Exception {
404401
}""";
405402

406403
// Should fail because ignore_above is not supported with derive source
407-
expectThrows(MapperParsingException.class, () -> prepareCreate("test_derive_5").setMapping(ignoreAboveMapping).get());
404+
expectThrows(
405+
MapperParsingException.class,
406+
() -> prepareCreate("test_derive_5").setSettings(Settings.builder().put("index.derived_source.enabled", true))
407+
.setMapping(ignoreAboveMapping)
408+
.get()
409+
);
408410

409411
// Test 6: Validate object field with nested enabled
410412
String nestedMapping = """
411413
{
412-
"_source": {
413-
"enabled": "derived"
414-
},
415414
"properties": {
416415
"nested_field": {
417416
"type": "nested",
@@ -425,14 +424,16 @@ public void testDeriveSourceMapperValidation() throws Exception {
425424
}""";
426425

427426
// Should fail because nested fields are not supported with derive source
428-
expectThrows(MapperParsingException.class, () -> prepareCreate("test_derive_6").setMapping(nestedMapping).get());
427+
expectThrows(
428+
MapperParsingException.class,
429+
() -> prepareCreate("test_derive_6").setSettings(Settings.builder().put("index.derived_source.enabled", true))
430+
.setMapping(nestedMapping)
431+
.get()
432+
);
429433

430434
// Test 7: Validate field with copy_to
431435
String copyToMapping = """
432436
{
433-
"_source": {
434-
"enabled": "derived"
435-
},
436437
"properties": {
437438
"field1": {
438439
"type": "keyword",
@@ -445,14 +446,16 @@ public void testDeriveSourceMapperValidation() throws Exception {
445446
}""";
446447

447448
// Should fail because copy_to is not supported with derive source
448-
expectThrows(MapperParsingException.class, () -> prepareCreate("test_derive_7").setMapping(copyToMapping).get());
449+
expectThrows(
450+
MapperParsingException.class,
451+
() -> prepareCreate("test_derive_7").setSettings(Settings.builder().put("index.derived_source.enabled", true))
452+
.setMapping(copyToMapping)
453+
.get()
454+
);
449455

450456
// Test 8: Validate multiple field types
451457
String multiTypeMapping = """
452458
{
453-
"_source": {
454-
"enabled": "derived"
455-
},
456459
"properties": {
457460
"keyword_field": {
458461
"type": "keyword"
@@ -473,14 +476,14 @@ public void testDeriveSourceMapperValidation() throws Exception {
473476
}""";
474477

475478
// Should succeed because all field types support derive source
476-
assertAcked(prepareCreate("test_derive_8").setMapping(multiTypeMapping));
479+
assertAcked(
480+
prepareCreate("test_derive_8").setSettings(Settings.builder().put("index.derived_source.enabled", true))
481+
.setMapping(multiTypeMapping)
482+
);
477483

478484
// Test 9: Validate with both doc_values and stored disabled
479485
String bothDisabledMapping = """
480486
{
481-
"_source": {
482-
"enabled": "derived"
483-
},
484487
"properties": {
485488
"keyword_field": {
486489
"type": "keyword",
@@ -491,6 +494,30 @@ public void testDeriveSourceMapperValidation() throws Exception {
491494
}""";
492495

493496
// Should fail because both doc_values and stored are disabled
494-
expectThrows(MapperParsingException.class, () -> prepareCreate("test_derive_9").setMapping(bothDisabledMapping).get());
497+
expectThrows(
498+
MapperParsingException.class,
499+
() -> prepareCreate("test_derive_9").setSettings(Settings.builder().put("index.derived_source.enabled", true))
500+
.setMapping(bothDisabledMapping)
501+
.get()
502+
);
503+
504+
// Test 10: Validate for the field type, for which derived source is not implemented
505+
String unsupportedFieldType = """
506+
{
507+
"properties": {
508+
"geo_shape_field": {
509+
"type": "geo_shape",
510+
"doc_values": true
511+
}
512+
}
513+
}""";
514+
515+
// Should fail because for geo_shape, derived source feature is not supported
516+
expectThrows(
517+
MapperParsingException.class,
518+
() -> prepareCreate("test_derive_10").setSettings(Settings.builder().put("index.derived_source.enabled", true))
519+
.setMapping(unsupportedFieldType)
520+
.get()
521+
);
495522
}
496523
}

‎server/src/internalClusterTest/java/org/opensearch/search/simple/SimpleSearchIT.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -730,20 +730,20 @@ public void testTooLongRegexInRegexpQuery() throws Exception {
730730
}
731731

732732
public void testDerivedSourceSearch() throws Exception {
733-
// Create index with _source option set as derived
733+
// Create index with derived source setting enabled
734734
String createIndexSource = """
735735
{
736736
"settings": {
737737
"index": {
738738
"number_of_shards": 2,
739-
"number_of_replicas": 0
739+
"number_of_replicas": 0,
740+
"derived_source": {
741+
"enabled": true
742+
}
740743
}
741744
},
742745
"mappings": {
743746
"_doc": {
744-
"_source": {
745-
"enabled": "derived"
746-
},
747747
"properties": {
748748
"geopoint_field": {
749749
"type": "geo_point"

‎server/src/internalClusterTest/java/org/opensearch/update/UpdateIT.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -901,21 +901,21 @@ private void waitForOutstandingRequests(TimeValue timeOut, Semaphore requestsOut
901901
}
902902

903903
public void testDerivedSourceWithUpdates() throws Exception {
904-
// Create index with _source option set to derived
904+
// Create index with derived source setting enabled
905905
String createIndexSource = """
906906
{
907907
"settings": {
908908
"index": {
909909
"number_of_shards": 2,
910910
"number_of_replicas": 0,
911-
"refresh_interval": -1
911+
"refresh_interval": -1,
912+
"derived_source": {
913+
"enabled": true
914+
}
912915
}
913916
},
914917
"mappings": {
915918
"_doc": {
916-
"_source": {
917-
"enabled": "derived"
918-
},
919919
"properties": {
920920
"geopoint_field": {
921921
"type": "geo_point"

‎server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,9 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
268268
IndexMetadata.INGESTION_SOURCE_PARAMS_SETTING,
269269
IndexMetadata.INGESTION_SOURCE_ERROR_STRATEGY_SETTING,
270270

271+
// Setting for derived source feature
272+
IndexSettings.INDEX_DERIVED_SOURCE_SETTING,
273+
271274
// validate that built-in similarities don't get redefined
272275
Setting.groupSetting("index.similarity.", (s) -> {
273276
Map<String, Settings> groups = s.getAsGroups();

‎server/src/main/java/org/opensearch/index/IndexSettings.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -782,6 +782,12 @@ public static IndexMergePolicy fromString(String text) {
782782
Property.IndexScope
783783
);
784784

785+
public static final Setting<Boolean> INDEX_DERIVED_SOURCE_SETTING = Setting.boolSetting(
786+
"index.derived_source.enabled",
787+
false,
788+
Property.IndexScope
789+
);
790+
785791
private final Index index;
786792
private final Version version;
787793
private final Logger logger;
@@ -831,6 +837,7 @@ public static IndexMergePolicy fromString(String text) {
831837
private final RemoteStorePathStrategy remoteStorePathStrategy;
832838
private final boolean isTranslogMetadataEnabled;
833839
private volatile boolean allowDerivedField;
840+
private final boolean derivedSourceEnabled;
834841

835842
/**
836843
* The maximum age of a retention lease before it is considered expired.
@@ -1063,6 +1070,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
10631070
setMergeOnFlushPolicy(scopedSettings.get(INDEX_MERGE_ON_FLUSH_POLICY));
10641071
checkPendingFlushEnabled = scopedSettings.get(INDEX_CHECK_PENDING_FLUSH_ENABLED);
10651072
defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE);
1073+
derivedSourceEnabled = scopedSettings.get(INDEX_DERIVED_SOURCE_SETTING);
10661074
/* There was unintentional breaking change got introduced with [OpenSearch-6424](https://github.com/opensearch-project/OpenSearch/pull/6424) (version 2.7).
10671075
* For indices created prior version (prior to 2.7) which has IndexSort type, they used to type cast the SortField.Type
10681076
* to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to
@@ -2034,4 +2042,8 @@ public void setRemoteStoreRepository(String remoteStoreRepository) {
20342042
public void setRemoteStoreTranslogRepository(String remoteStoreTranslogRepository) {
20352043
this.remoteStoreTranslogRepository = remoteStoreTranslogRepository;
20362044
}
2045+
2046+
public boolean isDerivedSourceEnabled() {
2047+
return derivedSourceEnabled;
2048+
}
20372049
}

‎server/src/main/java/org/opensearch/index/engine/TranslogLeafReader.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ public void document(int docID, StoredFieldVisitor visitor) throws IOException {
330330
throw new IllegalArgumentException("no such doc ID " + docID);
331331
}
332332
if (visitor.needsField(FAKE_SOURCE_FIELD) == StoredFieldVisitor.Status.YES) {
333-
if (mapperService.isDerivedSourceEnabled()) {
333+
if (mapperService.getIndexSettings().isDerivedSourceEnabled()) {
334334
LeafReader leafReader = getInMemoryIndexReader();
335335
assert leafReader != null && leafReader.leaves().size() == 1;
336336
visitor.binaryField(

0 commit comments

Comments
 (0)