Merge pull request #21634 from vespa-engine/bjorncs/fix-grouping-result-merging

Fix grouping result merging

2 files changed, 29 insertions, 39 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java b/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java
index 5ce7accfdd4..05c6c7f848e 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java
@@ -3,7 +3,9 @@ package com.yahoo.search.dispatch;
 
 import com.yahoo.prelude.fastsearch.DocsumDefinitionSet;
 import com.yahoo.prelude.fastsearch.GroupingListHit;
+import com.yahoo.search.Query;
 import com.yahoo.searchlib.aggregation.Grouping;
+import com.yahoo.searchlib.aggregation.Hit;
 
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -21,12 +23,14 @@ class GroupingResultAggregator {
     private static final Logger log = Logger.getLogger(GroupingResultAggregator.class.getName());
 
     private final Map<Integer, Grouping> groupings = new LinkedHashMap<>();
-    private DocsumDefinitionSet documentDefinitions = null;
+    private DocsumDefinitionSet docsumDefinitions = null;
+    private Query query = null;
     private int groupingHitsMerged = 0;
 
     void mergeWith(GroupingListHit result) {
-        if (groupingHitsMerged == 0) documentDefinitions = result.getDocsumDefinitionSet();
         ++groupingHitsMerged;
+        if (docsumDefinitions == null) docsumDefinitions = result.getDocsumDefinitionSet();
+        if (query == null) query = result.getQuery();
         log.log(Level.FINE, () ->
                 String.format("Merging hit #%d having %d groupings",
                         groupingHitsMerged, result.getGroupingList().size()));
@@ -41,10 +45,15 @@ class GroupingResultAggregator {
     Optional<GroupingListHit> toAggregatedHit() {
         if (groupingHitsMerged == 0) return Optional.empty();
         log.log(Level.FINE, () ->
-                String.format("Creating aggregated hit containing %d groupings from %d hits",
-                        groupings.size(), groupingHitsMerged));
-        groupings.values().forEach(Grouping::postMerge);
-        return Optional.of(new GroupingListHit(List.copyOf(groupings.values()), documentDefinitions));
+                String.format("Creating aggregated hit containing %d groupings from %d hits with docsums '%s' and %s",
+                        groupings.size(), groupingHitsMerged, docsumDefinitions, query));
+        GroupingListHit groupingHit = new GroupingListHit(List.copyOf(groupings.values()), docsumDefinitions);
+        groupingHit.setQuery(query);
+        groupingHit.getGroupingList().forEach(g -> {
+            g.select(o -> o instanceof Hit, o -> ((Hit)o).setContext(groupingHit));
+            g.postMerge();
+        });
+        return Optional.of(groupingHit);
     }
 
 }
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
index b3399390bd9..3e707b0cd38 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/vespa/GroupingExecutor.java
@@ -1,19 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.search.grouping.vespa;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.logging.Logger;
-
 import com.yahoo.component.ComponentId;
 import com.yahoo.component.chain.dependencies.After;
 import com.yahoo.component.chain.dependencies.Provides;
-import java.util.logging.Level;
 import com.yahoo.prelude.fastsearch.GroupingListHit;
 import com.yahoo.prelude.query.Item;
 import com.yahoo.prelude.query.QueryCanonicalizer;
@@ -29,8 +19,16 @@ import com.yahoo.search.result.ErrorMessage;
 import com.yahoo.search.result.Hit;
 import com.yahoo.search.searchchain.Execution;
 import com.yahoo.searchlib.aggregation.Grouping;
-import com.yahoo.vespa.objects.ObjectOperation;
-import com.yahoo.vespa.objects.ObjectPredicate;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 /**
  * Executes the {@link GroupingRequest grouping requests} set up by other searchers. This does the necessary
@@ -318,9 +316,11 @@ public class GroupingExecutor extends Searcher {
         for (Iterator<Hit> i = result.hits().unorderedIterator(); i.hasNext(); ) {
             Hit hit = i.next();
             if (hit instanceof GroupingListHit) {
-                ContextInjector injector = new ContextInjector(hit);
                 for (Grouping grp : ((GroupingListHit)hit).getGroupingList()) {
-                    grp.select(injector, injector);
+                    grp.select(
+                            o -> o instanceof com.yahoo.searchlib.aggregation.Hit
+                                    && ((com.yahoo.searchlib.aggregation.Hit)o).getContext() == null,
+                            o -> ((com.yahoo.searchlib.aggregation.Hit)o).setContext(hit));
                     Grouping old = ret.get(grp.getId());
                     if (old != null) {
                         old.merge(grp);
@@ -373,25 +373,6 @@ public class GroupingExecutor extends Searcher {
         return new CompoundName(GroupingExecutor.class.getName() + "." + name);
     }
 
-    private static class ContextInjector implements ObjectPredicate, ObjectOperation {
-
-        final Object context;
-
-        ContextInjector(Object context) {
-            this.context = context;
-        }
-
-        @Override
-        public boolean check(Object obj) {
-            return com.yahoo.searchlib.aggregation.Hit.class.isInstance(obj);
-        }
-
-        @Override
-        public void execute(Object obj) {
-            ((com.yahoo.searchlib.aggregation.Hit)obj).setContext(context);
-        }
-    }
-
     private static class RequestContext {
 
         final List<Integer> idList = new LinkedList<>();