Merge pull request #6227 from vespa-engine/arnej/last-summary-overrides-older

let last fetched summary override

3 files changed, 43 insertions, 9 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
index 1160ea0a204..3a2e922f9e0 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
@@ -50,9 +50,9 @@ public class FastHit extends Hit {
      * Summaries added to this hit which are not yet decoded into fields.
      * Fields are resolved by returning the first non-null value found by
      * 1) the field value from the Map of fields in the Hit supertype, and
-     * 2) each of the summaries, in the order of the list (which is the add order).
+     * 2) each of the summaries, reverse add order
      * This ensures that values set from code overwrites any value received as
-     * summary data.
+     * summary data, and fetching a new summary overrides previous summaries.
      *
      * The reason we keep this rather than eagerly decoding into a the field map
      * is to reduce garbage collection and decoding cost, with the assumption
@@ -163,7 +163,7 @@ public class FastHit extends Hit {
     public void addSummary(DocsumDefinition docsumDef, Inspector value) {
         if (removedFields != null)
             removedFields.removeAll(docsumDef.fieldNames());
-        summaries.add(new SummaryData(this, docsumDef, value, summaries.size()));
+        summaries.add(0, new SummaryData(this, docsumDef, value, 1 + summaries.size()));
     }
 
     /**
@@ -331,6 +331,7 @@ public class FastHit extends Hit {
     private Object getSummaryValue(String name) {
         if (removedFields != null && removedFields.contains(name))
             return null;
+        // fetch from last added summary with the field
         for (SummaryData summaryData : summaries) {
             Object value = summaryData.getField(name);
             if (value != null) return value;
@@ -520,7 +521,7 @@ public class FastHit extends Hit {
         private final DocsumDefinition type;
         private final Inspector data;
 
-        /** The index of this summary in the list of summaries added to this */
+        /** The index from the end of this summary in the list of summaries */
         private final int index;
 
         SummaryData(FastHit hit, DocsumDefinition type, Inspector data, int index) {
@@ -577,11 +578,11 @@ public class FastHit extends Hit {
 
         /**
          * Returns whether this field is present in the map properties
-         * or an earlier (lower index) summary in this hit
+         * or a summary added later in this hit
          */
         private boolean shadowed(String name) {
             if (hit.hasField(name)) return true;
-            for (int i = 0; i < index; i++) {
+            for (int i = 0; i < hit.summaries.size() - index; i++) {
                 if (hit.summaries.get(i).type.fieldNames().contains(name))
                     return true;
             }
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
index 421f70c8d07..81f16f7f261 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
@@ -35,9 +35,11 @@ import static org.junit.Assert.fail;
 
 public class SlimeSummaryTestCase {
 
-    private static final String summary_cf = "file:src/test/java/com/yahoo/prelude/fastsearch/summary.cfg";
-    private static final String partial_summary1_cf = "file:src/test/java/com/yahoo/prelude/fastsearch/partial-summary1.cfg";
-    private static final String partial_summary2_cf = "file:src/test/java/com/yahoo/prelude/fastsearch/partial-summary2.cfg";
+    private static final String cf_pre = "file:src/test/java/com/yahoo/prelude/fastsearch/";
+    private static final String summary_cf = cf_pre + "summary.cfg";
+    private static final String partial_summary1_cf = cf_pre + "partial-summary1.cfg";
+    private static final String partial_summary2_cf = cf_pre + "partial-summary2.cfg";
+    private static final String partial_summary3_cf = cf_pre + "partial-summary3.cfg";
 
     @Test
     public void testDecodingEmpty() {
@@ -143,6 +145,7 @@ public class SlimeSummaryTestCase {
     public void testFieldAccessAPI() {
         DocsumDefinitionSet partialDocsum1 = createDocsumDefinitionSet(partial_summary1_cf);
         DocsumDefinitionSet partialDocsum2 = createDocsumDefinitionSet(partial_summary2_cf);
+        DocsumDefinitionSet partialDocsum3 = createDocsumDefinitionSet(partial_summary3_cf);
         DocsumDefinitionSet fullDocsum = createDocsumDefinitionSet(summary_cf);
         FastHit hit = new FastHit();
         Map<String, Object> expected = new HashMap<>();
@@ -273,6 +276,18 @@ public class SlimeSummaryTestCase {
         expected.put("string_field", "string_value");
         expected.put("longstring_field", "longstring_value");
         assertFields(expected, hit);
+
+        hit.removeField("string_field");
+        hit.removeField("integer_field");
+        partialDocsum3.lazyDecode("partial3", partialSummary3(), hit);
+        expected.put("string_field", "new str val");
+        expected.put("integer_field", 5);
+        assertFields(expected, hit);
+
+        hit.removeField("integer_field");
+        partialDocsum2.lazyDecode("partial2", partialSummary2(), hit);
+        expected.put("integer_field", 4);
+        assertFields(expected, hit);
     }
 
 
@@ -344,6 +359,14 @@ public class SlimeSummaryTestCase {
         return encode((slime));
     }
 
+    private byte[] partialSummary3() {
+        Slime slime = new Slime();
+        Cursor docsum = slime.setObject();
+        docsum.setString("string_field", "new str val");
+        docsum.setLong("integer_field", 5);
+        return encode((slime));
+    }
+
     private byte[] fullishSummary() {
         Slime slime = new Slime();
         Cursor docsum = slime.setObject();
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/partial-summary3.cfg b/container-search/src/test/java/com/yahoo/prelude/fastsearch/partial-summary3.cfg
new file mode 100644
index 00000000000..5d7319fd393
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/partial-summary3.cfg
@@ -0,0 +1,10 @@
+documentdb[1]
+documentdb[0].name test
+documentdb[0].summaryclass[1]
+documentdb[0].summaryclass[0].name partial3
+documentdb[0].summaryclass[0].id 3
+documentdb[0].summaryclass[0].fields[3]
+documentdb[0].summaryclass[0].fields[0].name integer_field
+documentdb[0].summaryclass[0].fields[0].type integer
+documentdb[0].summaryclass[0].fields[1].name string_field
+documentdb[0].summaryclass[0].fields[1].type string