Move interpolated lookup into the same fold the same way as array lookup.

6 files changed, 88 insertions, 117 deletions

diff --git a/searchlib/src/tests/aggregator/attr_test.cpp b/searchlib/src/tests/aggregator/attr_test.cpp
index b082e89ce19..899c22d9707 100644
--- a/searchlib/src/tests/aggregator/attr_test.cpp
+++ b/searchlib/src/tests/aggregator/attr_test.cpp
@@ -240,7 +240,7 @@ TEST_F("testWithRelevance", AttributeFixture()) {
         r *= 0.1;
         TEST_STATE(vespalib::make_string("i=%d", i).c_str());
         EXPECT_TRUE(et.execute(0, HitRank(r)));
-        EXPECT_EQUAL(expect0[i], et.getResult()->getFloat());
+        EXPECT_EQUAL(et.getResult()->getFloat(), expect0[i]);
     }
 
     EXPECT_TRUE(et.execute(0, HitRank(f1.doc0attr[2])));
diff --git a/searchlib/src/vespa/searchlib/expression/arrayatlookupfunctionnode.cpp b/searchlib/src/vespa/searchlib/expression/arrayatlookupfunctionnode.cpp
index ac07a0d0140..6122a0f1e50 100644
--- a/searchlib/src/vespa/searchlib/expression/arrayatlookupfunctionnode.cpp
+++ b/searchlib/src/vespa/searchlib/expression/arrayatlookupfunctionnode.cpp
@@ -93,7 +93,7 @@ ArrayAtLookup::selectMembers(const vespalib::ObjectPredicate & predicate, vespal
 {
     AttributeNode::selectMembers(predicate, operation);
     if (_indexExpression) {
-        _indexExpression->selectMembers(predicate, operation);
+        _indexExpression->select(predicate, operation);
     }
 }
 
diff --git a/searchlib/src/vespa/searchlib/expression/expressiontree.cpp b/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
index cef596ca3be..efc9e77baf0 100644
--- a/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
+++ b/searchlib/src/vespa/searchlib/expression/expressiontree.cpp
@@ -3,7 +3,6 @@
 #include "expressiontree.h"
 #include "documentaccessornode.h"
 #include "relevancenode.h"
-#include "interpolatedlookupfunctionnode.h"
 #include "attributenode.h"
 
 namespace search::expression {
@@ -26,8 +25,7 @@ ExpressionTree::ExpressionTree() :
     _root(),
     _attributeNodes(),
     _documentAccessorNodes(),
-    _relevanceNodes(),
-    _interpolatedLookupNodes()
+    _relevanceNodes()
 {
     prepare(false);
 }
@@ -36,8 +34,7 @@ ExpressionTree::ExpressionTree(const ExpressionNode &root) :
     _root(root.clone()),
     _attributeNodes(),
     _documentAccessorNodes(),
-    _relevanceNodes(),
-    _interpolatedLookupNodes()
+    _relevanceNodes()
 {
     prepare(false);
 }
@@ -80,7 +77,6 @@ ExpressionTree::onPrepare(bool preserveAccurateTypes)
         gather(_attributeNodes).from(*_root);
         gather(_documentAccessorNodes).from(*_root);
         gather(_relevanceNodes).from(*_root);
-        gather(_interpolatedLookupNodes).from(*_root);
     }
 }
 
@@ -89,8 +85,7 @@ ExpressionTree::ExpressionTree(ExpressionNode::UP root) :
     _root(std::move(root)),
     _attributeNodes(),
     _documentAccessorNodes(),
-    _relevanceNodes(),
-    _interpolatedLookupNodes()
+    _relevanceNodes()
 {
     prepare(false);
 }
@@ -100,8 +95,7 @@ ExpressionTree::ExpressionTree(const ExpressionTree & rhs) :
     _root(rhs._root),
     _attributeNodes(),
     _documentAccessorNodes(),
-    _relevanceNodes(),
-    _interpolatedLookupNodes()
+    _relevanceNodes()
 {
     prepare(false);
 }
@@ -131,7 +125,6 @@ ExpressionTree::swap(ExpressionTree & e)
     _attributeNodes.swap(e._attributeNodes);
     _documentAccessorNodes.swap(e._documentAccessorNodes);
     _relevanceNodes.swap(e._relevanceNodes);
-    _interpolatedLookupNodes.swap(e._interpolatedLookupNodes);
 }
 
 ExpressionTree::~ExpressionTree() = default;
@@ -139,43 +132,16 @@ ExpressionTree::~ExpressionTree() = default;
 bool
 ExpressionTree::execute(const document::Document & doc, HitRank rank) const
 {
-    for(auto * node : _documentAccessorNodes) {
-        node->setDoc(doc);
-    }
-    for(auto * node : _relevanceNodes) {
-        node->setRelevance(rank);
-    }
+    std::for_each(_documentAccessorNodes.cbegin(), _documentAccessorNodes.cend(), [&doc](DocumentAccessorNode * node) { node->setDoc(doc); });
+    std::for_each(_relevanceNodes.cbegin(), _relevanceNodes.cend(), [rank](RelevanceNode * node) { node->setRelevance(rank); });
     return _root->execute();
 }
 
-struct DocIdSetter {
-    DocId _docId;
-    void operator() (InterpolatedLookup *node) {
-        node->setDocId(_docId);
-    }
-    void operator() (AttributeNode *node) {
-        node->setDocId(_docId);
-    }
-    DocIdSetter(DocId docId) : _docId(docId) {}
-};
-
-struct RankSetter {
-    HitRank _rank;
-    void operator() (RelevanceNode *node) {
-        node->setRelevance(_rank);
-    }
-    RankSetter(HitRank rank) : _rank(rank) {}
-};
-
-
 bool
 ExpressionTree::execute(DocId docId, HitRank rank) const
 {
-    DocIdSetter setDocId(docId);
-    RankSetter setHitRank(rank);
-    std::for_each(_attributeNodes.cbegin(), _attributeNodes.cend(), setDocId);
-    std::for_each(_relevanceNodes.cbegin(), _relevanceNodes.cend(), setHitRank);
-    std::for_each(_interpolatedLookupNodes.cbegin(), _interpolatedLookupNodes.cend(), setDocId);
+    std::for_each(_attributeNodes.cbegin(), _attributeNodes.cend(), [docId](AttributeNode * node) { node->setDocId(docId); });
+    std::for_each(_relevanceNodes.cbegin(), _relevanceNodes.cend(), [rank](RelevanceNode * node) { node->setRelevance(rank); });
 
     return _root->execute();
 }
diff --git a/searchlib/src/vespa/searchlib/expression/expressiontree.h b/searchlib/src/vespa/searchlib/expression/expressiontree.h
index 54600e7fcf5..34184ae4f2d 100644
--- a/searchlib/src/vespa/searchlib/expression/expressiontree.h
+++ b/searchlib/src/vespa/searchlib/expression/expressiontree.h
@@ -73,7 +73,6 @@ private:
     AttributeNodeList         _attributeNodes;
     DocumentAccessorNodeList  _documentAccessorNodes;
     RelevanceNodeList         _relevanceNodes;
-    InterpolatedLookupList    _interpolatedLookupNodes;
 };
 
 }
diff --git a/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.cpp b/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.cpp
index 705fba1f75e..cf73c5d5ba3 100644
--- a/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.cpp
+++ b/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.cpp
@@ -10,66 +10,39 @@ namespace search::expression {
 using vespalib::Serializer;
 using vespalib::Deserializer;
 
-IMPLEMENT_EXPRESSIONNODE(InterpolatedLookup, UnaryFunctionNode);
+IMPLEMENT_EXPRESSIONNODE(InterpolatedLookup, AttributeNode);
 
 InterpolatedLookup::InterpolatedLookup() noexcept
-    : _attribute(nullptr),
-      _docId(0)
+    : AttributeNode(),
+      _lookupExpression()
 {
 }
 
 InterpolatedLookup::~InterpolatedLookup() = default;
 
 InterpolatedLookup::InterpolatedLookup(const vespalib::string &attribute, ExpressionNode::UP arg)
-    : UnaryFunctionNode(std::move(arg)),
-      _attributeName(attribute),
-      _attribute(0),
-      _docId(0)
+    : AttributeNode(attribute),
+      _lookupExpression(std::move(arg))
 {
 }
 
-InterpolatedLookup::InterpolatedLookup(const attribute::IAttributeVector &attr, ExpressionNode::UP lookupArg)
-    : UnaryFunctionNode(std::move(lookupArg)),
-      _attributeName(attr.getName()),
-      _attribute(&attr),
-      _docId(0)
+InterpolatedLookup::InterpolatedLookup(const attribute::IAttributeVector &attr, ExpressionNode::UP arg)
+    : AttributeNode(attr),
+      _lookupExpression(std::move(arg))
 {
 }
 
 
-InterpolatedLookup::InterpolatedLookup(const InterpolatedLookup &rhs) :
-    UnaryFunctionNode(rhs),
-    _attributeName(rhs._attributeName),
-    _attribute(rhs._attribute),
-    _docId(0)
-{
-}
-
-InterpolatedLookup &
-InterpolatedLookup::operator= (const InterpolatedLookup &rhs)
-{
-    if (this != &rhs) {
-        UnaryFunctionNode::operator =(rhs);
-        _attributeName = rhs._attributeName;
-        _attribute = rhs._attribute;
-        _docId = 0;
-    }
-    return *this;
-}
-
-void
-InterpolatedLookup::onPrepareResult()
-{
-    setResultType(std::make_unique<FloatResultNode>());
-}
+InterpolatedLookup::InterpolatedLookup(const InterpolatedLookup &rhs) = default;
+InterpolatedLookup & InterpolatedLookup::operator= (const InterpolatedLookup &rhs) = default;
 
 namespace {
 
 double
-simpleInterpolate(size_t sz, std::vector<double> v, double lookup) {
-    if (sz == 0 || lookup < v[0])
+simpleInterpolate(const std::vector<double> & v, double lookup) {
+    if (v.empty() || lookup < v[0])
         return 0;
-    for (size_t i = 1; i < sz; ++i) {
+    for (size_t i = 1; i < v.size(); ++i) {
         if (lookup < v[i]) {
             double total = v[i] - v[i - 1];
             double above = lookup - v[i - 1];
@@ -78,39 +51,50 @@ simpleInterpolate(size_t sz, std::vector<double> v, double lookup) {
             return result;
         }
     }
-    return sz - 1;
+    return v.size() - 1;
 }
 
+class InterpolateHandler : public AttributeNode::Handler {
+public:
+    InterpolateHandler(FloatResultNode & result, const ExpressionNode * lookupExpression) noexcept
+        : AttributeNode::Handler(),
+          _lookupExpression(lookupExpression),
+          _result(result),
+          _values()
+    { }
+    void handle(const AttributeResult & r) override;
+private:
+    const ExpressionNode *_lookupExpression;
+    FloatResultNode      &_result;
+    std::vector<double>   _values;
+};
+
+void
+InterpolateHandler::handle(const AttributeResult &r) {
+    _lookupExpression->execute();
+    double lookup = _lookupExpression->getResult()->getFloat();
+    size_t numValues = r.getAttribute()->getValueCount(r.getDocId());
+    _values.resize(numValues);
+    r.getAttribute()->get(r.getDocId(), _values.data(), _values.size());
+    _result.set(simpleInterpolate(_values, lookup));
 }
 
-bool
-InterpolatedLookup::onExecute() const
-{
-    getArg().execute();
-    double lookup = getArg().getResult()->getFloat();
-    // get attribute data
-    size_t numValues = _attribute->getValueCount(_docId);
-    std::vector<double> valueVector;
-    valueVector.resize(numValues);
-    _attribute->get(_docId, valueVector.data(), numValues);
-    double result = simpleInterpolate(numValues, valueVector, lookup);
-    static_cast<FloatResultNode &>(updateResult()).set(result);
-    return true;
 }
 
-void
-InterpolatedLookup::wireAttributes(const search::attribute::IAttributeContext & attrCtx)
-{
-    _attribute = attrCtx.getAttribute(_attributeName);
-    if (_attribute == nullptr) {
-        throw std::runtime_error(vespalib::make_string("Failed locating attribute vector '%s'", _attributeName.c_str()));
-    }
+std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<AttributeNode::Handler>>
+InterpolatedLookup::createResultHandler(bool, const attribute::IAttributeVector &) const {
+    auto result = std::make_unique<FloatResultNode>();
+    auto handler = std::make_unique<InterpolateHandler>(*result, _lookupExpression.get());
+    return { std::move(result), std::move(handler) };
 }
 
 Serializer &
 InterpolatedLookup::onSerialize(Serializer & os) const
 {
-    UnaryFunctionNode::onSerialize(os);
+    // Here we are doing a dirty skipping AttributeNode in the inheritance.
+    // This is due to refactoring and the need to keep serialization the same.
+    FunctionNode::onSerialize(os);
+    os << uint32_t(1u) << _lookupExpression; // Simulating a single element vector.
     os << _attributeName;
     return os;
 }
@@ -118,9 +102,33 @@ InterpolatedLookup::onSerialize(Serializer & os) const
 Deserializer &
 InterpolatedLookup::onDeserialize(Deserializer & is)
 {
-    UnaryFunctionNode::onDeserialize(is);
+    // See comment in onSerialize method.
+    FunctionNode::onDeserialize(is);
+    uint32_t count(0);
+    is >> count;
+    if (count > 0) {
+        is >> _lookupExpression;
+    } else {
+        _lookupExpression.reset();
+    }
     is >> _attributeName;
     return is;
 }
 
+void
+InterpolatedLookup::visitMembers(vespalib::ObjectVisitor &visitor) const
+{
+    AttributeNode::visitMembers(visitor);
+    visit(visitor, "index", *_lookupExpression);
+}
+
+void
+InterpolatedLookup::selectMembers(const vespalib::ObjectPredicate & predicate, vespalib::ObjectOperation & operation)
+{
+    AttributeNode::selectMembers(predicate, operation);
+    if (_lookupExpression) {
+        _lookupExpression->select(predicate, operation);
+    }
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.h b/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.h
index de2ba5c362c..9d067681da9 100644
--- a/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.h
+++ b/searchlib/src/vespa/searchlib/expression/interpolatedlookupfunctionnode.h
@@ -1,13 +1,13 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 #pragma once
 
-#include "unaryfunctionnode.h"
+#include "attributenode.h"
 
 namespace search::attribute { class IAttributeVector; }
 
 namespace search::expression {
 
-class InterpolatedLookup : public UnaryFunctionNode
+class InterpolatedLookup : public AttributeNode
 {
 public:
     DECLARE_EXPRESSIONNODE(InterpolatedLookup);
@@ -19,14 +19,12 @@ public:
     InterpolatedLookup(const search::attribute::IAttributeVector &attr, ExpressionNode::UP lookupArg);
     InterpolatedLookup(const InterpolatedLookup &rhs);
     InterpolatedLookup & operator= (const InterpolatedLookup &rhs);
-    void setDocId(DocId docId) { _docId = docId; }
+    void visitMembers(vespalib::ObjectVisitor & visitor) const override;
+    void selectMembers(const vespalib::ObjectPredicate & predicate, vespalib::ObjectOperation & operation) override;
 private:
-    bool onExecute() const override;
-    void onPrepareResult() override;
-    void wireAttributes(const search::attribute::IAttributeContext &attrCtx) override;
-    vespalib::string _attributeName;
-    const search::attribute::IAttributeVector * _attribute;
-    DocId _docId;
+    std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<Handler>>
+    createResultHandler(bool preserveAccurateType, const attribute::IAttributeVector & attribute) const override;
+    ExpressionNode::CP  _lookupExpression;
 };
 
 }