Skip to content

Fix dynamic_templates merge in legacy index templates#21517

Open
Harshavardhan2951 wants to merge 1 commit intoopensearch-project:mainfrom
Harshavardhan2951:fix/dynamic-templates-merge-issue-#21430
Open

Fix dynamic_templates merge in legacy index templates#21517
Harshavardhan2951 wants to merge 1 commit intoopensearch-project:mainfrom
Harshavardhan2951:fix/dynamic-templates-merge-issue-#21430

Conversation

@Harshavardhan2951
Copy link
Copy Markdown

Description

Fixes dynamic_templates merge regression in legacy index templates.

Issues Resolved

Fixes #21430

Problem

When multiple legacy index templates (_template API) matching the
same index pattern each define dynamic_templates, only one template's
dynamic_templates survive in the final mapping. The others are
silently lost. This worked correctly in 2.19.0 but regressed in 3.x.

Root Cause

In MetadataCreateIndexService.mergeTemplateFieldMappings(), there
was no special handling for the dynamic_templates key which holds
a List. When the target map already contained dynamic_templates,
the source list was silently ignored.

Fix

Added explicit handling to combine both lists instead of ignoring
the source list.

Testing

  • Manually reproduced bug on OpenSearch 3.6.0 via Docker
  • Verified fix works on local 3.7.0-SNAPSHOT build
  • Added unit test testValidateDynamicTemplatesMerge

Checklist

  • By submitting this pull request, I confirm that my contribution
    is made under the terms of the Apache 2.0 license.
    For more information on following Developer Certificate of Origin
    and signing off your commits, please check
    here.

Dynamic templates from multiple legacy index templates were being
overwritten instead of merged when creating an index.

Added special handling in mergeTemplateFieldMappings to combine
dynamic_templates lists instead of overwriting.

Fixes opensearch-project#21430

Signed-off-by: Harshavardhan2951 <harshavardhan200101@gmail.com>
@Harshavardhan2951 Harshavardhan2951 requested a review from a team as a code owner May 6, 2026 17:42
@github-actions github-actions Bot added bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing labels May 6, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Merge Order

The fix combines targetList first, then appends sourceList. Since templates are processed in priority order, it's worth verifying that the resulting merge order (target entries first, source entries appended) is semantically correct for dynamic_templates precedence. Dynamic templates are evaluated in order, so the merge order directly affects which template rule wins for a given field.

List<Object> combined = new ArrayList<>(targetList);
combined.addAll(sourceList);
target.put(key, combined);
Duplicate Entries

When merging lists, there is no deduplication logic. If two templates define a dynamic_template with the same name/key, both entries will appear in the combined list, potentially causing unexpected behavior or conflicts at mapping time.

List<Object> combined = new ArrayList<>(targetList);
combined.addAll(sourceList);
target.put(key, combined);

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Deduplicate merged dynamic template entries

When merging dynamic_templates, duplicate entries (same template name/key) from
multiple templates could be silently included, potentially causing unexpected
behavior. Consider deduplicating entries by their template name key before
combining, so that higher-priority templates override lower-priority ones rather
than stacking duplicates.

server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java [1023-1025]

 List<Object> combined = new ArrayList<>(targetList);
-combined.addAll(sourceList);
+Set<String> existingKeys = new HashSet<>();
+for (Object entry : targetList) {
+    if (entry instanceof Map) {
+        existingKeys.addAll(((Map<?, ?>) entry).keySet().stream()
+            .map(Object::toString).collect(java.util.stream.Collectors.toSet()));
+    }
+}
+for (Object entry : sourceList) {
+    if (entry instanceof Map) {
+        boolean isDuplicate = ((Map<?, ?>) entry).keySet().stream()
+            .map(Object::toString).anyMatch(existingKeys::contains);
+        if (!isDuplicate) {
+            combined.add(entry);
+        }
+    } else {
+        combined.add(entry);
+    }
+}
 target.put(key, combined);
Suggestion importance[1-10]: 5

__

Why: The suggestion raises a valid concern about duplicate dynamic_templates entries when merging multiple templates. However, the existing behavior of combining all entries (including duplicates) may be intentional to preserve all template rules, and the deduplication logic adds significant complexity. The impact is moderate as duplicates could cause unexpected behavior in edge cases.

Low
Validate merged dynamic templates ordering

The test only verifies that both template entries are present but does not validate
their order. Since dynamic_templates ordering matters in OpenSearch (templates are
evaluated in order), the test should also assert that the higher-priority template
(from template_1) appears before the lower-priority one (template_2).

server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java [4033]

 assertEquals("dynamic_templates from Multiple legacy templats should be merged", 2, dynamicTemplates.size());
+// Verify order: template_1 (higher priority) entries should come before template_2 entries
+Map<?, ?> firstEntry = (Map<?, ?>) dynamicTemplates.get(0);
+Map<?, ?> secondEntry = (Map<?, ?>) dynamicTemplates.get(1);
+assertTrue("First dynamic template should be from template_1 (strings)", firstEntry.containsKey("strings"));
+assertTrue("Second dynamic template should be from template_2 (floats)", secondEntry.containsKey("floats"));
Suggestion importance[1-10]: 5

__

Why: Since dynamic_templates ordering matters in OpenSearch for evaluation priority, verifying the order in the test is a meaningful improvement. The suggested improved_code correctly extends the existing assertion to also check that template_1's entry appears before template_2's entry.

Low

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

❌ Gradle check result for 2dc9cf0: FAILURE

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Indexing Indexing, Bulk Indexing and anything related to indexing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Dynamic template mappings defined in legacy index templates no longer merge

1 participant