Test all operations over both integer and floating point attributes.

2 files changed, 88 insertions, 9 deletions

diff --git a/searchcore/src/tests/proton/matching/attribute_operation_test.cpp b/searchcore/src/tests/proton/matching/attribute_operation_test.cpp
index c1e757788c7..8aafc518487 100644
--- a/searchcore/src/tests/proton/matching/attribute_operation_test.cpp
+++ b/searchcore/src/tests/proton/matching/attribute_operation_test.cpp
@@ -1,6 +1,8 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include <vespa/searchcore/proton/matching/attribute_operation.h>
+#include <vespa/searchlib/attribute/attributefactory.h>
+#include <vespa/searchlib/attribute/singlenumericattribute.h>
 #include <vespa/vespalib/testkit/testapp.h>
 
 #include <vespa/log/log.h>
@@ -8,6 +10,10 @@ LOG_SETUP("attribute_operation_test");
 
 using proton::matching::AttributeOperation;
 using search::attribute::BasicType;
+using search::AttributeVector;
+using search::AttributeFactory;
+using search::attribute::CollectionType;
+using search::attribute::Config;
 
 TEST("test legal operations on integer attribute") {
     const std::vector<uint32_t> docs;
@@ -18,7 +24,7 @@ TEST("test legal operations on integer attribute") {
 
 TEST("test illegal operations on integer attribute") {
     const std::vector<uint32_t> docs;
-    for (auto operation : {"", "-", "+", "+=7.1", "=a", "*=8.z", "=", "=.7"}) {
+    for (auto operation : {"", "-", "+", "+=7.1", "=a", "*=8.z", "=", "=.7", "/=0", "%=0"}) {
         EXPECT_FALSE(AttributeOperation::create(BasicType::INT64, operation, docs));
     }
 }
@@ -32,9 +38,78 @@ TEST("test legal operations on float attribute") {
 
 TEST("test illegal operations on float attribute") {
     const std::vector<uint32_t> docs;
-    for (auto operation : {"", "-", "+", "=a", "*=8.z", "="}) {
+    for (auto operation : {"", "-", "+", "=a", "*=8.z", "=", "/=0", "%=0"}) {
         EXPECT_FALSE(AttributeOperation::create(BasicType::DOUBLE, operation, docs));
     }
 }
 
+AttributeVector::SP
+createAttribute(BasicType basicType, const vespalib::string &fieldName)
+{
+    Config cfg(basicType, CollectionType::SINGLE);
+    auto av = search::AttributeFactory::createAttribute(fieldName, cfg);
+    while (20 >= av->getNumDocs()) {
+        AttributeVector::DocId checkDocId(0u);
+        ASSERT_TRUE(av->addDoc(checkDocId));
+    }
+    av->commit();
+    return av;
+}
+
+template <typename T, typename A>
+void verify(vespalib::stringref operation, AttributeVector & attr, T initial, T expected) {
+    auto & attrT = dynamic_cast<A &>(attr);
+    for (uint32_t docid(0); docid < attr.getNumDocs(); docid++) {
+        attrT.set(docid, initial);
+    }
+    std::vector<uint32_t> docs = {1,7,9,10,17,19};
+    auto op = AttributeOperation::create(attr.getBasicType(), operation, docs);
+    EXPECT_TRUE(op);
+    op->operator()(attr);
+    for (uint32_t docid(0); docid < attr.getNumDocs(); docid++) {
+        if (docs.empty() || (docid < docs.front())) {
+            EXPECT_EQUAL(initial, attrT.get(docid));
+        } else {
+            EXPECT_EQUAL(expected, attrT.get(docid));
+            docs.erase(docs.begin());
+        }
+    }
+}
+
+template <typename T>
+void verify(vespalib::stringref operation, AttributeVector & attr, T initial, T expected) {
+    BasicType::Type type = attr.getBasicType();
+    if (type == BasicType::INT64) {
+        verify<int64_t, search::SingleValueNumericAttribute<search::IntegerAttributeTemplate<int64_t>>>(operation, attr, initial, expected);
+    } else if (type == BasicType::INT32) {
+        verify<int32_t, search::SingleValueNumericAttribute<search::IntegerAttributeTemplate<int32_t>>>(operation, attr, initial, expected);
+    } else if (type == BasicType::DOUBLE) {
+        verify<double , search::SingleValueNumericAttribute<search::FloatingPointAttributeTemplate<double >>>(operation, attr, initial, expected);
+    } else if (type == BasicType::FLOAT) {
+        verify<float , search::SingleValueNumericAttribute<search::FloatingPointAttributeTemplate<float >>>(operation, attr, initial, expected);
+    } else {
+        ASSERT_TRUE(false);
+    }
+}
+
+TEST("test all integer operations") {
+    auto attr = createAttribute(BasicType::INT64, "ai");
+    const std::vector<std::pair<const char *, int64_t>> expectedOperation = {
+        {"++", 8}, {"--", 6}, {"+=7", 14}, {"-=9", -2}, {"*=3", 21}, {"/=3", 2}, {"%=3", 1}
+    };
+    for (auto operation : expectedOperation) {
+        TEST_DO(verify<int64_t>(operation.first, *attr, 7, operation.second));
+    }
+}
+
+TEST("test all float operations") {
+    auto attr = createAttribute(BasicType::DOUBLE, "af");
+    const std::vector<std::pair<const char *, double>> expectedOperation = {
+            {"++", 8}, {"--", 6}, {"+=7.3", 14.3}, {"-=0.9", 6.1}, {"*=3.1", 21.7}, {"/=2", 3.5}, {"%=3", 7}
+    };
+    for (auto operation : expectedOperation) {
+        TEST_DO(verify<double>(operation.first, *attr, 7, operation.second));
+    }
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/searchcore/src/vespa/searchcore/proton/matching/attribute_operation.cpp b/searchcore/src/vespa/searchcore/proton/matching/attribute_operation.cpp
index d91a5322876..a5832a83690 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/attribute_operation.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/attribute_operation.cpp
@@ -33,7 +33,7 @@ struct Dec {
 template <typename T>
 struct Add {
     using V = T;
-    Add(T m) : _m(m) { }
+    Add(T m) : _m(m) {}
     T _m;
     T operator()(T oldVal) const { return oldVal + _m; }
 };
@@ -41,7 +41,7 @@ struct Add {
 template <typename T>
 struct Mul {
     using V = T;
-    Mul(T m) : _m(m) { }
+    Mul(T m) : _m(m) {}
     T _m;
     T operator()(T oldVal) const { return oldVal * _m; }
 };
@@ -49,7 +49,7 @@ struct Mul {
 template <typename T>
 struct Div {
     using V = T;
-    Div(T m) : _m(m) { }
+    Div(T m) : _m(m) {}
     T _m;
     T operator()(T oldVal) const { return oldVal / _m; }
 };
@@ -57,7 +57,7 @@ struct Div {
 template <typename T>
 struct Mod {
     using V = T;
-    Mod(T m) : _m(m) { }
+    Mod(T m) : _m(m) {}
     T _m;
     T operator()(T oldVal) const { return oldVal % static_cast<int64_t>(_m); }
 };
@@ -65,21 +65,21 @@ struct Mod {
 template <>
 struct Mod<double> {
     using V = double;
-    Mod(double ) { }
+    Mod(double ) {}
     double operator()(double oldVal) const { return oldVal; }
 };
 
 template <>
 struct Mod<float> {
     using V = float;
-    Mod(float ) { }
+    Mod(float ) {}
     float operator()(float oldVal) const { return oldVal; }
 };
 
 template <typename T>
 struct Set {
     using V = T;
-    Set(T m) : _m(m) { }
+    Set(T m) : _m(m) {}
     T _m;
     T operator()(T) const { return  _m; }
 };
@@ -243,6 +243,10 @@ Operation::create(V vector) const {
                 LOG(warning, "Invalid operand, unable to consume all of (%s). (%s) is unconsumed.", operand.data(), is.c_str());
                 validOp = BAD;
             }
+            if (((validOp == DIV) || (validOp == MOD)) && (value == 0)) {
+                LOG(warning, "Division by zero is not acceptable (%s).", operand.data());
+                validOp = BAD;
+            }
         } catch (vespalib::IllegalArgumentException & e) {
             LOG(warning, "Invalid operand, ignoring : %s", e.what());
             validOp = BAD;