Use "copy" summary transform when field is only defined as summary field.

5 files changed, 107 insertions, 11 deletions

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 1dd6ead00a4..52652de81d4 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
@@ -13,6 +13,9 @@ import com.yahoo.vespa.documentmodel.SummaryField;
 import com.yahoo.vespa.documentmodel.SummaryTransform;
 import com.yahoo.vespa.model.container.search.QueryProfiles;
 
+import java.util.LinkedHashSet;
+import java.util.Set;
+
 /**
  * This processor creates a {@link com.yahoo.schema.document.SDDocumentType} for each {@link Schema}
  * object which holds all the data that search
@@ -21,6 +24,8 @@ import com.yahoo.vespa.model.container.search.QueryProfiles;
  */
 public class AddExtraFieldsToDocument extends Processor {
 
+    Set<String> extraSummaryFields = new LinkedHashSet<String>();
+
     AddExtraFieldsToDocument(Schema schema, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) {
         super(schema, deployLogger, rankProfileRegistry, queryProfiles);
     }
@@ -48,6 +53,19 @@ public class AddExtraFieldsToDocument extends Processor {
                     }
                 }
             }
+            /*
+             * Don't use extra summary fields when generating summaries. They will not be created nor populated by
+             * future vespa versions. When vespa version 'X' stops using these fields and vespa version 'Y' stops
+             * populating the fields, rollback from vespa version >= 'Y' to vespa version < 'X' will break (missing
+             * summary fields).
+             */
+            for (var docsum : schema.getSummaries().values()) {
+                for (var summaryField : docsum.getSummaryFields().values()) {
+                    if (extraSummaryFields.contains(summaryField.getName())) {
+                        considerCopyTransformForExtraSummaryField(schema, summaryField);
+                    }
+                }
+            }
         }
     }
 
@@ -70,6 +88,7 @@ public class AddExtraFieldsToDocument extends Processor {
             if (existingField == null) {
                 SDField newField = new SDField(document, field.getName(), field.getDataType());
                 newField.setIsExtraField(true);
+                extraSummaryFields.add(field.getName());
                 document.addField(newField);
             } else if (!existingField.isImportedField()) {
                 document.addField(existingField.asField());
@@ -87,4 +106,16 @@ public class AddExtraFieldsToDocument extends Processor {
         }
         document.addField(field);
     }
+
+    private void considerCopyTransformForExtraSummaryField(Schema schema, SummaryField field) {
+        if (field.getTransform() == SummaryTransform.NONE && field.hasExplicitSingleSource() &&
+                fieldIsExtraSummaryField(schema, field.getName())) {
+            field.setTransform(SummaryTransform.COPY);
+        }
+    }
+
+    private boolean fieldIsExtraSummaryField(Schema schema, String name) {
+        var field = schema.getConcreteField(name);
+        return field == null || extraSummaryFields.contains(name);
+    }
 }
diff --git a/config-model/src/test/derived/multiplesummaries/summary.cfg b/config-model/src/test/derived/multiplesummaries/summary.cfg
index 0b2be2b3ab1..cb53fdc4577 100644
--- a/config-model/src/test/derived/multiplesummaries/summary.cfg
+++ b/config-model/src/test/derived/multiplesummaries/summary.cfg
@@ -49,14 +49,14 @@ classes[].fields[].name "adynamic2"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "a"
 classes[].fields[].name "alltags"
-classes[].fields[].command ""
-classes[].fields[].source ""
+classes[].fields[].command "copy"
+classes[].fields[].source "mytags"
 classes[].fields[].name "sometags"
 classes[].fields[].command "matchedelementsfilter"
 classes[].fields[].source "mytags"
 classes[].fields[].name "anotherb"
-classes[].fields[].command ""
-classes[].fields[].source ""
+classes[].fields[].command "copy"
+classes[].fields[].source "b"
 classes[].fields[].name "abolded2"
 classes[].fields[].command "dynamicteaser"
 classes[].fields[].source "a"
@@ -136,8 +136,8 @@ classes[].fields[].name "c"
 classes[].fields[].command "attribute"
 classes[].fields[].source "c"
 classes[].fields[].name "alltags"
-classes[].fields[].command ""
-classes[].fields[].source ""
+classes[].fields[].command "copy"
+classes[].fields[].source "mytags"
 classes[].fields[].name "sometags"
 classes[].fields[].command "matchedelementsfilter"
 classes[].fields[].source "mytags"
@@ -145,8 +145,8 @@ classes[].fields[].name "anothera"
 classes[].fields[].command "attribute"
 classes[].fields[].source "a"
 classes[].fields[].name "anotherb"
-classes[].fields[].command ""
-classes[].fields[].source ""
+classes[].fields[].command "copy"
+classes[].fields[].source "b"
 classes[].fields[].name "rankfeatures"
 classes[].fields[].command "rankfeatures"
 classes[].fields[].source ""
diff --git a/config-model/src/test/derived/streamingstruct/summary.cfg b/config-model/src/test/derived/streamingstruct/summary.cfg
index f648eca0a86..f70f1209a50 100644
--- a/config-model/src/test/derived/streamingstruct/summary.cfg
+++ b/config-model/src/test/derived/streamingstruct/summary.cfg
@@ -7,8 +7,8 @@ classes[].fields[].name "coupleof"
 classes[].fields[].command ""
 classes[].fields[].source ""
 classes[].fields[].name "anothersummaryfield"
-classes[].fields[].command ""
-classes[].fields[].source ""
+classes[].fields[].command "copy"
+classes[].fields[].source "normalfields"
 classes[].fields[].name "a"
 classes[].fields[].command ""
 classes[].fields[].source ""
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 5e02f63ec39..a27bc824b45 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
@@ -80,7 +80,7 @@ public class SummaryTestCase extends AbstractSchemaTestCase {
         assertSummaryField("description", SummaryClassField.Type.LONGSTRING, 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("longstat", SummaryClassField.Type.LONGSTRING, "copy", "longdesc", 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());
diff --git a/config-model/src/test/java/com/yahoo/schema/processing/AddExtraFieldsToDocumentTest.java b/config-model/src/test/java/com/yahoo/schema/processing/AddExtraFieldsToDocumentTest.java
new file mode 100644
index 00000000000..43b403c42c6
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/schema/processing/AddExtraFieldsToDocumentTest.java
@@ -0,0 +1,65 @@
+// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.schema.processing;
+
+import com.yahoo.document.DataType;
+import com.yahoo.schema.ApplicationBuilder;
+import com.yahoo.schema.Schema;
+import com.yahoo.schema.parser.ParseException;
+import com.yahoo.vespa.documentmodel.SummaryTransform;
+import com.yahoo.vespa.model.test.utils.DeployLoggerStub;
+import org.junit.jupiter.api.Test;
+
+import static com.yahoo.config.model.test.TestUtil.joinLines;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/*
+ * Test AddExtraFieldsToDocument processor.
+ */
+public class AddExtraFieldsToDocumentTest {
+
+    @Test
+    public void testCopyTransformForExtraSummaryFields() throws ParseException {
+        String sd = joinLines("schema test {",
+                "  document test {",
+                "    field a type string { indexing: index }",
+                "    field b type int { }",
+                "    field c type long { indexing: attribute }",
+                "  }",
+                "  document-summary foo {",
+                "    summary my_a { source: a }",
+                "    summary my_b { source: b }",
+                "    summary my_c { source: c }",
+                "    from-disk",
+                "  }",
+                "}");
+        DeployLoggerStub logger = new DeployLoggerStub();
+        var builder = ApplicationBuilder.createFromStrings(logger, sd);
+        var schema = builder.getSchema();
+        assertTrue(logger.entries.isEmpty());
+        // Don't use extra fields when generating summary.
+        assertSummary(schema, "foo", "my_a", SummaryTransform.COPY, "a");
+        assertSummary(schema, "foo", "my_b", SummaryTransform.COPY, "b");
+        assertSummary(schema, "foo", "my_c", SummaryTransform.ATTRIBUTE, "c");
+        // Extra fields should still be created
+        assertField(schema, "my_a", DataType.STRING);
+        assertField(schema,"my_b", DataType.INT);
+        assertNull(schema.getDocument().getField("my_c"));
+    }
+
+    private void assertSummary(Schema schema, String dsName, String name, SummaryTransform transform, String source) {
+        var docsum = schema.getSummary(dsName);
+        var field = docsum.getSummaryField(name);
+        assertEquals(transform, field.getTransform());
+        assertEquals(source, field.getSingleSource());
+    }
+
+    private void assertField(Schema schema, String name, DataType type) {
+        var field = schema.getDocument().getField(name);
+        assertNotNull(field);
+        assertEquals(field.getDataType(), type);
+    }
+
+}