Unify AttributeNode variants further

4 files changed, 53 insertions, 62 deletions

diff --git a/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.cpp b/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.cpp
index 5302aaaf89a..f9f93bc8661 100644
--- a/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.cpp
+++ b/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.cpp
@@ -217,6 +217,15 @@ getUndefinedValue(BasicType::Type basicType)
     }
 }
 
+template <typename ResultNodeType>
+std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<AttributeNode::Handler>>
+prepareIntValues(std::unique_ptr<AttributeMapLookupNode::KeyHandler> keyHandler, const IAttributeVector &attribute, IAttributeVector::largeint_t undefinedValue)
+{
+    auto resultNode = std::make_unique<ResultNodeType>();
+    auto handler = std::make_unique<IntegerValueHandler<ResultNodeType>>(std::move(keyHandler), attribute, *resultNode, undefinedValue);
+    return { std::move(resultNode), std::move(handler) };
+}
+
 }
 
 AttributeMapLookupNode::AttributeMapLookupNode()
@@ -246,17 +255,8 @@ AttributeMapLookupNode::~AttributeMapLookupNode() = default;
 AttributeMapLookupNode &
 AttributeMapLookupNode::operator=(const AttributeMapLookupNode &rhs) = default;
 
-template <typename ResultNodeType>
-void
-AttributeMapLookupNode::prepareIntValues(std::unique_ptr<KeyHandler> keyHandler, const IAttributeVector &attribute, IAttributeVector::largeint_t undefinedValue)
-{
-    auto resultNode = std::make_unique<ResultNodeType>();
-    _handler = std::make_unique<IntegerValueHandler<ResultNodeType>>(std::move(keyHandler), attribute, *resultNode, undefinedValue);
-    setResultType(std::move(resultNode));
-}
-
 std::unique_ptr<AttributeMapLookupNode::KeyHandler>
-AttributeMapLookupNode::makeKeyHandlerHelper()
+AttributeMapLookupNode::makeKeyHandlerHelper() const
 {
     const IAttributeVector &attribute = *_keyAttribute;
     if (_keySourceAttribute != nullptr) {
@@ -285,7 +285,7 @@ AttributeMapLookupNode::makeKeyHandlerHelper()
 }
 
 std::unique_ptr<AttributeMapLookupNode::KeyHandler>
-AttributeMapLookupNode::makeKeyHandler()
+AttributeMapLookupNode::makeKeyHandler() const
 {
     try {
         return makeKeyHandlerHelper();
@@ -294,57 +294,49 @@ AttributeMapLookupNode::makeKeyHandler()
     }
 }
 
-void
-AttributeMapLookupNode::onPrepare(bool preserveAccurateTypes)
-{
+std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<AttributeNode::Handler>>
+AttributeMapLookupNode::createResultHandler(bool preserveAccurateTypes, const attribute::IAttributeVector & attribute) const {
     auto keyHandler = makeKeyHandler();
-    const IAttributeVector * attribute = _scratchResult->getAttribute();
-    if (attribute != nullptr) {
-        BasicType::Type basicType = attribute->getBasicType();
-        if (attribute->isIntegerType()) {
-            IAttributeVector::largeint_t undefinedValue = getUndefinedValue(basicType);
-            if (preserveAccurateTypes) {
-                switch (basicType) {
+    BasicType::Type basicType = attribute.getBasicType();
+    if (attribute.isIntegerType()) {
+        IAttributeVector::largeint_t undefinedValue = getUndefinedValue(basicType);
+        if (preserveAccurateTypes) {
+            switch (basicType) {
                 case BasicType::INT8:
-                    prepareIntValues<Int8ResultNode>(std::move(keyHandler), *attribute, undefinedValue);
-                    break;
+                    return prepareIntValues<Int8ResultNode>(std::move(keyHandler), attribute, undefinedValue);
                 case BasicType::INT16:
-                    prepareIntValues<Int16ResultNode>(std::move(keyHandler), *attribute, undefinedValue);
-                    break;
+                    return prepareIntValues<Int16ResultNode>(std::move(keyHandler), attribute, undefinedValue);
                 case BasicType::INT32:
-                    prepareIntValues<Int32ResultNode>(std::move(keyHandler), *attribute, undefinedValue);
-                    break;
+                    return prepareIntValues<Int32ResultNode>(std::move(keyHandler), attribute, undefinedValue);
                 case BasicType::INT64:
-                    prepareIntValues<Int64ResultNode>(std::move(keyHandler), *attribute, undefinedValue);
-                    break;
+                    return prepareIntValues<Int64ResultNode>(std::move(keyHandler), attribute, undefinedValue);
                 default:
-                    throw std::runtime_error("This is no valid integer attribute " + attribute->getName());
-                }
-            } else {
-                prepareIntValues<Int64ResultNode>(std::move(keyHandler), *attribute, undefinedValue);
-            }
-        } else if (attribute->isFloatingPointType()) {
-            auto resultNode = std::make_unique<FloatResultNode>();
-            _handler = std::make_unique<FloatValueHandler>(std::move(keyHandler), *attribute, *resultNode, getUndefined<double>());
-            setResultType(std::move(resultNode));
-        } else if (attribute->isStringType()) {
-            if (_useEnumOptimization) {
-                auto resultNode = std::make_unique<EnumResultNode>();
-                const StringAttribute & sattr = dynamic_cast<const StringAttribute &>(*attribute);
-                EnumHandle undefined(0);
-                bool found = attribute->findEnum(sattr.defaultValue(), undefined);
-                assert(found);
-                _handler = std::make_unique<EnumValueHandler>(std::move(keyHandler), *attribute, *resultNode, undefined);
-                setResultType(std::move(resultNode));
-            } else {
-                auto resultNode = std::make_unique<StringResultNode>();
-                _handler = std::make_unique<StringValueHandler>(std::move(keyHandler), *attribute, *resultNode, "");
-                setResultType(std::move(resultNode));
+                    throw std::runtime_error("This is no valid integer attribute " + attribute.getName());
             }
         } else {
-            throw std::runtime_error(vespalib::make_string("Can not deduce correct resultclass for attribute vector '%s'",
-                                                           attribute->getName().c_str()));
+            return prepareIntValues<Int64ResultNode>(std::move(keyHandler), attribute, undefinedValue);
         }
+    } else if (attribute.isFloatingPointType()) {
+        auto resultNode = std::make_unique<FloatResultNode>();
+        auto handler = std::make_unique<FloatValueHandler>(std::move(keyHandler), attribute, *resultNode, getUndefined<double>());
+        return { std::move(resultNode), std::move(handler) };
+    } else if (attribute.isStringType()) {
+        if (_useEnumOptimization) {
+            auto resultNode = std::make_unique<EnumResultNode>();
+            const StringAttribute & sattr = dynamic_cast<const StringAttribute &>(attribute);
+            EnumHandle undefined(0);
+            bool found = attribute.findEnum(sattr.defaultValue(), undefined);
+            assert(found);
+            auto handler = std::make_unique<EnumValueHandler>(std::move(keyHandler), attribute, *resultNode, undefined);
+            return { std::move(resultNode), std::move(handler) };
+        } else {
+            auto resultNode = std::make_unique<StringResultNode>();
+            auto handler = std::make_unique<StringValueHandler>(std::move(keyHandler), attribute, *resultNode, "");
+            return { std::move(resultNode), std::move(handler) };
+        }
+    } else {
+        throw std::runtime_error(vespalib::make_string("Can not deduce correct resultclass for attribute vector '%s'",
+                                                       attribute.getName().c_str()));
     }
 }
 
diff --git a/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.h b/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.h
index 87504206d52..8b8fb4fac87 100644
--- a/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.h
+++ b/searchlib/src/vespa/searchlib/expression/attribute_map_lookup_node.h
@@ -22,13 +22,12 @@ private:
     const IAttributeVector *_keyAttribute;
     const IAttributeVector *_keySourceAttribute;
 
-    template <typename ResultNodeType>
-    void prepareIntValues(std::unique_ptr<KeyHandler> keyHandler, const IAttributeVector &attribute, IAttributeVector::largeint_t undefinedValue);
-    std::unique_ptr<KeyHandler> makeKeyHandlerHelper();
-    std::unique_ptr<KeyHandler> makeKeyHandler();
+    std::unique_ptr<KeyHandler> makeKeyHandlerHelper() const;
+    std::unique_ptr<KeyHandler> makeKeyHandler() const;
     void cleanup() override;
     void wireAttributes(const search::attribute::IAttributeContext & attrCtx) override;
-    void onPrepare(bool preserveAccurateTypes) override;
+    std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<Handler>>
+    createResultHandler(bool preserveAccurateType, const attribute::IAttributeVector & attribute) const override;
 public:
     DECLARE_NBO_SERIALIZE;
     DECLARE_EXPRESSIONNODE(AttributeMapLookupNode);
diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.cpp b/searchlib/src/vespa/searchlib/expression/attributenode.cpp
index 5fc17380ee4..b05f5a91491 100644
--- a/searchlib/src/vespa/searchlib/expression/attributenode.cpp
+++ b/searchlib/src/vespa/searchlib/expression/attributenode.cpp
@@ -172,7 +172,7 @@ AttributeNode::operator = (const AttributeNode & attr)
 }
 
 std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<AttributeNode::Handler>>
-AttributeNode::createResultAndHandler(bool preserveAccurateTypes, const attribute::IAttributeVector & attribute) const {
+AttributeNode::createResultHandler(bool preserveAccurateTypes, const attribute::IAttributeVector & attribute) const {
     BasicType::Type basicType = attribute.getBasicType();
     if (attribute.isIntegerType()) {
         if (_hasMultiValue) {
@@ -246,7 +246,7 @@ AttributeNode::onPrepare(bool preserveAccurateTypes)
 {
     const IAttributeVector * attribute = getAttribute();
     if (attribute != nullptr) {
-        auto[result, handler] = createResultAndHandler(preserveAccurateTypes, *attribute);
+        auto[result, handler] = createResultHandler(preserveAccurateTypes, *attribute);
         _handler = std::move(handler);
         if (_index == nullptr) {
             setResultType(std::move(result));
diff --git a/searchlib/src/vespa/searchlib/expression/attributenode.h b/searchlib/src/vespa/searchlib/expression/attributenode.h
index bb8e574d853..2a677889597 100644
--- a/searchlib/src/vespa/searchlib/expression/attributenode.h
+++ b/searchlib/src/vespa/searchlib/expression/attributenode.h
@@ -64,8 +64,8 @@ public:
         virtual void handle(const AttributeResult & r) = 0;
     };
 private:
-    std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<Handler>>
-    createResultAndHandler(bool preserveAccurateType, const attribute::IAttributeVector & attribute) const;
+    virtual std::pair<std::unique_ptr<ResultNode>, std::unique_ptr<Handler>>
+    createResultHandler(bool preserveAccurateType, const attribute::IAttributeVector & attribute) const;
     template <typename V> class IntegerHandler;
     class FloatHandler;
     class StringHandler;