Do not log disk access warning when using an imported complex field in summary.

6 files changed, 99 insertions, 12 deletions

diff --git a/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java b/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
index 993bf16405a..993c9180f78 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/ComplexAttributeFieldUtils.java
@@ -95,29 +95,22 @@ public class ComplexAttributeFieldUtils {
         if (isArrayOfSimpleStruct(field)) {
             return hasOnlyStructFieldAttributes(field);
         } else if (isMapOfSimpleStruct(field)) {
-            return hasSingleAttribute(field.getStructField("key")) &&
+            return (field.getStructField("key").hasSingleAttribute()) &&
                     hasOnlyStructFieldAttributes(field.getStructField("value"));
         } else if (isMapOfPrimitiveType(field)) {
-            return hasSingleAttribute(field.getStructField("key")) &&
-                    hasSingleAttribute(field.getStructField("value"));
+            return (field.getStructField("key").hasSingleAttribute() &&
+                    field.getStructField("value").hasSingleAttribute());
         }
         return false;
     }
 
     private static boolean hasOnlyStructFieldAttributes(ImmutableSDField field) {
         for (ImmutableSDField structField : field.getStructFields()) {
-            if (!hasSingleAttribute(structField)) {
+            if (!structField.hasSingleAttribute()) {
                 return false;
             }
         }
         return true;
     }
 
-    private static boolean hasSingleAttribute(ImmutableSDField field) {
-        if (field.getAttributes().size() != 1) {
-            return false;
-        }
-        return (field.getAttributes().get(field.getName()) != null);
-    }
-
 }
diff --git a/config-model/src/main/java/com/yahoo/schema/document/ImmutableImportedSDField.java b/config-model/src/main/java/com/yahoo/schema/document/ImmutableImportedSDField.java
index 221280dc1e4..4f0b684a9c0 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/ImmutableImportedSDField.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/ImmutableImportedSDField.java
@@ -88,6 +88,15 @@ public class ImmutableImportedSDField implements ImmutableSDField {
     }
 
     @Override
+    public boolean hasSingleAttribute() {
+        if (getAttributes().size() != 1) {
+            return false;
+        }
+        // Must use the name of the target field as the attributes also exist on the target field.
+        return (getAttributes().get(importedField.targetField().getName()) != null);
+    }
+
+    @Override
     public DataType getDataType() {
         return importedField.targetField().getDataType();
     }
diff --git a/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java b/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java
index 37cbb3a8171..2d826e164b7 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/ImmutableSDField.java
@@ -53,6 +53,11 @@ public interface ImmutableSDField {
      */
     boolean wasConfiguredToDoIndexing();
 
+    /**
+     * Returns whether this field has a single attribute with the same name as this field.
+     */
+    boolean hasSingleAttribute();
+
     DataType getDataType();
 
     Index getIndex(String name);
diff --git a/config-model/src/main/java/com/yahoo/schema/document/SDField.java b/config-model/src/main/java/com/yahoo/schema/document/SDField.java
index 2d79d89a2a0..4eaa3e05047 100644
--- a/config-model/src/main/java/com/yahoo/schema/document/SDField.java
+++ b/config-model/src/main/java/com/yahoo/schema/document/SDField.java
@@ -387,6 +387,14 @@ public class SDField extends Field implements TypedKey, ImmutableSDField {
         return wasConfiguredToDoIndexing;
     }
 
+    @Override
+    public boolean hasSingleAttribute() {
+        if (getAttributes().size() != 1) {
+            return false;
+        }
+        return (getAttributes().get(getName()) != null);
+    }
+
     /** Parse an indexing expression which will use the simple linguistics implementation suitable for testing */
     public void parseIndexingScript(String script) {
         parseIndexingScript(script, new SimpleLinguistics(), Embedder.throwsOnUse.asMap());
diff --git a/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java b/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java
index d96cd88f6be..d95b4ed4a33 100644
--- a/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java
+++ b/config-model/src/main/java/com/yahoo/schema/processing/AddAttributeTransformToSummaryOfImportedFields.java
@@ -52,7 +52,10 @@ public class AddAttributeTransformToSummaryOfImportedFields extends Processor {
     }
 
     private static void setAttributeCombinerTransform(SummaryField summaryField) {
-        if (summaryField.getTransform() == SummaryTransform.MATCHED_ELEMENTS_FILTER) {
+        if (summaryField.getTransform() == SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER) {
+            // This field already has the correct transform.
+            return;
+        } else if (summaryField.getTransform() == SummaryTransform.MATCHED_ELEMENTS_FILTER) {
             summaryField.setTransform(SummaryTransform.MATCHED_ATTRIBUTE_ELEMENTS_FILTER);
         } else {
             summaryField.setTransform(SummaryTransform.ATTRIBUTECOMBINER);
diff --git a/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java b/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java
new file mode 100644
index 00000000000..638fd6ebacf
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/schema/processing/SummaryDiskAccessValidatorTestCase.java
@@ -0,0 +1,69 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.schema.processing;
+
+import com.yahoo.schema.ApplicationBuilder;
+import com.yahoo.schema.derived.TestableDeployLogger;
+import com.yahoo.schema.parser.ParseException;
+import org.junit.jupiter.api.Test;
+
+import static com.yahoo.config.model.test.TestUtil.joinLines;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class SummaryDiskAccessValidatorTestCase {
+
+    @Test
+    void logs_warning_when_accessing_field_that_needs_disk_access() throws ParseException {
+        var sd = joinLines(
+                "schema test {",
+                "  document test {",
+                "    field str_map type map<string, string> {",
+                "      indexing: summary",
+                "      # Not all struct fields are attributes -> needs disk access",
+                "      struct-field key { indexing: attribute }",
+                "    }",
+                "  }",
+                "  document-summary my_sum {",
+                "    summary str_map type map<string, string> { source: str_map }",
+                "  }",
+                "}");
+
+        var logger = new TestableDeployLogger();
+        ApplicationBuilder.createFromString(sd, logger);
+        assertEquals(1, logger.warnings.size());
+        assertThat(logger.warnings.get(0),
+                containsString("In schema 'test', document summary 'my_sum': " +
+                        "Fields [str_map] references non-attribute fields: Using this summary will cause disk accesses"));
+    }
+
+    @Test
+    void does_not_log_warning_when_accessing_imported_map_field() throws ParseException {
+        var parent = joinLines(
+                "schema parent {",
+                "  document parent {",
+                "    field str_map type map<string, string> {",
+                "      indexing: summary",
+                "      # All struct fields must be attributes in order to import this fields",
+                "      struct-field key { indexing: attribute }",
+                "      struct-field value { indexing: attribute }",
+                "    }",
+                "  }",
+                "}");
+
+        var child = joinLines(
+                "schema child {",
+                "  document child {",
+                "    field ref type reference<parent> { indexing: attribute }",
+                "  }",
+                "  import field ref.str_map as ref_str_map {}",
+                "  document-summary my_sum {",
+                "    summary ref_str_map type map<string, string> { source: ref_str_map }",
+                "  }",
+                "}");
+        var logger = new TestableDeployLogger();
+        ApplicationBuilder.createFromStrings(logger, child, parent);
+        assertEquals(0, logger.warnings.size());
+    }
+
+}