- Let DotProduct,Wand and WeightedSet be Term nodes in the query tree as they really are.

  That restricts the nodes to what they can really do and makes them significantly cheaper.
- In addition type conversion of numeric terms is delayed to when it is necessary.
   And as next step they can be avoided completely.

3 files changed, 67 insertions, 98 deletions

diff --git a/searchlib/src/tests/query/customtypevisitor_test.cpp b/searchlib/src/tests/query/customtypevisitor_test.cpp
index 3f7d57b7aa4..d0812a00ebf 100644
--- a/searchlib/src/tests/query/customtypevisitor_test.cpp
+++ b/searchlib/src/tests/query/customtypevisitor_test.cpp
@@ -15,18 +15,6 @@ using namespace search::query;
 
 namespace {
 
-class Test : public vespalib::TestApp {
-    const char *current_state;
-    virtual void DumpState(bool) {
-      fprintf(stderr, "%s: ERROR: in %s\n", GetName(), current_state);
-    }
-
-    template <class T> void requireThatNodeIsVisited();
-
-public:
-    int Main() override;
-};
-
 template <class Base>
 struct InitTerm : Base {
     InitTerm() : Base(typename Base::Type(), "view", 0, Weight(0)) {}
@@ -49,9 +37,9 @@ struct MyStringTerm : InitTerm<StringTerm>  {};
 struct MySubstrTerm : InitTerm<SubstringTerm>  {};
 struct MySuffixTerm : InitTerm<SuffixTerm>  {};
 struct MyWeakAnd : WeakAnd { MyWeakAnd() : WeakAnd(1234, "view") {} };
-struct MyWeightedSetTerm : WeightedSetTerm { MyWeightedSetTerm() : WeightedSetTerm("view", 0, Weight(42)) {} };
-struct MyDotProduct : DotProduct { MyDotProduct() : DotProduct("view", 0, Weight(42)) {} };
-struct MyWandTerm : WandTerm { MyWandTerm() : WandTerm("view", 0, Weight(42), 57, 67, 77.7) {} };
+struct MyWeightedSetTerm : WeightedSetTerm { MyWeightedSetTerm() : WeightedSetTerm(0, "view", 0, Weight(42)) {} };
+struct MyDotProduct : DotProduct { MyDotProduct() : DotProduct(0, "view", 0, Weight(42)) {} };
+struct MyWandTerm : WandTerm { MyWandTerm() : WandTerm(0, "view", 0, Weight(42), 57, 67, 77.7) {} };
 struct MyPredicateQuery : InitTerm<PredicateQuery> {};
 struct MyRegExpTerm : InitTerm<RegExpTerm>  {};
 struct MyNearestNeighborTerm : NearestNeighborTerm {};
@@ -119,7 +107,7 @@ public:
 };
 
 template <class T>
-void Test::requireThatNodeIsVisited() {
+void requireThatNodeIsVisited() {
     MyCustomVisitor visitor;
     Node::UP query(new T);
     visitor.isVisited<T>() = false;
@@ -127,37 +115,28 @@ void Test::requireThatNodeIsVisited() {
     ASSERT_TRUE(visitor.isVisited<T>());
 }
 
-#define TEST_CALL(func) \
-    current_state = #func; \
-    func();
-
-int
-Test::Main()
-{
-    TEST_INIT("customtypevisitor_test");
-
-    TEST_CALL(requireThatNodeIsVisited<MyAnd>);
-    TEST_CALL(requireThatNodeIsVisited<MyAndNot>);
-    TEST_CALL(requireThatNodeIsVisited<MyNear>);
-    TEST_CALL(requireThatNodeIsVisited<MyONear>);
-    TEST_CALL(requireThatNodeIsVisited<MyOr>);
-    TEST_CALL(requireThatNodeIsVisited<MyPhrase>);
-    TEST_CALL(requireThatNodeIsVisited<MySameElement>);
-    TEST_CALL(requireThatNodeIsVisited<MyRangeTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyRank>);
-    TEST_CALL(requireThatNodeIsVisited<MyNumberTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyPrefixTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyStringTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MySubstrTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MySuffixTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyWeightedSetTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyDotProduct>);
-    TEST_CALL(requireThatNodeIsVisited<MyWandTerm>);
-    TEST_CALL(requireThatNodeIsVisited<MyPredicateQuery>);
-    TEST_CALL(requireThatNodeIsVisited<MyRegExpTerm>);
-
-    TEST_DONE();
+TEST("customtypevisitor_test") {
+
+    requireThatNodeIsVisited<MyAnd>();
+    requireThatNodeIsVisited<MyAndNot>();
+    requireThatNodeIsVisited<MyNear>();
+    requireThatNodeIsVisited<MyONear>();
+    requireThatNodeIsVisited<MyOr>();
+    requireThatNodeIsVisited<MyPhrase>();
+    requireThatNodeIsVisited<MySameElement>();
+    requireThatNodeIsVisited<MyRangeTerm>();
+    requireThatNodeIsVisited<MyRank>();
+    requireThatNodeIsVisited<MyNumberTerm>();
+    requireThatNodeIsVisited<MyPrefixTerm>();
+    requireThatNodeIsVisited<MyStringTerm>();
+    requireThatNodeIsVisited<MySubstrTerm>();
+    requireThatNodeIsVisited<MySuffixTerm>();
+    requireThatNodeIsVisited<MyWeightedSetTerm>();
+    requireThatNodeIsVisited<MyDotProduct>();
+    requireThatNodeIsVisited<MyWandTerm>();
+    requireThatNodeIsVisited<MyPredicateQuery>();
+    requireThatNodeIsVisited<MyRegExpTerm>();
 }
 }  // namespace
 
-TEST_APPHOOK(Test);
+TEST_MAIN() { TEST_RUN_ALL(); }
\ No newline at end of file
diff --git a/searchlib/src/tests/query/query_visitor_test.cpp b/searchlib/src/tests/query/query_visitor_test.cpp
index 946ad17352d..ef255ad6878 100644
--- a/searchlib/src/tests/query/query_visitor_test.cpp
+++ b/searchlib/src/tests/query/query_visitor_test.cpp
@@ -15,25 +15,6 @@ using namespace search::query;
 
 namespace {
 
-class Test : public vespalib::TestApp {
-    void requireThatAllNodesCanBeVisited();
-
-    template <class T> void checkVisit(T *node);
-
-public:
-    int Main() override;
-};
-
-int
-Test::Main()
-{
-    TEST_INIT("query_visitor_test");
-
-    TEST_DO(requireThatAllNodesCanBeVisited());
-
-    TEST_DONE();
-}
-
 class MyVisitor : public QueryVisitor
 {
 public:
@@ -69,7 +50,7 @@ public:
 };
 
 template <class T>
-void Test::checkVisit(T *node) {
+void checkVisit(T *node) {
     Node::UP query(node);
     MyVisitor visitor;
     visitor.isVisited<T>() = false;
@@ -77,7 +58,7 @@ void Test::checkVisit(T *node) {
     ASSERT_TRUE(visitor.isVisited<T>());
 }
 
-void Test::requireThatAllNodesCanBeVisited() {
+TEST("requireThatAllNodesCanBeVisited") {
     checkVisit<And>(new SimpleAnd);
     checkVisit<AndNot>(new SimpleAndNot);
     checkVisit<Near>(new SimpleNear(0));
@@ -85,9 +66,9 @@ void Test::requireThatAllNodesCanBeVisited() {
     checkVisit<Or>(new SimpleOr);
     checkVisit<Phrase>(new SimplePhrase("field", 0, Weight(42)));
     checkVisit<SameElement>(new SimpleSameElement("field"));
-    checkVisit<WeightedSetTerm>(new SimpleWeightedSetTerm("field", 0, Weight(42)));
-    checkVisit<DotProduct>(new SimpleDotProduct("field", 0, Weight(42)));
-    checkVisit<WandTerm>(new SimpleWandTerm("field", 0, Weight(42), 57, 67, 77.7));
+    checkVisit<WeightedSetTerm>(new SimpleWeightedSetTerm(0, "field", 0, Weight(42)));
+    checkVisit<DotProduct>(new SimpleDotProduct(0, "field", 0, Weight(42)));
+    checkVisit<WandTerm>(new SimpleWandTerm(0, "field", 0, Weight(42), 57, 67, 77.7));
     checkVisit<Rank>(new SimpleRank);
     checkVisit<NumberTerm>(new SimpleNumberTerm("0.42", "field", 0, Weight(0)));
     const Location location(Point{10, 10}, 20, 0);
@@ -104,4 +85,4 @@ void Test::requireThatAllNodesCanBeVisited() {
 
 }  // namespace
 
-TEST_APPHOOK(Test);
+TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp
index 055d245e420..67b45d43406 100644
--- a/searchlib/src/tests/query/querybuilder_test.cpp
+++ b/searchlib/src/tests/query/querybuilder_test.cpp
@@ -90,18 +90,17 @@ Node::UP createQueryTree() {
             builder.addStringTerm(str[4], view[4], id[4], weight[4]);
             builder.addStringTerm(str[5], view[5], id[5], weight[5]);
         }
-        builder.addPredicateQuery(getPredicateQueryTerm(),
-                                  view[3], id[3], weight[3]);
-        builder.addDotProduct(3, view[2], id[2], weight[2]);
+        builder.addPredicateQuery(getPredicateQueryTerm(), view[3], id[3], weight[3]);
         {
-            builder.addStringTerm(str[3], view[3], id[3], weight[3]);
-            builder.addStringTerm(str[4], view[4], id[4], weight[4]);
-            builder.addStringTerm(str[5], view[5], id[5], weight[5]);
+            auto & n = builder.addDotProduct(3, view[2], id[2], weight[2]);
+            n.addTerm(str[3], weight[3]);
+            n.addTerm(str[4], weight[4]);
+            n.addTerm(str[5], weight[5]);
         }
-        builder.addWandTerm(2, view[0], id[0], weight[0], 57, 67, 77.7);
         {
-            builder.addStringTerm(str[1], view[1], id[1], weight[1]);
-            builder.addStringTerm(str[2], view[2], id[2], weight[2]);
+            auto & n = builder.addWandTerm(2, view[0], id[0], weight[0], 57, 67, 77.7);
+            n.addTerm(str[1], weight[1]);
+            n.addTerm(str[2], weight[2]);
         }
         builder.addRegExpTerm(str[5], view[5], id[5], weight[5]);
         builder.addSameElement(3, view[4]);
@@ -246,23 +245,33 @@ void checkQueryTreeTypes(Node *node) {
     EXPECT_TRUE(checkTerm(predicateQuery, getPredicateQueryTerm(), view[3], id[3], weight[3]));
 
     auto* dotProduct = as_node<DotProduct>(and_node->getChildren()[6]);
-    EXPECT_EQUAL(3u, dotProduct->getChildren().size());
-    string_term = as_node<StringTerm>(dotProduct->getChildren()[0]);
-    EXPECT_TRUE(checkTerm(string_term, str[3], view[3], id[3], weight[3]));
-    string_term = as_node<StringTerm>(dotProduct->getChildren()[1]);
-    EXPECT_TRUE(checkTerm(string_term, str[4], view[4], id[4], weight[4]));
-    string_term = as_node<StringTerm>(dotProduct->getChildren()[2]);
-    EXPECT_TRUE(checkTerm(string_term, str[5], view[5], id[5], weight[5]));
+    EXPECT_EQUAL(3u, dotProduct->getNumTerms());
+
+    {
+        const auto &w1 = dotProduct->getAsString(0);
+        EXPECT_EQUAL(w1.first, str[3]);
+        EXPECT_TRUE(w1.second == weight[3]);
+        const auto &w2 = dotProduct->getAsString(1);
+        EXPECT_EQUAL(w2.first, str[4]);
+        EXPECT_TRUE(w2.second == weight[4]);
+        const auto &w3 = dotProduct->getAsString(2);
+        EXPECT_EQUAL(w3.first, str[5]);
+        EXPECT_TRUE(w3.second == weight[5]);
+    }
 
     auto* wandTerm = as_node<WandTerm>(and_node->getChildren()[7]);
     EXPECT_EQUAL(57u, wandTerm->getTargetNumHits());
     EXPECT_EQUAL(67, wandTerm->getScoreThreshold());
     EXPECT_EQUAL(77.7, wandTerm->getThresholdBoostFactor());
-    EXPECT_EQUAL(2u, wandTerm->getChildren().size());
-    string_term = as_node<StringTerm>(wandTerm->getChildren()[0]);
-    EXPECT_TRUE(checkTerm(string_term, str[1], view[1], id[1], weight[1]));
-    string_term = as_node<StringTerm>(wandTerm->getChildren()[1]);
-    EXPECT_TRUE(checkTerm(string_term, str[2], view[2], id[2], weight[2]));
+    EXPECT_EQUAL(2u, wandTerm->getNumTerms());
+    {
+        const auto &w1 = wandTerm->getAsString(0);
+        EXPECT_EQUAL(w1.first, str[1]);
+        EXPECT_TRUE(w1.second == weight[1]);
+        const auto &w2 = wandTerm->getAsString(1);
+        EXPECT_EQUAL(w2.first, str[2]);
+        EXPECT_TRUE(w2.second == weight[2]);
+    }
 
     auto* regexp_term = as_node<RegExpTerm>(and_node->getChildren()[8]);
     EXPECT_TRUE(checkTerm(regexp_term, str[5], view[5], id[5], weight[5]));
@@ -336,15 +345,15 @@ struct MyPhrase : Phrase { MyPhrase(const string &f, int32_t i, Weight w) : Phra
 struct MySameElement : SameElement { MySameElement(const string &f) : SameElement(f) {}};
 
 struct MyWeightedSetTerm : WeightedSetTerm {
-    MyWeightedSetTerm(const string &f, int32_t i, Weight w) : WeightedSetTerm(f, i, w) {}
+    MyWeightedSetTerm(uint32_t n, const string &f, int32_t i, Weight w) : WeightedSetTerm(n, f, i, w) {}
 };
 struct MyDotProduct : DotProduct {
-    MyDotProduct(const string &f, int32_t i, Weight w) : DotProduct(f, i, w) {}
+    MyDotProduct(uint32_t n, const string &f, int32_t i, Weight w) : DotProduct(n, f, i, w) {}
 };
 struct MyWandTerm : WandTerm {
-    MyWandTerm(const string &f, int32_t i, Weight w, uint32_t targetNumHits,
+    MyWandTerm(uint32_t n, const string &f, int32_t i, Weight w, uint32_t targetNumHits,
                int64_t scoreThreshold, double thresholdBoostFactor)
-        : WandTerm(f, i, w, targetNumHits, scoreThreshold, thresholdBoostFactor) {}
+        : WandTerm(n, f, i, w, targetNumHits, scoreThreshold, thresholdBoostFactor) {}
 };
 struct MyRank : Rank {};
 struct MyNumberTerm : NumberTerm {
@@ -593,7 +602,7 @@ TEST("require that empty intermediate node can be added") {
 }
 
 TEST("control size of SimpleQueryStackDumpIterator") {
-    EXPECT_EQUAL(144u, sizeof(SimpleQueryStackDumpIterator));
+    EXPECT_EQUAL(128u, sizeof(SimpleQueryStackDumpIterator));
 }
 
 TEST("test query parsing error") {