[BUG] derived_source forces store=true on text multi-fields, wasting disk for no reconstruction benefit

[waterWang]

Description

Fixes #22536 — When index.derived_source.enabled=true, TextFieldMapper.build() unconditionally forces store=true on every text mapper, including multi-field text subfields (e.g. keyword + fields.analyzed: text).

Multi-fields are not walked by derived-source _source reconstruction (ObjectMapper.deriveSource() only iterates top-level / object child mappers). The parent keyword field is rebuilt from doc_values. The text multi-field is never consulted for _source, so forcing stored fields wastes disk for no reconstruction benefit.

Root Cause

In TextFieldMapper.Builder.build():

if (context.indexSettings().getAsBoolean(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false)
    || context.indexSettings().getAsBoolean(IndexSettings.PLUGGABLE_DATAFORMAT_ENABLED_SETTING.getKey(), false)) {
    fieldType.setStored(true);
}

This runs for ALL text fields, including multi-fields built via MultiFields.Builder.build(). Multi-fields are not walked by deriveSource(), so forcing store=true on them is unnecessary.

Fix

Three changes across three files:

1. Mapper.java (BuilderContext): Added isMultiField() / setMultiField() methods to track whether a field is being built as a multi-field.

2. FieldMapper.java (MultiFields.Builder.build()): Set context.setMultiField(true) before building multi-field mappers and reset to false after.

3. TextFieldMapper.java (Builder.build()): Added && !context.isMultiField() check before forcing store=true, so multi-fields are no longer forced to be stored.

Expected Behavior

Field	Auto store=true with derived source?
Top-level type: text	Yes — needed to reconstruct _source
Object child text field	Yes — in deriveSource tree
Multi-field *.analyzed under keyword parent	No — not part of _source reconstruction

Explicit "store": true on a multi-field is still honored if the user asked for it.

Testing

• Existing tests should continue to pass — no functional change for top-level or object-child text fields
• Multi-fields under derived source will no longer be unnecessarily stored, reducing disk usage

Closes #22536

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review State Leak on Exception context.setMultiField(true) is set before the loop but reset to false only after the loop completes normally. If any value.build(context) throws an exception, the multi-field flag remains true on the shared BuilderContext, potentially affecting subsequent field builds using the same context. Consider wrapping in try/finally to guarantee the flag is reset. context.setMultiField(true); Map mapperBuilders = this.mapperBuilders; for (final Map.Entry<String, Mapper.Builder> cursor : this.mapperBuilders.entrySet()) { String key = cursor.getKey(); Mapper.Builder value = cursor.getValue(); Mapper mapper = value.build(context); assert mapper instanceof FieldMapper; mapperBuilders.put(key, mapper); } context.setMultiField(false); context.path().remove(); Nested Multi-Field Handling The flag is set to true on entry and unconditionally reset to false on exit. If multi-field builders can be nested (a build path that re-enters MultiFields.build), the inner completion will incorrectly clear the flag while the outer is still processing siblings. Saving the prior value and restoring it would be safer than always writing false. context.setMultiField(true); Map mapperBuilders = this.mapperBuilders; for (final Map.Entry<String, Mapper.Builder> cursor : this.mapperBuilders.entrySet()) { String key = cursor.getKey(); Mapper.Builder value = cursor.getValue(); Mapper mapper = value.build(context); assert mapper instanceof FieldMapper; mapperBuilders.put(key, mapper); } context.setMultiField(false);

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Restore multi-field flag safely with finally The multiField flag is unconditionally reset to false after the loop, which breaks nested multi-field scenarios and can leak state if build() throws. Save the previous value before setting and restore it in a finally block to preserve correctness under exceptions and nesting. server/src/main/java/org/opensearch/index/mapper/FieldMapper.java [804-815] context.path().add(mainFieldBuilder.name()); +boolean previousMultiField = context.isMultiField(); context.setMultiField(true); -Map mapperBuilders = this.mapperBuilders; -for (final Map.Entry<String, Mapper.Builder> cursor : this.mapperBuilders.entrySet()) { - String key = cursor.getKey(); - Mapper.Builder value = cursor.getValue(); - Mapper mapper = value.build(context); - assert mapper instanceof FieldMapper; - mapperBuilders.put(key, mapper); +try { + Map mapperBuilders = this.mapperBuilders; + for (final Map.Entry<String, Mapper.Builder> cursor : this.mapperBuilders.entrySet()) { + String key = cursor.getKey(); + Mapper.Builder value = cursor.getValue(); + Mapper mapper = value.build(context); + assert mapper instanceof FieldMapper; + mapperBuilders.put(key, mapper); + } +} finally { + context.setMultiField(previousMultiField); + context.path().remove(); } -context.setMultiField(false); -context.path().remove(); Suggestion importance[1-10]: 6 __ Why: Using try/finally to restore the previous multiField state is a reasonable robustness improvement, protecting against exceptions and potential nested multi-field builds. However, multi-fields are typically not nested and the impact is minor in practice.	Low

[github-actions]

❌ Gradle check result for 225f643:

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?