Merge pull request #23893 from vespa-engine/arnej/optimize-field-filler

try harder to avoid disk access

2 files changed, 38 insertions, 13 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java b/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java
index db44e13c27e..833c1251a7b 100644
--- a/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java
+++ b/container-search/src/main/java/com/yahoo/search/yql/FieldFiller.java
@@ -60,24 +60,27 @@ public class FieldFiller extends Searcher {
 
     @Override
     public void fill(Result result, String summaryClass, Execution execution) {
-        execution.fill(result, summaryClass);
-
         Set<String> summaryFields = result.getQuery().getPresentation().getSummaryFields();
-
-        if (summaryFields.isEmpty() || summaryClass == null ||
-            result.getQuery().properties().getBoolean(FIELD_FILLER_DISABLE)) {
+        if (summaryFields.isEmpty() || result.getQuery().properties().getBoolean(FIELD_FILLER_DISABLE)) {
+            // no special handling:
+            execution.fill(result, summaryClass);
             return;
         }
-
-        if (intersectionOfAttributes.containsAll(summaryFields)) {
-            if (! SORTABLE_ATTRIBUTES_SUMMARY_CLASS.equals(summaryClass)) {
-                execution.fill(result, SORTABLE_ATTRIBUTES_SUMMARY_CLASS);
+        if (summaryClass != null) {
+            // always fill requested class:
+            execution.fill(result, summaryClass);
+            if (hasAll(summaryFields, summaryClass, result.getQuery().getModel().getRestrict())) {
+                // no more was needed:
+                return;
             }
+        }
+        // we need more:
+        if (intersectionOfAttributes.containsAll(summaryFields)) {
+            // only attributes needed:
+            execution.fill(result, SORTABLE_ATTRIBUTES_SUMMARY_CLASS);
         } else {
-            // Yes, summaryClass may be SORTABLE_ATTRIBUTES_SUMMARY_CLASS here
-            if ( ! hasAll(summaryFields, summaryClass, result.getQuery().getModel().getRestrict())) {
-                execution.fill(result, null);
-            }
+            // fetch all summary fields:
+            execution.fill(result, null);
         }
     }
 
diff --git a/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
index cf3c7911d0e..0bb3095fa9d 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/YqlFieldAndSourceTestCase.java
@@ -106,6 +106,28 @@ public class YqlFieldAndSourceTestCase {
     }
 
     @Test
+    final void testWithOnlyAttributeNoClassRequested() {
+        final Query query = new Query("?query=test&presentation.summaryFields=" + FIELD2);
+        Result result = execution.search(query);
+        execution.fill(result, null);
+        assertEquals(1, result.getConcreteHitCount());
+        assertFalse(result.hits().get(0).isFilled(THIRD_OPTION));
+        assertFalse(result.hits().get(0).isFilled(DEFAULT_SUMMARY_CLASS));
+        assertTrue(result.hits().get(0).isFilled(SORTABLE_ATTRIBUTES_SUMMARY_CLASS));
+    }
+
+    @Test
+    final void testWithOnlyDiskfieldNoClassRequested() {
+        final Query query = new Query("?query=test&presentation.summaryFields=" + FIELD3);
+        Result result = execution.search(query);
+        execution.fill(result, null);
+        assertEquals(1, result.getConcreteHitCount());
+        assertFalse(result.hits().get(0).isFilled(THIRD_OPTION));
+        assertTrue(result.hits().get(0).isFilled(DEFAULT_SUMMARY_CLASS));
+        assertFalse(result.hits().get(0).isFilled(SORTABLE_ATTRIBUTES_SUMMARY_CLASS));
+    }
+
+    @Test
     final void testWithOnlyDiskfieldCorrectClassRequested() {
         final Query query = new Query("?query=test&presentation.summaryFields=" + FIELD3);
         Result result = execution.search(query);