Merge pull request #8578 from vespa-engine/geirst/integrate-tensor-remove-update-in-searchcore

Geirst/integrate tensor remove update in searchcore

5 files changed, 85 insertions, 29 deletions

diff --git a/document/src/tests/documentupdatetestcase.cpp b/document/src/tests/documentupdatetestcase.cpp
index c74c211756f..017d83893f0 100644
--- a/document/src/tests/documentupdatetestcase.cpp
+++ b/document/src/tests/documentupdatetestcase.cpp
@@ -922,6 +922,17 @@ TEST(DocumentUpdateTest, tensor_add_update_can_be_applied)
                                 .add({{"x", "c"}}, 7));
 }
 
+TEST(DocumentUpdateTest, tensor_remove_update_can_be_applied)
+{
+    TensorUpdateFixture f;
+    f.assertApplyUpdate(f.spec().add({{"x", "a"}}, 2)
+                                .add({{"x", "b"}}, 3),
+
+                        TensorRemoveUpdate(f.makeTensor(f.spec().add({{"x", "b"}}, 1))),
+
+                        f.spec().add({{"x", "a"}}, 2));
+}
+
 TEST(DocumentUpdateTest, tensor_modify_update_can_be_applied)
 {
     TensorUpdateFixture f;
diff --git a/document/src/vespa/document/update/tensor_remove_update.cpp b/document/src/vespa/document/update/tensor_remove_update.cpp
index 3e2bb86c66b..671bf260629 100644
--- a/document/src/vespa/document/update/tensor_remove_update.cpp
+++ b/document/src/vespa/document/update/tensor_remove_update.cpp
@@ -6,6 +6,8 @@
 #include <vespa/document/fieldvalue/document.h>
 #include <vespa/document/fieldvalue/tensorfieldvalue.h>
 #include <vespa/document/serialization/vespadocumentdeserializer.h>
+#include <vespa/eval/tensor/cell_values.h>
+#include <vespa/eval/tensor/sparse/sparse_tensor.h>
 #include <vespa/eval/tensor/tensor.h>
 #include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/vespalib/util/xmlstream.h>
@@ -77,17 +79,35 @@ TensorRemoveUpdate::checkCompatibility(const Field &field) const
 std::unique_ptr<Tensor>
 TensorRemoveUpdate::applyTo(const Tensor &tensor) const
 {
-    // TODO: implement
-    (void) tensor;
+    auto &addressTensor = _tensor->getAsTensorPtr();
+    if (addressTensor) {
+        if (const auto *sparseTensor = dynamic_cast<const vespalib::tensor::SparseTensor *>(addressTensor.get())) {
+            vespalib::tensor::CellValues cellAddresses(*sparseTensor);
+            return tensor.remove(cellAddresses);
+        } else {
+            throw IllegalArgumentException(make_string("Expected address tensor to be sparse, but has type '%s'",
+                                                       addressTensor->type().to_spec().c_str()));
+        }
+    }
     return std::unique_ptr<Tensor>();
 }
 
 bool
 TensorRemoveUpdate::applyTo(FieldValue &value) const
 {
-    // TODO: implement
-    (void) value;
-    return false;
+    if (value.inherits(TensorFieldValue::classId)) {
+        TensorFieldValue &tensorFieldValue = static_cast<TensorFieldValue &>(value);
+        auto &oldTensor = tensorFieldValue.getAsTensorPtr();
+        auto newTensor = applyTo(*oldTensor);
+        if (newTensor) {
+            tensorFieldValue = std::move(newTensor);
+        }
+    } else {
+        std::string err = make_string("Unable to perform a tensor remove update on a '%s' field value.",
+                                      value.getClass().name());
+        throw IllegalStateException(err, VESPA_STRLOC);
+    }
+    return true;
 }
 
 void
diff --git a/document/src/vespa/document/update/valueupdate.h b/document/src/vespa/document/update/valueupdate.h
index 0e15943f8e4..6939d10ce2c 100644
--- a/document/src/vespa/document/update/valueupdate.h
+++ b/document/src/vespa/document/update/valueupdate.h
@@ -55,7 +55,8 @@ public:
         Map        = IDENTIFIABLE_CLASSID(MapValueUpdate),
         Remove     = IDENTIFIABLE_CLASSID(RemoveValueUpdate),
         TensorModifyUpdate = IDENTIFIABLE_CLASSID(TensorModifyUpdate),
-        TensorAddUpdate = IDENTIFIABLE_CLASSID(TensorAddUpdate)
+        TensorAddUpdate = IDENTIFIABLE_CLASSID(TensorAddUpdate),
+        TensorRemoveUpdate = IDENTIFIABLE_CLASSID(TensorRemoveUpdate)
     };
 
     ValueUpdate()
diff --git a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
index afbb1c30f17..78cd9ce44b9 100644
--- a/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
+++ b/searchcore/src/tests/proton/common/attribute_updater/attribute_updater_test.cpp
@@ -20,6 +20,7 @@
 #include <vespa/document/update/removevalueupdate.h>
 #include <vespa/document/update/tensor_add_update.h>
 #include <vespa/document/update/tensor_modify_update.h>
+#include <vespa/document/update/tensor_remove_update.h>
 #include <vespa/eval/tensor/default_tensor_engine.h>
 #include <vespa/eval/tensor/tensor.h>
 #include <vespa/searchcore/proton/common/attribute_updater.h>
@@ -28,8 +29,8 @@
 #include <vespa/searchlib/attribute/reference_attribute.h>
 #include <vespa/searchlib/tensor/dense_tensor_attribute.h>
 #include <vespa/searchlib/tensor/generic_tensor_attribute.h>
-#include <vespa/vespalib/testkit/testapp.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
+#include <vespa/vespalib/testkit/testapp.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP("attribute_updater_test");
@@ -76,7 +77,8 @@ makeDocumentTypeRepo()
                              .addField("wsfloat", Wset(DataType::T_FLOAT))
                              .addField("wsstring", Wset(DataType::T_STRING))
                              .addField("ref", 333)
-                             .addField("dense_tensor", DataType::T_TENSOR),
+                             .addField("dense_tensor", DataType::T_TENSOR)
+                             .addField("sparse_tensor", DataType::T_TENSOR),
                      Struct("testdoc.body"))
                      .referenceType(333, 222);
     return std::make_unique<DocumentTypeRepo>(builder.config());
@@ -416,35 +418,54 @@ makeTensorFieldValue(const TensorSpec &spec)
     return result;
 }
 
-void
-setTensor(TensorAttribute &attribute, uint32_t lid, const TensorSpec &spec)
-{
-    auto tensor = makeTensor(spec);
-    attribute.setTensor(lid, *tensor);
-    attribute.commit();
-}
+template <typename TensorAttributeType>
+struct TensorFixture : public Fixture {
+    vespalib::string type;
+    std::unique_ptr<TensorAttributeType> attribute;
 
-TEST_F("require that tensor modify update is applied", Fixture)
-{
-    vespalib::string type = "tensor(x[2])";
-    auto attribute = makeTensorAttribute<DenseTensorAttribute>("dense_tensor", type);
-    setTensor(*attribute, 1, TensorSpec(type).add({{"x", 0}}, 3).add({{"x", 1}}, 5));
+    TensorFixture(const vespalib::string &type_, const vespalib::string &name)
+        : type(type_),
+          attribute(makeTensorAttribute<TensorAttributeType>(name, type))
+    {
+    }
 
-    f.applyValueUpdate(*attribute, 1,
+    void setTensor(const TensorSpec &spec) {
+        auto tensor = makeTensor(spec);
+        attribute->setTensor(1, *tensor);
+        attribute->commit();
+    }
+
+    void assertTensor(const TensorSpec &expSpec) {
+        EXPECT_EQUAL(expSpec, attribute->getTensor(1)->toSpec());
+    }
+};
+
+TEST_F("require that tensor modify update is applied",
+        TensorFixture<DenseTensorAttribute>("tensor(x[2])", "dense_tensor"))
+{
+    f.setTensor(TensorSpec(f.type).add({{"x", 0}}, 3).add({{"x", 1}}, 5));
+    f.applyValueUpdate(*f.attribute, 1,
                        TensorModifyUpdate(TensorModifyUpdate::Operation::REPLACE,
                                           makeTensorFieldValue(TensorSpec("tensor(x{})").add({{"x", 0}}, 7))));
-    EXPECT_EQUAL(TensorSpec(type).add({{"x", 0}}, 7).add({{"x", 1}}, 5), attribute->getTensor(1)->toSpec());
+    f.assertTensor(TensorSpec(f.type).add({{"x", 0}}, 7).add({{"x", 1}}, 5));
 }
 
-TEST_F("require that tensor add update is applied", Fixture)
+TEST_F("require that tensor add update is applied",
+        TensorFixture<GenericTensorAttribute>("tensor(x{})", "sparse_tensor"))
 {
-    vespalib::string type = "tensor(x{})";
-    auto attribute = makeTensorAttribute<GenericTensorAttribute>("dense_tensor", type);
-    setTensor(*attribute, 1, TensorSpec(type).add({{"x", "a"}}, 2));
+    f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2));
+    f.applyValueUpdate(*f.attribute, 1,
+                       TensorAddUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "a"}}, 3))));
+    f.assertTensor(TensorSpec(f.type).add({{"x", "a"}}, 3));
+}
 
-    f.applyValueUpdate(*attribute, 1,
-                       TensorAddUpdate(makeTensorFieldValue(TensorSpec(type).add({{"x", "a"}}, 3))));
-    EXPECT_EQUAL(TensorSpec(type).add({{"x", "a"}}, 3), attribute->getTensor(1)->toSpec());
+TEST_F("require that tensor remove update is applied",
+        TensorFixture<GenericTensorAttribute>("tensor(x{})", "sparse_tensor"))
+{
+    f.setTensor(TensorSpec(f.type).add({{"x", "a"}}, 2).add({{"x", "b"}}, 3));
+    f.applyValueUpdate(*f.attribute, 1,
+                       TensorRemoveUpdate(makeTensorFieldValue(TensorSpec(f.type).add({{"x", "b"}}, 1))));
+    f.assertTensor(TensorSpec(f.type).add({{"x", "a"}}, 2));
 }
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp b/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp
index 933857cffed..fcca1c2a737 100644
--- a/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp
+++ b/searchcore/src/vespa/searchcore/proton/common/attribute_updater.cpp
@@ -16,6 +16,7 @@
 #include <vespa/document/update/removevalueupdate.h>
 #include <vespa/document/update/tensor_add_update.h>
 #include <vespa/document/update/tensor_modify_update.h>
+#include <vespa/document/update/tensor_remove_update.h>
 #include <vespa/eval/tensor/tensor.h>
 #include <vespa/searchlib/attribute/attributevector.hpp>
 #include <vespa/searchlib/attribute/changevector.hpp>
@@ -238,6 +239,8 @@ AttributeUpdater::handleUpdate(TensorAttribute &vec, uint32_t lid, const ValueUp
         applyTensorUpdate(vec, lid, static_cast<const TensorModifyUpdate &>(upd));
     } else if (op == ValueUpdate::TensorAddUpdate) {
         applyTensorUpdate(vec, lid, static_cast<const TensorAddUpdate &>(upd));
+    } else if (op == ValueUpdate::TensorRemoveUpdate) {
+        applyTensorUpdate(vec, lid, static_cast<const TensorRemoveUpdate &>(upd));
     } else if (op == ValueUpdate::Clear) {
         vec.clearDoc(lid);
     } else {