Merge pull request #15395 from vespa-engine/bjorncs/tensor-data-type-validation-in-config-model

Bjorncs/tensor data type validation in config model

7 files changed, 46 insertions, 71 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
index a10aac30298..0aee0675ea7 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidator.java
@@ -6,13 +6,10 @@ import com.yahoo.documentmodel.NewDocumentType;
 import com.yahoo.searchdefinition.derived.AttributeFields;
 import com.yahoo.searchdefinition.derived.IndexSchema;
 import com.yahoo.searchdefinition.document.Attribute;
-import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.searchdefinition.document.HnswIndexParams;
 import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction;
-import com.yahoo.vespa.model.application.validation.change.VespaRefeedAction;
 import com.yahoo.vespa.model.application.validation.change.VespaRestartAction;
 
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -51,12 +48,11 @@ public class AttributeChangeValidator {
         this.nextDocType = nextDocType;
     }
 
-    public List<VespaConfigChangeAction> validate(ValidationOverrides overrides, Instant now) {
+    public List<VespaConfigChangeAction> validate() {
         List<VespaConfigChangeAction> result = new ArrayList<>();
         result.addAll(validateAddAttributeAspect());
         result.addAll(validateRemoveAttributeAspect());
         result.addAll(validateAttributeSettings());
-        result.addAll(validateTensorTypes(overrides, now));
         return result;
     }
 
@@ -144,36 +140,4 @@ public class AttributeChangeValidator {
         }
     }
 
-    private List<VespaConfigChangeAction> validateTensorTypes(ValidationOverrides overrides, Instant now) {
-        List<VespaConfigChangeAction> result = new ArrayList<>();
-
-        for (Attribute nextAttr : nextFields.attributes()) {
-            Attribute currentAttr = currentFields.getAttribute(nextAttr.getName());
-
-            if (currentAttr != null && currentAttr.tensorType().isPresent()) {
-                // If the tensor attribute is not present on the new attribute, it means that the data type of the attribute
-                // has been changed. This is already handled by DocumentTypeChangeValidator, so we can ignore it here
-                if (!nextAttr.tensorType().isPresent()) {
-                    continue;
-                }
-
-                // Tensor attribute has changed type
-                if (!nextAttr.tensorType().get().equals(currentAttr.tensorType().get())) {
-                    result.add(createTensorTypeChangedRefeedAction(id, currentAttr, nextAttr, overrides, now));
-                }
-            }
-        }
-
-        return result;
-    }
-
-    private static VespaRefeedAction createTensorTypeChangedRefeedAction(ClusterSpec.Id id, Attribute currentAttr, Attribute nextAttr, ValidationOverrides overrides, Instant now) {
-        return VespaRefeedAction.of(id,
-                                    "tensor-type-change",
-                                    overrides,
-                                    new ChangeMessageBuilder(nextAttr.getName()).addChange("tensor type",
-                                                                                           currentAttr.tensorType().get().toString(),
-                                                                                           nextAttr.tensorType().get().toString()).build(), now);
-    }
-
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java
index 68a97f33dfd..3dcfbe3629d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/application/validation/change/search/DocumentDatabaseChangeValidator.java
@@ -38,20 +38,20 @@ public class DocumentDatabaseChangeValidator {
 
     public List<VespaConfigChangeAction> validate(ValidationOverrides overrides, Instant now) {
         List<VespaConfigChangeAction> result = new ArrayList<>();
-        result.addAll(validateAttributeChanges(overrides, now));
+        result.addAll(validateAttributeChanges());
         result.addAll(validateStructFieldAttributeChanges(overrides, now));
         result.addAll(validateIndexingScriptChanges(overrides, now));
         result.addAll(validateDocumentTypeChanges(overrides, now));
         return result;
     }
 
-    private List<VespaConfigChangeAction> validateAttributeChanges(ValidationOverrides overrides, Instant now) {
+    private List<VespaConfigChangeAction> validateAttributeChanges() {
         return new AttributeChangeValidator(id,
                                             currentDatabase.getDerivedConfiguration().getAttributeFields(),
                                             currentDatabase.getDerivedConfiguration().getIndexSchema(), currentDocType,
                                             nextDatabase.getDerivedConfiguration().getAttributeFields(),
                                             nextDatabase.getDerivedConfiguration().getIndexSchema(), nextDocType)
-                       .validate(overrides, now);
+                       .validate();
     }
 
     private List<VespaConfigChangeAction> validateStructFieldAttributeChanges(ValidationOverrides overrides, Instant now) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
index 168ee797fbf..e89f0c0a9cd 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/AttributeChangeValidatorTest.java
@@ -1,15 +1,12 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.application.validation.change.search;
 
-import com.yahoo.config.application.api.ValidationOverrides;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.vespa.model.application.validation.change.VespaConfigChangeAction;
 import org.junit.Test;
 
-import java.time.Instant;
 import java.util.List;
 
-import static com.yahoo.vespa.model.application.validation.change.ConfigChangeTestUtils.newRefeedAction;
 import static com.yahoo.vespa.model.application.validation.change.ConfigChangeTestUtils.newRestartAction;
 
 public class AttributeChangeValidatorTest {
@@ -30,7 +27,7 @@ public class AttributeChangeValidatorTest {
 
         @Override
         public List<VespaConfigChangeAction> validate() {
-            return validator.validate(ValidationOverrides.empty, Instant.now());
+            return validator.validate();
         }
 
     }
@@ -111,33 +108,6 @@ public class AttributeChangeValidatorTest {
     }
 
     @Test
-    public void changing_tensor_type_of_tensor_field_requires_refeed() throws Exception {
-        new Fixture(
-                "field f1 type tensor(x[2]) { indexing: attribute }",
-                "field f1 type tensor(x[3]) { indexing: attribute }")
-                .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"),
-                                                  "tensor-type-change",
-                                                  ValidationOverrides.empty,
-                                                  "Field 'f1' changed: tensor type: 'tensor(x[2])' -> 'tensor(x[3])'", Instant.now()));
-
-        new Fixture(
-                "field f1 type tensor(x[5]) { indexing: attribute }",
-                "field f1 type tensor(x[3]) { indexing: attribute }")
-                .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"),
-                                                  "tensor-type-change",
-                                                  ValidationOverrides.empty,
-                                                  "Field 'f1' changed: tensor type: 'tensor(x[5])' -> 'tensor(x[3])'", Instant.now()));
-    }
-
-    @Test
-    public void not_changing_tensor_type_of_tensor_field_is_ok() throws Exception {
-        new Fixture(
-                "field f1 type tensor(x[2]) { indexing: attribute }",
-                "field f1 type tensor(x[2]) { indexing: attribute }")
-                .assertValidation();
-    }
-
-    @Test
     public void adding_rank_filter_requires_restart() throws Exception {
         new Fixture("field f1 type string { indexing: attribute }",
                 "field f1 type string { indexing: attribute \n rank: filter }").
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
index 190c2c8c645..a074f961a53 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/application/validation/change/search/DocumentTypeChangeValidatorTest.java
@@ -208,6 +208,25 @@ public class DocumentTypeChangeValidatorTest {
                 action.toString());
     }
 
+    @Test
+    public void changing_tensor_type_of_tensor_field_requires_refeed() throws Exception {
+        new Fixture(
+                "field f1 type tensor(x[2]) { indexing: attribute }",
+                "field f1 type tensor(x[3]) { indexing: attribute }")
+                .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"),
+                        "field-type-change",
+                        ValidationOverrides.empty,
+                        "Field 'f1' changed: data type: 'tensor(x[2])' -> 'tensor(x[3])'", Instant.now()));
+
+        new Fixture(
+                "field f1 type tensor(x[5]) { indexing: attribute }",
+                "field f1 type tensor(x[3]) { indexing: attribute }")
+                .assertValidation(newRefeedAction(ClusterSpec.Id.from("test"),
+                        "field-type-change",
+                        ValidationOverrides.empty,
+                        "Field 'f1' changed: data type: 'tensor(x[5])' -> 'tensor(x[3])'", Instant.now()));
+    }
+
     private static NewDocumentType createDocumentTypeWithReferenceField(String nameReferencedDocumentType) {
         StructDataType headerfields = new StructDataType("headerfields");
         headerfields.addField(new Field("ref", new ReferenceDataType(new DocumentType(nameReferencedDocumentType), 0)));
diff --git a/document/abi-spec.json b/document/abi-spec.json
index b119f9991b3..903b7a897df 100644
--- a/document/abi-spec.json
+++ b/document/abi-spec.json
@@ -1911,6 +1911,8 @@
       "public java.lang.Class getValueClass()",
       "public boolean isValueCompatible(com.yahoo.document.datatypes.FieldValue)",
       "public com.yahoo.tensor.TensorType getTensorType()",
+      "public boolean equals(java.lang.Object)",
+      "public int hashCode()",
       "public bridge synthetic com.yahoo.document.DataType clone()",
       "public bridge synthetic com.yahoo.vespa.objects.Identifiable clone()",
       "public bridge synthetic java.lang.Object clone()"
diff --git a/document/src/main/java/com/yahoo/document/DataType.java b/document/src/main/java/com/yahoo/document/DataType.java
index fa5dffd042a..104d63cae96 100644
--- a/document/src/main/java/com/yahoo/document/DataType.java
+++ b/document/src/main/java/com/yahoo/document/DataType.java
@@ -84,6 +84,7 @@ public abstract class DataType extends Identifiable implements Serializable, Com
         this.dataTypeId = dataTypeId;
     }
 
+    @Override
     public DataType clone() {
         return (DataType)super.clone();
     }
@@ -248,14 +249,17 @@ public abstract class DataType extends Identifiable implements Serializable, Com
         manager.registerSingleType(this);
     }
 
+    @Override
     public int hashCode() {
         return name.hashCode();
     }
 
+    @Override
     public boolean equals(Object other) {
         return (other instanceof DataType) && (dataTypeId == ((DataType)other).dataTypeId);
     }
 
+    @Override
     public String toString() {
         return "datatype " + name + " (code: " + dataTypeId + ")";
     }
diff --git a/document/src/main/java/com/yahoo/document/TensorDataType.java b/document/src/main/java/com/yahoo/document/TensorDataType.java
index b21461597bf..c4fdff30f8b 100644
--- a/document/src/main/java/com/yahoo/document/TensorDataType.java
+++ b/document/src/main/java/com/yahoo/document/TensorDataType.java
@@ -6,6 +6,8 @@ import com.yahoo.document.datatypes.TensorFieldValue;
 import com.yahoo.tensor.TensorType;
 import com.yahoo.vespa.objects.Ids;
 
+import java.util.Objects;
+
 /**
  * A DataType containing a tensor type
  *
@@ -23,6 +25,7 @@ public class TensorDataType extends DataType {
         this.tensorType = tensorType;
     }
 
+    @Override
     public TensorDataType clone() {
         return (TensorDataType)super.clone();
     }
@@ -48,4 +51,17 @@ public class TensorDataType extends DataType {
     /** Returns the type of the tensor this field can hold */
     public TensorType getTensorType() { return tensorType; }
 
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        if (!super.equals(o)) return false;
+        TensorDataType that = (TensorDataType) o;
+        return Objects.equals(tensorType, that.tensorType);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode(), tensorType);
+    }
 }