verify that Rank and AndNot stays on top when using location search too.

2 files changed, 64 insertions, 3 deletions

diff --git a/searchcore/src/tests/proton/matching/query_test.cpp b/searchcore/src/tests/proton/matching/query_test.cpp
index 912c0be5c0e..0580ef55425 100644
--- a/searchcore/src/tests/proton/matching/query_test.cpp
+++ b/searchcore/src/tests/proton/matching/query_test.cpp
@@ -27,6 +27,7 @@
 #include <vespa/searchlib/queryeval/fake_requestcontext.h>
 #include <vespa/searchlib/queryeval/termasstring.h>
 #include <vespa/searchlib/parsequery/stackdumpiterator.h>
+#include <vespa/vespalib/data/slime/slime.h>
 
 #include <vespa/document/datatype/positiondatatype.h>
 
@@ -56,6 +57,7 @@ using search::queryeval::Blueprint;
 using search::queryeval::FakeResult;
 using search::queryeval::FakeSearchable;
 using search::queryeval::FakeRequestContext;
+using search::queryeval::FakeBlueprint;
 using search::queryeval::FieldSpec;
 using search::queryeval::FieldSpecList;
 using search::queryeval::Searchable;
@@ -105,6 +107,7 @@ class Test : public vespalib::TestApp {
     void requireThatUnknownFieldActsEmpty();
     void requireThatIllegalFieldsAreIgnored();
     void requireThatQueryGluesEverythingTogether();
+    void requireThatLocationIsAddedTheCorrectPlace();
     void requireThatQueryAddsLocation();
     void requireThatQueryAddsLocationCutoff();
     void requireThatFakeFieldSearchDumpsDiffer();
@@ -139,6 +142,7 @@ void Test::tearDown() {
 }
 
 const string field = "field";
+const string loc_field = "location";
 const string resolved_field1 = "resolved1";
 const string resolved_field2 = "resolved2";
 const string unknown_field = "unknown_field";
@@ -173,6 +177,10 @@ void setupIndexEnvironments()
 
     FieldInfo attr_info(FieldType::ATTRIBUTE, CollectionType::SINGLE, field, 0);
     attribute_index_env.getFields().push_back(attr_info);
+    FieldInfo loc_field_info = FieldInfo(FieldType::ATTRIBUTE, CollectionType::SINGLE,
+                                     PositionDataType::getZCurveFieldName(loc_field), field_id + 1);
+    plain_index_env.getFields().push_back(loc_field_info);
+    attribute_index_env.getFields().push_back(loc_field_info);
 }
 
 Node::UP buildQueryTree(const ViewResolver &resolver,
@@ -689,8 +697,6 @@ void Test::requireThatQueryGluesEverythingTogether() {
 }
 
 void checkQueryAddsLocation(Test &test, const string &loc_string) {
-    const string loc_field = "location";
-
     fef_test::IndexEnvironment index_environment;
     FieldInfo field_info(FieldType::INDEX, CollectionType::SINGLE, field, 0);
     index_environment.getFields().push_back(field_info);
@@ -726,6 +732,60 @@ void checkQueryAddsLocation(Test &test, const string &loc_string) {
     }
 }
 
+template<typename T1, typename T2>
+void verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation(QueryBuilder<ProtonNodeTypes> & builder) {
+    const string loc_string = "(2,10,10,3,0,1,0,0)";
+    builder.addStringTerm("foo", field, field_id, string_weight);
+    builder.addStringTerm("bar", field, field_id, string_weight);
+    builder.addStringTerm("baz", field, field_id, string_weight);
+    std::string stackDump = StackDumpCreator::create(*builder.build());
+
+    Query query;
+    query.buildTree(stackDump, loc_field + ":" + loc_string, ViewResolver(), attribute_index_env);
+    FakeSearchContext context(42);
+    context.addIdx(0).idx(0).getFake()
+            .addResult(field, "foo", FakeResult().doc(1));
+    context.setLimit(42);
+
+    query.setWhiteListBlueprint(std::make_unique<SimpleBlueprint>(SimpleResult()));
+
+    FakeRequestContext requestContext;
+    MatchDataLayout mdl;
+    query.reserveHandles(requestContext, context, mdl);
+    const IntermediateBlueprint * root = dynamic_cast<const T1 *>(query.peekRoot());
+    ASSERT_TRUE(root != nullptr);
+    EXPECT_EQUAL(2u, root->childCnt());
+    const IntermediateBlueprint * second = dynamic_cast<const T2 *>(&root->getChild(0));
+    ASSERT_TRUE(second != nullptr);
+    EXPECT_EQUAL(2u, second->childCnt());
+    auto first = dynamic_cast<const AndBlueprint *>(&second->getChild(0));
+    ASSERT_TRUE(first != nullptr);
+    EXPECT_EQUAL(2u, first->childCnt());
+    EXPECT_TRUE(dynamic_cast<const AndBlueprint *>(&first->getChild(0)));
+    auto bottom = dynamic_cast<const AndBlueprint *>(&first->getChild(0));
+    EXPECT_EQUAL(2u, bottom->childCnt());
+    EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&bottom->getChild(0)));
+    EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&bottom->getChild(1)));
+    EXPECT_TRUE(dynamic_cast<const SimpleBlueprint *>(&first->getChild(1)));
+    EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&second->getChild(1)));
+    EXPECT_TRUE(dynamic_cast<const FakeBlueprint *>(&root->getChild(1)));
+}
+
+void Test::requireThatLocationIsAddedTheCorrectPlace() {
+    {
+        QueryBuilder<ProtonNodeTypes> builder;
+        builder.addRank(2);
+        builder.addAndNot(2);
+        verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation<RankBlueprint, AndNotBlueprint>(builder);
+    }
+    {
+        QueryBuilder<ProtonNodeTypes> builder;
+        builder.addAndNot(2);
+        builder.addRank(2);
+        verifyThatRankBlueprintAndAndNotStaysOnTopAfterLocation<AndNotBlueprint, RankBlueprint>(builder);
+    }
+}
+
 void Test::requireThatQueryAddsLocation() {
     checkQueryAddsLocation(*this, "(2,10,10,3,0,1,0,0)");
 }
@@ -1035,6 +1095,7 @@ Test::Main()
     TEST_CALL(requireThatUnknownFieldActsEmpty);
     TEST_CALL(requireThatIllegalFieldsAreIgnored);
     TEST_CALL(requireThatQueryGluesEverythingTogether);
+    TEST_CALL(requireThatLocationIsAddedTheCorrectPlace);
     TEST_CALL(requireThatQueryAddsLocation);
     TEST_CALL(requireThatQueryAddsLocationCutoff);
     TEST_CALL(requireThatFakeFieldSearchDumpsDiffer);
diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
index 6ce77bfd738..3bac4746d4b 100644
--- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp
+++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp
@@ -50,8 +50,8 @@ inject(Node::UP query, Node::UP to_inject) {
         root.prepend(inject(root.stealFirst(), std::move(to_inject)));
     } else {
         auto new_root = std::make_unique<ProtonAnd>();
-        new_root->append(std::move(query));
         new_root->append(std::move(to_inject));
+        new_root->append(std::move(query));
         query = std::move(new_root);
     }
     return query;