Allow dynamic transforms on array<string> summary fields.

6 files changed, 86 insertions, 29 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 b8aa89fc82a..b9355693da8 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
@@ -5,6 +5,7 @@ import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.document.DataType;
 import com.yahoo.prelude.fastsearch.DocsumDefinitionSet;
 import com.yahoo.schema.Schema;
+import com.yahoo.schema.processing.DynamicSummaryTransformUtils;
 import com.yahoo.vespa.config.search.SummaryConfig;
 import com.yahoo.vespa.documentmodel.DocumentSummary;
 import com.yahoo.vespa.documentmodel.SummaryField;
@@ -158,14 +159,10 @@ public class SummaryClass extends Derived {
                 summaryField.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER)
         {
             return summaryField.getSingleSource();
+        } else if (summaryField.getTransform().isDynamic()) {
+            return DynamicSummaryTransformUtils.getSource(summaryField);
         } else {
-            // Note: Currently source mapping is handled in the indexing statement,
-            // by creating a summary field for each of the values
-            // This works, but is suboptimal. We could consolidate to a minimal set and
-            // use the right value from the minimal set as the third parameter here,
-            // and add "override" commands to multiple static values
-            boolean useFieldNameAsArgument = summaryField.getTransform().isDynamic();
-            return useFieldNameAsArgument ? summaryField.getName() : "";
+            return "";
         }
     }
 
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java b/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java
index 3afc25131c0..77a17878840 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/AddExtraFieldsToDocument.java
@@ -10,6 +10,7 @@ import com.yahoo.schema.document.ImmutableSDField;
 import com.yahoo.schema.document.SDDocumentType;
 import com.yahoo.schema.document.SDField;
 import com.yahoo.vespa.documentmodel.SummaryField;
+import com.yahoo.vespa.documentmodel.SummaryTransform;
 import com.yahoo.vespa.model.container.search.QueryProfiles;
 
 /**
@@ -33,15 +34,14 @@ public class AddExtraFieldsToDocument extends Processor {
             }
             for (var docsum : schema.getSummaries().values()) {
                 for (var summaryField : docsum.getSummaryFields().values()) {
-                    switch (summaryField.getTransform()) {
-                    case NONE:
-                    case BOLDED:
-                    case DYNAMICBOLDED:
-                    case DYNAMICTEASER:
-                    case DOCUMENT_ID: // TODO: Adding the 'documentid' field should no longer be needed when the docsum framework in the backend has been simplified and the transform is always used.
+                    var transform = summaryField.getTransform();
+                    if (transform.isDynamic() && DynamicSummaryTransformUtils.summaryFieldIsRequiredInDocumentType(summaryField) ||
+                            transform == SummaryTransform.NONE ||
+                            transform == SummaryTransform.DOCUMENT_ID)
+                    {
+                        // TODO: Adding the 'documentid' field should no longer be needed when the docsum framework in the backend has been simplified and the transform is always used.
                         addSummaryField(schema, document, summaryField, validate);
-                        break;
-                    default:
+                    } else {
                         // skip: generated from attribute or similar,
                         // so does not need to be included as an extra
                         // field in the document type
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java b/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java
index 53a3d462d54..73ad4225c88 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/Bolding.java
@@ -3,7 +3,6 @@ package com.yahoo.schema.processing;
 
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.schema.RankProfileRegistry;
-import com.yahoo.document.DataType;
 import com.yahoo.schema.document.ImmutableSDField;
 import com.yahoo.schema.Schema;
 import com.yahoo.vespa.documentmodel.SummaryField;
@@ -26,16 +25,12 @@ public class Bolding extends Processor {
         if ( ! validate) return;
         for (ImmutableSDField field : schema.allConcreteFields()) {
             for (SummaryField summary : field.getSummaryFields().values()) {
-                if (summary.getTransform().isBolded() &&
-                    !((summary.getDataType() == DataType.STRING) || (summary.getDataType() == DataType.URI)))
-                {
+                if (summary.getTransform().isBolded() && !DynamicSummaryTransformUtils.hasSupportedType(summary)) {
                     throw new IllegalArgumentException("'bolding: on' for non-text field " +
                                                        "'" + field.getName() + "'" +
                                                        " (" + summary.getDataType() + ")" +
                                                        " is not allowed");
-                } else if (summary.getTransform().isDynamic() &&
-                           !((summary.getDataType() == DataType.STRING) || (summary.getDataType() == DataType.URI)))
-                {
+                } else if (summary.getTransform().isDynamic() && !DynamicSummaryTransformUtils.hasSupportedType(summary)) {
                     throw new IllegalArgumentException("'summary: dynamic' for non-text field " +
                                                        "'" + field.getName() + "'" +
                                                        " (" + summary.getDataType() + ")" +
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
new file mode 100644
index 00000000000..20597eca64f
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/schema/processing/DynamicSummaryTransformUtils.java
@@ -0,0 +1,59 @@
+package com.yahoo.schema.processing;
+
+import com.yahoo.document.DataType;
+import com.yahoo.vespa.documentmodel.SummaryField;
+
+/**
+ * This class contains utils used when handling summary fields with dynamic transforms during processing and deriving.
+ *
+ * Originally (before Vespa 8.52), dynamic transforms where only supported for string fields.
+ * Due to legacy functionality in the backend docsum framework,
+ * such summary fields are in some cases added as extra document fields and populated in indexing scripts.
+ * This is something we want to avoid in the future, but it might not be entirely possible before Vespa 9.
+ *
+ * With the introduction of dynamic transform for array of string fields,
+ * we move in the right direction and avoid adding extra document fields with indexing script population for this type.
+ * Instead, we configure the dynamic transform in the backend to use the original source field directly.
+ *
+ * See SummaryTransform.isDynamic() for which transforms this applies to.
+ */
+public class DynamicSummaryTransformUtils {
+
+    public static boolean hasSupportedType(SummaryField field) {
+        return isSupportedType(field.getDataType());
+    }
+
+    public static boolean isSupportedType(DataType type) {
+        return isOriginalSupportedType(type) || isNewSupportedType(type);
+    }
+
+    private static boolean isOriginalSupportedType(DataType type) {
+        return (type == DataType.STRING) ||
+                (type == DataType.URI);
+    }
+
+    private static boolean isNewSupportedType(DataType type) {
+        return (type.equals(DataType.getArray(DataType.STRING)));
+    }
+
+    /**
+     * Whether a summary field must be populated by the source field with the given type in an indexing script.
+     */
+    public static boolean summaryFieldIsPopulatedBySourceField(DataType sourceFieldType) {
+        return isOriginalSupportedType(sourceFieldType);
+    }
+
+    /**
+     * Whether a summary field is required as an extra field in the document type.
+     */
+    public static boolean summaryFieldIsRequiredInDocumentType(SummaryField summaryField) {
+        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();
+    }
+}
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java b/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java
index ea65a223686..2c24d3e53e1 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/IndexingOutputs.java
@@ -68,11 +68,13 @@ public class IndexingOutputs extends Processor {
         }
         if (summaryTransform.isDynamic()) {
             DataType fieldType = field.getDataType();
-            if (fieldType != DataType.URI && fieldType != DataType.STRING) {
-                warn(schema, field, "Dynamic summaries are only supported for fields of type " +
-                                    "string, ignoring summary field '" + summaryField.getName() +
-                                    "' for sd field '" + field.getName() + "' of type " +
-                                    fieldType.getName() + ".");
+            if (!DynamicSummaryTransformUtils.summaryFieldIsPopulatedBySourceField(fieldType)) {
+                if (!DynamicSummaryTransformUtils.isSupportedType(fieldType)) {
+                    warn(schema, field, "Dynamic summaries are only supported for fields of type " +
+                            "string and array<string>, ignoring summary field '" + summaryField.getName() +
+                            "' for sd field '" + field.getName() + "' of type " +
+                            fieldType.getName() + ".");
+                }
                 return;
             }
             dynamicSummary.add(summaryName);
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java b/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java
index ed1f47611eb..a899f5e82ab 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/SummaryDynamicStructsArrays.java
@@ -13,7 +13,7 @@ import com.yahoo.vespa.model.container.search.QueryProfiles;
 
 /**
  * Fail if:
- * An SD field explicitly says summary:dynamic , but the field is wset, array or struct.
+ * An SD field explicitly says summary:dynamic , but the field is non-string array, wset, or struct.
  * If there is an explicitly defined summary class, saying dynamic in one of its summary
  * fields is always legal.
  *
@@ -31,7 +31,7 @@ public class SummaryDynamicStructsArrays extends Processor {
 
         for (SDField field : schema.allConcreteFields()) {
             DataType type = field.getDataType();
-            if (type instanceof ArrayDataType || type instanceof WeightedSetDataType || type instanceof StructDataType) {
+            if (isNonStringArray(type) || type instanceof WeightedSetDataType || type instanceof StructDataType) {
                 for (SummaryField sField : field.getSummaryFields().values()) {
                     if (sField.getTransform().equals(SummaryTransform.DYNAMICTEASER)) {
                         throw new IllegalArgumentException("For field '"+field.getName()+"': dynamic summary is illegal " +
@@ -44,4 +44,8 @@ public class SummaryDynamicStructsArrays extends Processor {
         }
     }
 
+    private boolean isNonStringArray(DataType type) {
+        return (type instanceof ArrayDataType) && (!type.equals(DataType.getArray(DataType.STRING)));
+    }
+
 }