Merge pull request #4373 from vespa-engine/geirst/fix-handling-of-imported-predicate-fields-in-config-model

Geirst/fix handling of imported predicate fields in config model

10 files changed, 163 insertions, 2 deletions

diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/document/ImmutableImportedSDField.java b/config-model/src/main/java/com/yahoo/searchdefinition/document/ImmutableImportedSDField.java
index a70e77a17a2..c8918f39834 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/document/ImmutableImportedSDField.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/document/ImmutableImportedSDField.java
@@ -79,7 +79,12 @@ public class ImmutableImportedSDField implements ImmutableSDField {
 
     @Override
     public Index getIndex(String name) {
-        throw createUnsupportedException();
+        if (!importedField.fieldName().equals(name)) {
+            throw new IllegalArgumentException("Getting an index (" + name + ") with different name than the imported field ("
+                    + importedField.fieldName() + ") is not supported");
+        }
+        String targetIndexName = importedField.targetField().getName();
+        return importedField.targetField().getIndex(targetIndexName);
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummayValidator.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummayValidator.java
new file mode 100644
index 00000000000..eaa85815736
--- /dev/null
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummayValidator.java
@@ -0,0 +1,63 @@
+package com.yahoo.searchdefinition.processing;
+
+import com.yahoo.config.application.api.DeployLogger;
+import com.yahoo.document.DataType;
+import com.yahoo.searchdefinition.RankProfileRegistry;
+import com.yahoo.searchdefinition.Search;
+import com.yahoo.searchdefinition.document.ImportedField;
+import com.yahoo.vespa.documentmodel.DocumentSummary;
+import com.yahoo.vespa.documentmodel.SummaryField;
+import com.yahoo.vespa.model.container.search.QueryProfiles;
+
+import java.util.Map;
+
+/**
+ * Validates that imported fields in document summaries are of supported types.
+ * Currently, predicate fields are NOT supported.
+ *
+ * @author geirst
+ */
+public class ImportedFieldsInSummayValidator extends Processor {
+
+
+    public ImportedFieldsInSummayValidator(Search search, DeployLogger deployLogger, RankProfileRegistry rankProfileRegistry, QueryProfiles queryProfiles) {
+        super(search, deployLogger, rankProfileRegistry, queryProfiles);
+    }
+
+    @Override
+    public void process() {
+        if (search.importedFields().isPresent()) {
+            validateDocumentSummaries(search.getSummaries());
+        }
+    }
+
+    private void validateDocumentSummaries(Map<String, DocumentSummary> summaries) {
+        for (DocumentSummary summary : summaries.values()) {
+            for (SummaryField field : summary.getSummaryFields()) {
+                ImportedField importedField = getImportedField(field);
+                if (importedField != null) {
+                    validateImportedSummaryField(summary, field, importedField);
+                }
+            }
+        }
+    }
+
+    private ImportedField getImportedField(SummaryField field) {
+        return search.importedFields().get().fields().get(field.getName());
+    }
+
+    private void validateImportedSummaryField(DocumentSummary summary, SummaryField field, ImportedField importedField) {
+        if (field.getDataType().equals(DataType.PREDICATE) &&
+                importedField.targetField().getDataType().equals(DataType.PREDICATE)) {
+            fail(summary, field, "Is of type predicate. Not supported in document summaries");
+        }
+    }
+
+    private void fail(DocumentSummary summary, SummaryField importedField, String msg) {
+        throw new IllegalArgumentException("For search '" + search.getName() + "', document summary '" + summary.getName() +
+                "', imported summary field '" + importedField.getName() + "': " + msg);
+    }
+}
+
+
+
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
index 26f98026d4f..90183848094 100644
--- a/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
+++ b/config-model/src/main/java/com/yahoo/searchdefinition/processing/Processing.java
@@ -73,6 +73,7 @@ public class Processing {
                 TensorFieldProcessor::new,
                 RankProfileTypeSettingsProcessor::new,
                 ReferenceFieldsProcessor::new,
+                ImportedFieldsInSummayValidator::new,
                 FastAccessValidator::new,
                 ReservedMacroNames::new,
 
diff --git a/config-model/src/test/derived/importedfields/attributes.cfg b/config-model/src/test/derived/importedfields/attributes.cfg
index 62fe0052bc3..f51bbc012b9 100644
--- a/config-model/src/test/derived/importedfields/attributes.cfg
+++ b/config-model/src/test/derived/importedfields/attributes.cfg
@@ -138,3 +138,23 @@ attribute[].upperbound 9223372036854775807
 attribute[].densepostinglistthreshold 0.4
 attribute[].tensortype ""
 attribute[].imported true
+attribute[].name "my_predicate_field"
+attribute[].datatype PREDICATE
+attribute[].collectiontype SINGLE
+attribute[].removeifzero false
+attribute[].createifnonexistent false
+attribute[].fastsearch false
+attribute[].huge false
+attribute[].sortascending true
+attribute[].sortfunction UCA
+attribute[].sortstrength PRIMARY
+attribute[].sortlocale ""
+attribute[].enablebitvectors false
+attribute[].enableonlybitvector false
+attribute[].fastaccess false
+attribute[].arity 8
+attribute[].lowerbound 5
+attribute[].upperbound 300
+attribute[].densepostinglistthreshold 0.4
+attribute[].tensortype ""
+attribute[].imported true
diff --git a/config-model/src/test/derived/importedfields/child.sd b/config-model/src/test/derived/importedfields/child.sd
index 703451c1293..07fa7d15719 100644
--- a/config-model/src/test/derived/importedfields/child.sd
+++ b/config-model/src/test/derived/importedfields/child.sd
@@ -9,6 +9,7 @@ search child {
   import field b_ref.string_field as my_string_field {}
   import field a_ref.int_array_field as my_int_array_field {}
   import field a_ref.int_wset_field as my_int_wset_field {}
+  import field a_ref.predicate_field as my_predicate_field {}
 
   fieldset myfieldset {
     fields: my_int_field, my_string_field
diff --git a/config-model/src/test/derived/importedfields/imported-fields.cfg b/config-model/src/test/derived/importedfields/imported-fields.cfg
index 3f2a083bdc2..59413ca5eb7 100644
--- a/config-model/src/test/derived/importedfields/imported-fields.cfg
+++ b/config-model/src/test/derived/importedfields/imported-fields.cfg
@@ -18,3 +18,8 @@ attribute[].referencefield "a_ref"
 attribute[].targetfield "int_wset_field"
 attribute[].datatype NONE
 attribute[].collectiontype SINGLE
+attribute[].name "my_predicate_field"
+attribute[].referencefield "a_ref"
+attribute[].targetfield "predicate_field"
+attribute[].datatype NONE
+attribute[].collectiontype SINGLE
diff --git a/config-model/src/test/derived/importedfields/index-info.cfg b/config-model/src/test/derived/importedfields/index-info.cfg
index 7cb53c480ce..adba0036409 100644
--- a/config-model/src/test/derived/importedfields/index-info.cfg
+++ b/config-model/src/test/derived/importedfields/index-info.cfg
@@ -49,6 +49,12 @@ indexinfo[].command[].indexname "my_int_wset_field"
 indexinfo[].command[].command "multivalue"
 indexinfo[].command[].indexname "my_int_wset_field"
 indexinfo[].command[].command "attribute"
+indexinfo[].command[].indexname "my_predicate_field"
+indexinfo[].command[].command "predicate-bounds [5..300]"
+indexinfo[].command[].indexname "my_predicate_field"
+indexinfo[].command[].command "index"
+indexinfo[].command[].indexname "my_predicate_field"
+indexinfo[].command[].command "attribute"
 indexinfo[].command[].indexname "myfieldset"
 indexinfo[].command[].command "attribute"
 indexinfo[].command[].indexname "myfieldset"
diff --git a/config-model/src/test/derived/importedfields/parent_a.sd b/config-model/src/test/derived/importedfields/parent_a.sd
index eea12375daf..edc81df5609 100644
--- a/config-model/src/test/derived/importedfields/parent_a.sd
+++ b/config-model/src/test/derived/importedfields/parent_a.sd
@@ -10,5 +10,13 @@ search parent_a {
     field int_wset_field type weightedset<int> {
       indexing: attribute
     }
+    field predicate_field type predicate {
+      indexing: attribute
+      index {
+        arity: 8
+        lower-bound: 5
+        upper-bound: 300
+      }
+    }
   }
 }
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummaryValidatorTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummaryValidatorTestCase.java
new file mode 100644
index 00000000000..4e1f8f1edd7
--- /dev/null
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsInSummaryValidatorTestCase.java
@@ -0,0 +1,39 @@
+package com.yahoo.searchdefinition.processing;
+
+import com.yahoo.document.DataType;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+
+/**
+ * @author geirst
+ */
+public class ImportedFieldsInSummaryValidatorTestCase {
+
+    @Rule
+    public final ExpectedException exceptionRule = ExpectedException.none();
+
+    @Test
+    public void validator_fails_if_imported_predicate_field_is_used_in_document_summary() {
+        exceptionRule.expect(IllegalArgumentException.class);
+        exceptionRule.expectMessage("For search 'child', document summary 'my_summary', " +
+                "imported summary field 'my_predicate_field': Is of type predicate. Not supported in document summaries");
+        new SearchModel()
+                .addImportedField("my_predicate_field", "ref", "predicate_field")
+                .addSummaryField("my_predicate_field", DataType.PREDICATE)
+                .resolve();
+    }
+
+    private static class SearchModel extends ImportedFieldsResolverTestCase.SearchModel {
+
+        public SearchModel() {
+            super();
+        }
+
+        public void resolve() {
+            super.resolve();
+            new ImportedFieldsInSummayValidator(childSearch, null, null, null).process();
+        }
+    }
+}
+
diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java
index 5d270bc085a..9f7c7458738 100644
--- a/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java
+++ b/config-model/src/test/java/com/yahoo/searchdefinition/processing/ImportedFieldsResolverTestCase.java
@@ -18,6 +18,8 @@ import com.yahoo.searchdefinition.document.SDField;
 import com.yahoo.searchdefinition.document.TemporaryImportedField;
 import com.yahoo.searchdefinition.document.TemporarySDField;
 import com.yahoo.tensor.TensorType;
+import com.yahoo.vespa.documentmodel.DocumentSummary;
+import com.yahoo.vespa.documentmodel.SummaryField;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -107,7 +109,7 @@ public class ImportedFieldsResolverTestCase {
                 .resolve();
     }
 
-    private static class SearchModel {
+    static class SearchModel {
 
         private final ApplicationPackage app = MockApplicationPackage.createEmpty();
         public final Search grandParentSearch;
@@ -124,6 +126,7 @@ public class ImportedFieldsResolverTestCase {
             parentSearch.getDocument().addField(createField("attribute_and_index", DataType.INT, "{ attribute | index }"));
             parentSearch.getDocument().addField(new TemporarySDField("not_attribute", DataType.INT));
             parentSearch.getDocument().addField(createField("tensor_field", new TensorDataType(TensorType.fromSpec("tensor(x[])")), "{ attribute }"));
+            parentSearch.getDocument().addField(createField("predicate_field", DataType.PREDICATE, "{ attribute }"));
             addRefField(parentSearch, grandParentSearch, "ref");
             addImportedField(parentSearch, "ancient_field", "ref", "ancient_field");
 
@@ -163,6 +166,16 @@ public class ImportedFieldsResolverTestCase {
             return this;
         }
 
+        public SearchModel addSummaryField(String fieldName, DataType dataType) {
+            DocumentSummary summary = childSearch.getSummary("my_summary");
+            if (summary == null) {
+                summary = new DocumentSummary("my_summary");
+                childSearch.addSummary(summary);
+            }
+            summary.add(new SummaryField(fieldName, dataType));
+            return this;
+        }
+
         public void resolve() {
             resolve(grandParentSearch);
             resolve(parentSearch);