Repository: vespa-engine/vespa (language: java)

Issue:
Add validation for struct-field indexing statement: index is not supported
Example from [documentation ](https://docs.vespa.ai/en/reference/schema-reference.html#type:map)

```
struct person {
    field first_name type string {}
    field last_name type string {}
}
field identities type map<string, person> {
    indexing: summary
    struct-field key { indexing: attribute }
    struct-field value.last_name { indexing: attribute }
}
```
This is fine, but I have seen reports of vespa accepting indexing: index [which is not supported](https://docs.vespa.ai/en/reference/schema-reference.html#struct-field)

```
field identities type map<string, person> {
    indexing: summary
    struct-field key { indexing: attribute|index}
    struct-field value.last_name { indexing: index }
}
```








Discussion / hints:
Specifying _index_ is used by streaming search to signal whether the field is searchable. Validation must be added at a higher level where the document mode (streaming vs indexed) is known.

Fix patch:
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 335942de99db..221280dc1e42 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
@@ -82,6 +82,11 @@ public boolean wasConfiguredToDoAttributing() {
         return importedField.targetField().wasConfiguredToDoAttributing();
     }
 
+    @Override
+    public boolean wasConfiguredToDoIndexing() {
+        return importedField.targetField().wasConfiguredToDoIndexing();
+    }
+
     @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 44e442811ba9..37cbb3a81716 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
@@ -45,6 +45,14 @@ public interface ImmutableSDField {
      */
     boolean wasConfiguredToDoAttributing();
 
+    /**
+     * Whether this field at some time was configured to do indexing.
+     *
+     * This function can typically return a different value than doesIndexing(),
+     * which uses the final state of the underlying indexing script instead.
+     */
+    boolean wasConfiguredToDoIndexing();
+
     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 943d6c6fc142..2d79d89a2a02 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
@@ -117,6 +117,7 @@ public class SDField extends Field implements TypedKey, ImmutableSDField {
     private boolean isExtraField = false;
 
     private boolean wasConfiguredToDoAttributing = false;
+    private boolean wasConfiguredToDoIndexing = false;
 
     /**
      * Creates a new field. This method is only used to create reserved fields.
@@ -381,6 +382,11 @@ public boolean wasConfiguredToDoAttributing() {
         return wasConfiguredToDoAttributing;
     }
 
+    @Override
+    public boolean wasConfiguredToDoIndexing() {
+        return wasConfiguredToDoIndexing;
+    }
+
     /** 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());
@@ -408,6 +414,9 @@ public void setIndexingScript(ScriptExpression exp) {
         if (!wasConfiguredToDoAttributing()) {
             wasConfiguredToDoAttributing = doesAttributing();
         }
+        if (!wasConfiguredToDoIndexing()) {
+            wasConfiguredToDoIndexing = doesIndexing();
+        }
         if (!usesStructOrMap()) {
             new ExpressionVisitor() {
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
similarity index 95%
rename from config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java
rename to config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
index 01bf846d4cb2..8515c34a3773 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldAttributesValidator.java
@@ -22,7 +22,7 @@
  *
  * @author geirst
  */
-public class ComplexAttributeFieldsValidator extends Validator {
+public class ComplexFieldsWithStructFieldAttributesValidator extends Validator {
 
     @Override
     public void validate(VespaModel model, DeployState deployState) {
@@ -39,7 +39,7 @@ public void validate(VespaModel model, DeployState deployState) {
     private static void validateComplexFields(String clusterName, Schema schema) {
         String unsupportedFields = schema.allFields()
                                          .filter(field -> isUnsupportedComplexField(field))
-                                         .map(ComplexAttributeFieldsValidator::toString)
+                                         .map(ComplexFieldsWithStructFieldAttributesValidator::toString)
                                          .collect(Collectors.joining(", "));
 
         if (!unsupportedFields.isEmpty()) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java
new file mode 100644
index 000000000000..bbbb0c08bf42
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/ComplexFieldsWithStructFieldIndexesValidator.java
@@ -0,0 +1,72 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.model.application.validation;
+
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.schema.Schema;
+import com.yahoo.schema.document.ImmutableSDField;
+import com.yahoo.vespa.model.VespaModel;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Validates that complex fields (of type struct or map) do not have any struct fields with 'indexing: index'.
+ * This is not supported and will confuse the user if not validated.
+ *
+ * Only applies for indexed search clusters.
+ *
+ * @author geirst
+ */
+public class ComplexFieldsWithStructFieldIndexesValidator extends Validator {
+
+    @Override
+    public void validate(VespaModel model, DeployState deployState) {
+        for (var cluster : model.getSearchClusters()) {
+            if (cluster.isStreaming()) {
+                continue;
+            }
+            for (var spec : cluster.schemas().values()) {
+                validateComplexFields(cluster.getClusterName(), spec.fullSchema());
+            }
+        }
+    }
+
+    private static void validateComplexFields(String clusterName, Schema schema) {
+        String unsupportedFields = schema.allFields()
+                .filter(field -> hasStructFieldsWithIndex(field))
+                .map(ComplexFieldsWithStructFieldIndexesValidator::toString)
+                .collect(Collectors.joining(", "));
+
+        if (!unsupportedFields.isEmpty()) {
+            throw new IllegalArgumentException(
+                    String.format("For cluster '%s', schema '%s': The following complex fields have struct fields with 'indexing: index' which is not supported: %s. " +
+                                  "Change to 'indexing: attribute' instead",
+                                  clusterName, schema.getName(), unsupportedFields));
+        }
+    }
+
+    private static boolean hasStructFieldsWithIndex(ImmutableSDField field) {
+        return (field.usesStructOrMap() && hasStructFieldsWithIndex(field.getStructFields()));
+    }
+
+    private static String toString(ImmutableSDField field) {
+        return field.getName() + " (" + String.join(", ", getStructFieldsWithIndex(field.getStructFields())) + ")";
+    }
+
+    private static boolean hasStructFieldsWithIndex(Collection<? extends ImmutableSDField> structFields) {
+        return !getStructFieldsWithIndex(structFields).isEmpty();
+    }
+
+    private static List<String> getStructFieldsWithIndex(Collection<? extends ImmutableSDField> structFields) {
+        var result = new ArrayList<String>();
+        for (var structField : structFields) {
+            if (structField.wasConfiguredToDoIndexing()) {
+                result.add(structField.getName());
+            }
+            result.addAll(getStructFieldsWithIndex(structField.getStructFields()));
+        }
+        return result;
+    }
+}
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
index ce61d3edc3bc..dab1eeccc965 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/Validation.java
@@ -71,7 +71,8 @@ public List<ConfigChangeAction> validate(VespaModel model, ValidationParameters
         new SchemasDirValidator().validate(model, deployState);
         new BundleValidator().validate(model, deployState);
         new SearchDataTypeValidator().validate(model, deployState);
-        new ComplexAttributeFieldsValidator().validate(model, deployState);
+        new ComplexFieldsWithStructFieldAttributesValidator().validate(model, deployState);
+        new ComplexFieldsWithStructFieldIndexesValidator().validate(model, deployState);
         new StreamingValidator().validate(model, deployState);
         new RankSetupValidator(validationParameters.ignoreValidationErrors()).validate(model, deployState);
         new NoPrefixForIndexes().validate(model, deployState);


Test patch:
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
similarity index 81%
rename from config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java
rename to config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
index 85050aa0cf90..11e4ae855a5c 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexAttributeFieldsValidatorTestCase.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/ComplexFieldsValidatorTestCase.java
@@ -22,7 +22,7 @@
 /**
  * @author geirst
  */
-public class ComplexAttributeFieldsValidatorTestCase {
+public class ComplexFieldsValidatorTestCase {
 
     @SuppressWarnings("deprecation")
     @Rule
@@ -77,6 +77,30 @@ private String getExpectedMessage(String unsupportedFields) {
                 "Only supported for the following complex field types: array or map of struct with primitive types, map of primitive types";
     }
 
+    @Test
+    public void throws_when_complex_fields_have_struct_fields_with_index() throws IOException, SAXException {
+        exceptionRule.expect(IllegalArgumentException.class);
+        exceptionRule.expectMessage("For cluster 'mycluster', schema 'test': " +
+                "The following complex fields have struct fields with 'indexing: index' which is not supported: " +
+                "topics (topics.id, topics.label). Change to 'indexing: attribute' instead");
+        createModelAndValidate(joinLines(
+                "schema test {",
+                "document test {",
+                "struct topic {",
+                "  field id type string {}",
+                "  field label type string {}",
+                "  field desc type string {}",
+                "}",
+                "field topics type array<topic> {",
+                "  indexing: summary",
+                "  struct-field id { indexing: index }",
+                "  struct-field label { indexing: index | attribute }",
+                "  struct-field desc { indexing: attribute }",
+                "}",
+                "}",
+                "}"));
+    }
+
     @Test
     public void validation_passes_when_only_supported_struct_field_attributes_are_used() throws IOException, SAXException {
         createModelAndValidate(joinLines("search test {",