Merge pull request #28862 from vespa-engine/balder/avoid-adding-errors-to-self

Balder/avoid adding errors to self

4 files changed, 33 insertions, 14 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java
index 87a9cb61a0e..cc428aec7a7 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/FlatteningSearcher.java
@@ -22,12 +22,13 @@ import java.util.Iterator;
 @Before(GroupingExecutor.COMPONENT_NAME)
 public class FlatteningSearcher extends Searcher {
 
-    private final CompoundName flatten = CompoundName.from("grouping.flatten");
+    private final CompoundName groupingFlatten = CompoundName.from("grouping.flatten");
+    private final CompoundName flatten = CompoundName.from("flatten");
 
     @Override
     public Result search(Query query, Execution execution) {
+        if ( ! query.properties().getBoolean(groupingFlatten, true)) return execution.search(query);
         if ( ! query.properties().getBoolean(flatten, true)) return execution.search(query);
-        if ( ! query.properties().getBoolean("flatten", true)) return execution.search(query);
 
         query.trace("Flattening groups", 2);
         int originalHits = query.getHits();
diff --git a/container-search/src/main/java/com/yahoo/search/result/DefaultErrorHit.java b/container-search/src/main/java/com/yahoo/search/result/DefaultErrorHit.java
index 5fd41be0a54..fdd5c79adb6 100644
--- a/container-search/src/main/java/com/yahoo/search/result/DefaultErrorHit.java
+++ b/container-search/src/main/java/com/yahoo/search/result/DefaultErrorHit.java
@@ -75,6 +75,7 @@ public class DefaultErrorHit extends Hit implements ErrorHit, Cloneable {
 
     /** Add all errors from another error hit to this */
     public void addErrors(ErrorHit errorHit) {
+        if (this == errorHit) return;
         for (Iterator<? extends ErrorMessage> i = errorHit.errorIterator(); i.hasNext();) {
             addError(i.next());
         }
diff --git a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
index c6447f85e61..51c0caf38a9 100644
--- a/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
+++ b/container-search/src/main/java/com/yahoo/search/result/HitGroup.java
@@ -19,7 +19,6 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CompletableFuture;
-import java.util.stream.Collectors;
 
 /**
  * <p>A group of ordered hits. Since hitGroup is itself a kind of Hit,
@@ -504,9 +503,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
 
         int currentIndex = -1;
         ListenableArrayList<Hit> newHits = new ListenableArrayList<>(numHits);
-        for (Iterator<Hit> i = hits.iterator(); i.hasNext();) {
-            Hit hit = i.next();
-
+        for (Hit hit : hits) {
             if (hit.isAuxiliary()) {
                 newHits.add(hit);
             } else {
@@ -700,7 +697,6 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
     // -------------- State bookkeeping
 
     /** Ensures result invariants. Must be called when a hit is added to this result. */
-    @SuppressWarnings("deprecation")
     private void handleNewHit(Hit hit) {
         if (!hit.isAuxiliary())
             concreteHitCount++;
@@ -848,8 +844,8 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
         HitGroup hitGroupClone = (HitGroup) super.clone();
         hitGroupClone.hits = new ListenableArrayList<>(this.hits.size());
         hitGroupClone.unmodifiableHits = Collections.unmodifiableList(hitGroupClone.hits);
-        for (Iterator<Hit> i = this.hits.iterator(); i.hasNext();) {
-            Hit hitClone = i.next().clone();
+        for (Hit value : this.hits) {
+            Hit hitClone = value.clone();
             hitGroupClone.hits.add(hitClone);
         }
         if (this.errorHit != null) { // Find the cloned error and assign it
@@ -944,7 +940,7 @@ public class HitGroup extends Hit implements DataList<Hit>, Cloneable, Iterable<
     }
 
     private Iterable<Hit> fillableHits() {
-        Predicate<Hit> isFillable = hit -> hit.isFillable();
+        Predicate<Hit> isFillable = Hit::isFillable;
 
         return Iterables.filter(hits, isFillable);
     }
diff --git a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
index bbd307c6fac..7ec35151eab 100644
--- a/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/grouping/result/FlatteningSearcherTestCase.java
@@ -12,6 +12,9 @@ import com.yahoo.search.Searcher;
 import com.yahoo.search.grouping.GroupingRequest;
 import com.yahoo.search.grouping.request.GroupingOperation;
 import com.yahoo.search.grouping.vespa.GroupingExecutor;
+import com.yahoo.search.result.DefaultErrorHit;
+import com.yahoo.search.result.ErrorHit;
+import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
 import com.yahoo.search.result.HitGroup;
 import com.yahoo.search.searchchain.Execution;
@@ -72,18 +75,21 @@ public class FlatteningSearcherTestCase {
                 ));
         Execution execution = newExecution(new FlatteningSearcher(),
                 new GroupingExecutor(ComponentId.fromString("grouping")),
-                new ResultProvider(Arrays.asList(
+                new ResultProvider(List.of(
                         new GroupingListHit(List.of(group0), null),
-                        new GroupingListHit(List.of(group1), null))));
+                        new GroupingListHit(List.of(group1), null))),
+                new HitsProvider(List.of(
+                        new DefaultErrorHit("source 1", ErrorMessage.createBackendCommunicationError("backend communication error 1")),
+                        new DefaultErrorHit("source 2", ErrorMessage.createBackendCommunicationError("backend communication error 1")))));
         Result result = execution.search(query);
-        assertEquals(5, result.hits().size());
+        assertEquals(6, result.hits().size());
         assertFlat(result);
         assertEquals(2, result.getTotalHitCount());
     }
 
     private void assertFlat(Result result) {
         for (var hit : result.hits())
-            assertTrue(hit instanceof FastHit);
+            assertTrue(hit instanceof FastHit || hit instanceof ErrorHit);
     }
 
     private FS4Hit fs4Hit(double relevance) {
@@ -126,5 +132,20 @@ public class FlatteningSearcherTestCase {
             return result;
         }
     }
+    @After (GroupingExecutor.COMPONENT_NAME)
+    private static class HitsProvider extends Searcher {
+        private final List<Hit> hits;
+        HitsProvider(List<Hit> hits) {
+            this.hits = hits;
+        }
+        @Override
+        public Result search(Query query, Execution exec) {
+            Result result = exec.search(query);
+            for (Hit hit : hits) {
+                result.hits().add(hit);
+            }
+            return result;
+        }
+    }
 
 }