From 06ae58f434aa7a513259f43d3fa932bcc513682c Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 19 Nov 2020 16:42:22 +0100 Subject: Add '@Override' --- document/src/main/java/com/yahoo/document/DataType.java | 4 ++++ document/src/main/java/com/yahoo/document/TensorDataType.java | 1 + 2 files changed, 5 insertions(+) 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..a9624e96b16 100644 --- a/document/src/main/java/com/yahoo/document/TensorDataType.java +++ b/document/src/main/java/com/yahoo/document/TensorDataType.java @@ -23,6 +23,7 @@ public class TensorDataType extends DataType { this.tensorType = tensorType; } + @Override public TensorDataType clone() { return (TensorDataType)super.clone(); } -- cgit v1.2.3 From 025248c5690e1f1c5189d53e80d302b1d7369542 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 19 Nov 2020 16:49:55 +0100 Subject: Override equals()/hashCode() for TensorDataType --- document/abi-spec.json | 2 ++ .../src/main/java/com/yahoo/document/TensorDataType.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+) 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/TensorDataType.java b/document/src/main/java/com/yahoo/document/TensorDataType.java index a9624e96b16..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 * @@ -49,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); + } } -- cgit v1.2.3 From 128b1d270aaca00b958350718a928335f5381b78 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 19 Nov 2020 16:50:35 +0100 Subject: Verify that DocumentTypeChangeValidator detects tensor type changes --- .../search/DocumentTypeChangeValidatorTest.java | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) 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..8416cb459ac 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,33 @@ 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())); + } + + @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(); + } + private static NewDocumentType createDocumentTypeWithReferenceField(String nameReferencedDocumentType) { StructDataType headerfields = new StructDataType("headerfields"); headerfields.addField(new Field("ref", new ReferenceDataType(new DocumentType(nameReferencedDocumentType), 0))); -- cgit v1.2.3 From 1fc38888499542110c46605ba95fcf53b8fc19cb Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 4 Nov 2020 15:11:13 +0100 Subject: Reapply "Remove handling of tensor-type-change in AttributeChangeValidator" This is no longer needed as tensor type now follows the field type, and such changes are handled in DocumentChangeValidator. --- .../change/search/AttributeChangeValidator.java | 38 +--------------------- .../search/DocumentDatabaseChangeValidator.java | 6 ++-- .../search/AttributeChangeValidatorTest.java | 32 +----------------- 3 files changed, 5 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 validate(ValidationOverrides overrides, Instant now) { + public List validate() { List 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 validateTensorTypes(ValidationOverrides overrides, Instant now) { - List 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 validate(ValidationOverrides overrides, Instant now) { List 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 validateAttributeChanges(ValidationOverrides overrides, Instant now) { + private List 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 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 validate() { - return validator.validate(ValidationOverrides.empty, Instant.now()); + return validator.validate(); } } @@ -110,33 +107,6 @@ public class AttributeChangeValidatorTest { f.assertValidation(); } - @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 }", -- cgit v1.2.3 From 9dae24e2cab3e59b681a558dde405c8fdc3ee540 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 19 Nov 2020 17:25:55 +0100 Subject: Remove redundant test --- .../validation/change/search/DocumentTypeChangeValidatorTest.java | 8 -------- 1 file changed, 8 deletions(-) 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 8416cb459ac..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 @@ -227,14 +227,6 @@ public class DocumentTypeChangeValidatorTest { "Field 'f1' changed: data 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(); - } - private static NewDocumentType createDocumentTypeWithReferenceField(String nameReferencedDocumentType) { StructDataType headerfields = new StructDataType("headerfields"); headerfields.addField(new Field("ref", new ReferenceDataType(new DocumentType(nameReferencedDocumentType), 0))); -- cgit v1.2.3