From 0e159579c61032ca1cc2a0f3b5e175b8b32a6f5c Mon Sep 17 00:00:00 2001
From: Joe Gallo <joegallo@gmail.com>
Date: Tue, 9 Sep 2025 12:54:58 -0400
Subject: [PATCH] Fix allow_duplicates edge case bug in append processor
 (#134319)

---
 docs/changelog/134319.yaml                    |   5 +
 .../ingest/360_append_allow_duplicates.yml    | 110 ++++++++++++++++++
 .../elasticsearch/ingest/IngestDocument.java  |  12 +-
 3 files changed, 122 insertions(+), 5 deletions(-)
 create mode 100644 docs/changelog/134319.yaml
 create mode 100644 modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/360_append_allow_duplicates.yml

diff --git a/docs/changelog/134319.yaml b/docs/changelog/134319.yaml
new file mode 100644
index 0000000000000..79766ba07caad
--- /dev/null
+++ b/docs/changelog/134319.yaml
@@ -0,0 +1,5 @@
+pr: 134319
+summary: Fix `allow_duplicates` edge case bug in append processor
+area: Ingest Node
+type: bug
+issues: []
diff --git a/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/360_append_allow_duplicates.yml b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/360_append_allow_duplicates.yml
new file mode 100644
index 0000000000000..cd4a38f952ef8
--- /dev/null
+++ b/modules/ingest-common/src/yamlRestTest/resources/rest-api-spec/test/ingest/360_append_allow_duplicates.yml
@@ -0,0 +1,110 @@
+---
+setup:
+  - do:
+      ingest.put_pipeline:
+        id: "test-pipeline-1"
+        body: >
+          {
+            "processors": [
+              {
+                "append": {
+                  "if": "ctx?.templating == true",
+                  "field": "dest",
+                  "value": ["{{three}}", "{{three}}", "{{three}}"],
+                  "allow_duplicates": false
+                }
+              },
+              {
+                "foreach": {
+                  "description": "templating results in strings, so convert them if necessary",
+                  "field": "dest",
+                  "processor": {
+                    "convert": {
+                      "field": "_ingest._value",
+                      "type": "long"
+                    }
+                  }
+                }
+              },
+              {
+                "remove": {
+                  "description": "we only care about the dest, so remove everything else",
+                  "keep": ["dest"]
+                }
+              }
+            ]
+          }
+  - do:
+      indices.create:
+        index: "test-some-index"
+
+---
+teardown:
+  - do:
+      indices.delete:
+        index: "test-some-index"
+        ignore_unavailable: true
+  - do:
+      ingest.delete_pipeline:
+        id: "test-pipeline-1"
+        ignore: 404
+
+---
+"allow_duplicates (false) removes duplicates with a present array and templating":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "templating": true,
+            "three": 3,
+            "dest": ["3"],
+            "note": "blargh, duplicate removal is based on strings, because of templating"
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [3] }
+
+---
+"allow_duplicates (false) removes duplicates with an empty array and templating":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "templating": true,
+            "three": 3,
+            "dest": []
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [3] }
+
+---
+"allow_duplicates (false) removes duplicates with a missing array and templating":
+  - do:
+      index:
+        index: test-some-index
+        id: 1
+        pipeline: test-pipeline-1
+        body: >
+          {
+            "templating": true,
+            "three": 3
+          }
+
+  - do:
+      get:
+        index: test-some-index
+        id: "1"
+  - match: { _source.dest: [3] }
diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java
index fa850fe07bd6e..e2c59ad9ce77d 100644
--- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java
+++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java
@@ -555,7 +555,7 @@ private void setFieldValue(String path, Object value, boolean append, boolean al
                 Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get
                 if (object == NOT_FOUND) {
                     List<Object> list = new ArrayList<>();
-                    appendValues(list, value);
+                    appendValues(list, value, allowDuplicates);
                     map.put(leafKey, list);
                 } else {
                     Object list = appendValues(object, value, allowDuplicates);
@@ -605,15 +605,16 @@ private static Object appendValues(Object maybeList, Object value, boolean allow
             list.add(maybeList);
         }
         if (allowDuplicates) {
-            appendValues(list, value);
+            innerAppendValues(list, value);
             return list;
         } else {
             // if no values were appended due to duplication, return the original object so the ingest document remains unmodified
-            return appendValuesWithoutDuplicates(list, value) ? list : maybeList;
+            return innerAppendValuesWithoutDuplicates(list, value) ? list : maybeList;
         }
     }
 
-    private static void appendValues(List<Object> list, Object value) {
+    // helper method for use in appendValues above, please do not call this directly except from that method
+    private static void innerAppendValues(List<Object> list, Object value) {
         if (value instanceof List<?> l) {
             list.addAll(l);
         } else {
@@ -621,7 +622,8 @@ private static void appendValues(List<Object> list, Object value) {
         }
     }
 
-    private static boolean appendValuesWithoutDuplicates(List<Object> list, Object value) {
+    // helper method for use in appendValues above, please do not call this directly except from that method
+    private static boolean innerAppendValuesWithoutDuplicates(List<Object> list, Object value) {
         boolean valuesWereAppended = false;
         if (value instanceof List<?> valueList) {
             for (Object val : valueList) {