Merge pull request #25763 from vespa-engine/toregge/use-original-source-for-dynamic-summary-transform

Use original source for dynamic summary transform.

8 files changed, 58 insertions, 28 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java b/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java
index ec941eacce1..ca5931fd9e8 100644
--- a/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java
+++ b/config-model/src/main/java/com/yahoo/schema/derived/SummaryClass.java
@@ -70,7 +70,7 @@ public class SummaryClass extends Derived {
                 accessingDiskSummary = true;
             }
             addField(summaryField.getName(), summaryField.getDataType(), summaryField.getTransform(),
-                    getSource(summaryField), fields);
+                    getSource(summaryField, schema), fields);
         }
     }
 
@@ -143,7 +143,7 @@ public class SummaryClass extends Derived {
         }
     }
 
-    static String getSource(SummaryField summaryField) {
+    static String getSource(SummaryField summaryField, Schema schema) {
         if (summaryField.getTransform() == SummaryTransform.NONE) {
             return "";
         }
@@ -159,7 +159,7 @@ public class SummaryClass extends Derived {
         {
             return summaryField.getSingleSource();
         } else if (summaryField.getTransform().isDynamic()) {
-            return DynamicSummaryTransformUtils.getSource(summaryField);
+            return DynamicSummaryTransformUtils.getSource(summaryField, schema);
         } else {
             return "";
         }
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java b/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java
index 20597eca64f..50739ad3883 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java
@@ -1,6 +1,8 @@
 package com.yahoo.schema.processing;
 
 import com.yahoo.document.DataType;
+import com.yahoo.schema.Schema;
+import com.yahoo.schema.document.ImmutableSDField;
 import com.yahoo.vespa.documentmodel.SummaryField;
 
 /**
@@ -50,10 +52,22 @@ public class DynamicSummaryTransformUtils {
         return summaryFieldIsPopulatedBySourceField(summaryField.getDataType());
     }
 
-    public static String getSource(SummaryField summaryField) {
-        // Summary fields with the original supported type is always present in the document type,
-        // and we must use that field as source at run-time.
-        return isOriginalSupportedType(summaryField.getDataType()) ?
-                summaryField.getName() : summaryField.getSingleSource();
+    public static String getSource(SummaryField summaryField, Schema schema) {
+        // Summary fields with the original supported type is always present in the document type.
+        // However, if the source of that summary field is a single explicit source that exists in the schema we
+        // use that as source instead as this is handled by the backend code.
+        // This is a move in the right direction to avoid adding some summary fields as extra document fields.
+        if (isOriginalSupportedType(summaryField.getDataType())) {
+            if (summaryField.hasExplicitSingleSource()) {
+                String sourceFieldName = summaryField.getSingleSource();
+                ImmutableSDField source = schema.getField(sourceFieldName);
+                if (source != null) {
+                    return sourceFieldName;
+                }
+            }
+            return summaryField.getName();
+        } else {
+            return summaryField.getSingleSource();
+        }
     }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java
index 58044163885..7439e65dee6 100644
--- a/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java
+++ b/config-model/src/main/java/com/yahoo/vespa/documentmodel/SummaryField.java
@@ -276,6 +276,9 @@ public class SummaryField extends Field implements Cloneable, TypedKey {
         if (sourceName.contains(".")) {
             return false;
         }
+        if (sources.size() > 1) {
+            return false;
+        }
         return true;
     }
 
diff --git a/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg b/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg
index 3703bb8b218..c5d5b86de67 100644
--- a/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg
+++ b/config-model/src/test/derived/bolding_dynamic_summary/summary.cfg
@@ -35,13 +35,13 @@ classes[].fields[].command "summaryfeatures"
 classes[].fields[].source ""
 classes[].fields[].name "str_3_dyn"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "str_3_dyn"
+classes[].fields[].source "str_3"
 classes[].fields[].name "arr_3_dyn"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "arr_3"
 classes[].fields[].name "str_4_bold"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "str_4_bold"
+classes[].fields[].source "str_4"
 classes[].fields[].name "arr_4_bold"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "arr_4"
@@ -53,13 +53,13 @@ classes[].name "dyn"
 classes[].omitsummaryfeatures false
 classes[].fields[].name "str_3_dyn"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "str_3_dyn"
+classes[].fields[].source "str_3"
 classes[].fields[].name "arr_3_dyn"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "arr_3"
 classes[].fields[].name "str_4_bold"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "str_4_bold"
+classes[].fields[].source "str_4"
 classes[].fields[].name "arr_4_bold"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "arr_4"
diff --git a/config-model/src/test/derived/multiplesummaries/summary.cfg b/config-model/src/test/derived/multiplesummaries/summary.cfg
index 72543b46c8e..0b2be2b3ab1 100644
--- a/config-model/src/test/derived/multiplesummaries/summary.cfg
+++ b/config-model/src/test/derived/multiplesummaries/summary.cfg
@@ -26,7 +26,7 @@ classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "d"
 classes[].fields[].name "dynamice"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "dynamice"
+classes[].fields[].source "e"
 classes[].fields[].name "f"
 classes[].fields[].command ""
 classes[].fields[].source ""
@@ -47,7 +47,7 @@ classes[].fields[].command ""
 classes[].fields[].source ""
 classes[].fields[].name "adynamic2"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "adynamic2"
+classes[].fields[].source "a"
 classes[].fields[].name "alltags"
 classes[].fields[].command ""
 classes[].fields[].source ""
@@ -59,10 +59,10 @@ classes[].fields[].command ""
 classes[].fields[].source ""
 classes[].fields[].name "abolded2"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "abolded2"
+classes[].fields[].source "a"
 classes[].fields[].name "aboldeddynamic"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "aboldeddynamic"
+classes[].fields[].source "a"
 classes[].fields[].name "documentid"
 classes[].fields[].command "documentid"
 classes[].fields[].source ""
@@ -131,7 +131,7 @@ classes[].name "anothernotattributesonly2"
 classes[].omitsummaryfeatures false
 classes[].fields[].name "adynamic2"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "adynamic2"
+classes[].fields[].source "a"
 classes[].fields[].name "c"
 classes[].fields[].command "attribute"
 classes[].fields[].source "c"
@@ -209,7 +209,7 @@ classes[].name "notattributesonly4"
 classes[].omitsummaryfeatures false
 classes[].fields[].name "abolded2"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "abolded2"
+classes[].fields[].source "a"
 classes[].fields[].name "c"
 classes[].fields[].command "attribute"
 classes[].fields[].source "c"
@@ -224,7 +224,7 @@ classes[].name "notattributesonly5"
 classes[].omitsummaryfeatures false
 classes[].fields[].name "aboldeddynamic"
 classes[].fields[].command "dynamicteaser"
-classes[].fields[].source "aboldeddynamic"
+classes[].fields[].source "a"
 classes[].fields[].name "c"
 classes[].fields[].command "attribute"
 classes[].fields[].source "c"
diff --git a/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java b/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java
index 2071d142da9..b5ebefdbd9c 100644
--- a/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java
+++ b/config-model/src/test/java/com/yahoo/schema/derived/SummaryTestCase.java
@@ -77,11 +77,11 @@ public class SummaryTestCase extends AbstractSchemaTestCase {
         assertSummaryField("exact", SummaryClassField.Type.LONGSTRING, fields.next());
         assertSummaryField("title", SummaryClassField.Type.LONGSTRING, fields.next());
         assertSummaryField("description", SummaryClassField.Type.LONGSTRING, fields.next());
-        assertSummaryField("dyndesc", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "dyndesc", fields.next());
+        assertSummaryField("dyndesc", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "description", fields.next());
         assertSummaryField("longdesc", SummaryClassField.Type.LONGSTRING, fields.next());
         assertSummaryField("longstat", SummaryClassField.Type.LONGSTRING, fields.next());
-        assertSummaryField("dynlong", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "dynlong", fields.next());
-        assertSummaryField("dyndesc2", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "dyndesc2", fields.next());
+        assertSummaryField("dynlong", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "longdesc", fields.next());
+        assertSummaryField("dyndesc2", SummaryClassField.Type.LONGSTRING, "dynamicteaser", "longdesc", fields.next());
         assertSummaryField("measurement", SummaryClassField.Type.INTEGER, "attribute", "measurement", fields.next());
         assertSummaryField("rankfeatures", SummaryClassField.Type.FEATUREDATA, "rankfeatures", fields.next());
         assertSummaryField("summaryfeatures", SummaryClassField.Type.FEATUREDATA, "summaryfeatures", fields.next());
diff --git a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
index 398404c6e71..596525e17d7 100644
--- a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
+++ b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.cpp
@@ -240,14 +240,18 @@ DocsumStoreVsmDocument::insert_juniper_field(const vespalib::string& field_name,
     auto field_value = get_field_value(field_name);
     if (field_value) {
         FieldModifier* modifier = nullptr;
-        if (is_struct_or_multivalue_field_type(*field_value->getDataType())) {
-            auto entry_idx = _result_class.getIndexFromName(field_name.c_str());
-            if (entry_idx >= 0) {
-                assert((uint32_t) entry_idx < _result_class.getNumEntries());
+        auto entry_idx = _result_class.getIndexFromName(field_name.c_str());
+        if (entry_idx >= 0) {
+            assert((uint32_t) entry_idx < _result_class.getNumEntries());
+            if (is_struct_or_multivalue_field_type(*field_value->getDataType())) {
                 modifier = _docsum_filter.get_field_modifier(entry_idx);
+            } else {
+                if (!_docsum_filter.has_flatten_juniper_command(entry_idx)) {
+                    modifier = _docsum_filter.get_field_modifier(entry_idx);
+                } else {
+                    // Markup for juniper has already been added due to FLATTENJUNIPER command in vsm summary config.
+                }
             }
-        } else {
-            // Markup for juniper has already been added due to FLATTENJUNIPER command in vsm summary config.
         }
         SnippetModifierJuniperConverter string_converter(converter, modifier);
         SlimeFiller::insert_juniper_field(*field_value, inserter, string_converter);
@@ -407,6 +411,14 @@ DocsumFilter::insert_summary_field(uint32_t entry_idx, const Document& doc, vesp
     _flattenWriter.clear();
 }
 
+bool
+DocsumFilter::has_flatten_juniper_command(uint32_t entry_idx) const
+{
+    const auto& field_spec = _fields[entry_idx];
+    auto command = field_spec.getCommand();
+    return command == VsmsummaryConfig::Fieldmap::Command::FLATTENJUNIPER;
+}
+
 FieldModifier*
 DocsumFilter::get_field_modifier(uint32_t entry_idx)
 {
diff --git a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.h b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.h
index e87a3e9a431..584f7c8141e 100644
--- a/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.h
+++ b/streamingvisitors/src/vespa/vsm/vsm/docsumfilter.h
@@ -67,6 +67,7 @@ public:
 
     search::docsummary::DocsumStoreFieldValue get_summary_field(uint32_t entry_idx, const Document& doc);
     void insert_summary_field(uint32_t entry_idx, const Document& doc, vespalib::slime::Inserter& inserter);
+    bool has_flatten_juniper_command(uint32_t entry_idx) const;
     FieldModifier* get_field_modifier(uint32_t entry_idx);
 };