fix deduplication for indexed access

arangodb · gnusi · Jan 10, 2019 · Dec 21, 2018 · Dec 23, 2018 · Dec 24, 2018
commit 4030503d1df5c0e9290716669f534f32ac8669ce
diff --git a/arangod/IResearch/AqlHelper.cpp b/arangod/IResearch/AqlHelper.cpp
@@ -113,7 +113,9 @@ bool equalTo(aql::AstNode const* lhs, aql::AstNode const* rhs) {
       return equalMembers(lhs, rhs);
     }
 
-    case aql::NODE_TYPE_ATTRIBUTE_ACCESS: {
+    case aql::NODE_TYPE_ATTRIBUTE_ACCESS:
+    case aql::NODE_TYPE_INDEXED_ACCESS:
+    case aql::NODE_TYPE_EXPANSION: {
       return attributeAccessEqual(lhs, rhs, nullptr);
     }
 
@@ -214,7 +216,6 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
     }
 
     case aql::NODE_TYPE_INDEXED_ACCESS: {
-      hash = aql::AstNode(node->value).hashValue(hash);
       return hashMembers(*node, hash);
     }
 
@@ -484,14 +485,16 @@ bool attributeAccessEqual(arangodb::aql::AstNode const* lhs,
         if (root && offset) {
           aqlValue.reset(*offset);
 
-          if (!ctx) {
-            // can't evaluate expression at compile time
-            return true;
-          }
+          if (!aqlValue.isConstant()) {
+            if (!ctx) {
+              // can't evaluate expression at compile time
+              return true;
+            }
 
-          if (!aqlValue.execute(*ctx)) {
-            // failed to execute expression
-            return false;
+            if (!aqlValue.execute(*ctx)) {
+              // failed to execute expression
+              return false;
+            }
           }
 
           switch (aqlValue.type()) {

diff --git a/tests/IResearch/IResearchQueryScorer-test.cpp b/tests/IResearch/IResearchQueryScorer-test.cpp
@@ -464,7 +464,7 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") {
   {
     std::string const queryString =
       "LET i = 1"
-      "FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
+      "FOR d IN testView SEARCH d.name >= 'A' AND d.name < 'B' "
       "RETURN [ customscorer(d, i), customscorer(d, 1) ] ";
 
     CHECK(arangodb::tests::assertRules(
@@ -540,12 +540,36 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") {
       REQUIRE(arangodb::aql::NODE_TYPE_REFERENCE == sub->type);
       CHECK(static_cast<const void*>(var) == sub->getData());
     }
+
+    // check execution
+    auto queryResult = arangodb::tests::executeQuery(vocbase, queryString);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(1 == resultIt.size());
+
+    for (;resultIt.valid(); resultIt.next()) {
+      auto const actualValue = resultIt.value();
+      REQUIRE(actualValue.isArray());
+
+      VPackArrayIterator scoreIt(actualValue);
+      CHECK(2 == scoreIt.size());
+
+      for (; scoreIt.valid(); scoreIt.next()) {
+        auto const value = scoreIt.value();
+        REQUIRE(value.isNumber());
+        CHECK(1 == value.getNumber<size_t>());
+      }
+    }
   }
 
   // ensure scorers are deduplicated (attribute access)
   {
     std::string const queryString =
-      "LET obj = _NONDETERM_({ value : 0 })"
+      "LET obj = _NONDETERM_({ value : 2 }) "
       "FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
       "RETURN [ customscorer(d, obj.value), customscorer(d, obj.value) ] ";
 
@@ -627,12 +651,36 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") {
         CHECK(static_cast<const void*>(var) == sub->getData());
       }
     }
+
+    // check execution
+    auto queryResult = arangodb::tests::executeQuery(vocbase, queryString);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(3 == resultIt.size());
+
+    for (;resultIt.valid(); resultIt.next()) {
+      auto const actualValue = resultIt.value();
+      REQUIRE(actualValue.isArray());
+
+      VPackArrayIterator scoreIt(actualValue);
+      CHECK(2 == scoreIt.size());
+
+      for (; scoreIt.valid(); scoreIt.next()) {
+        auto const value = scoreIt.value();
+        REQUIRE(value.isNumber());
+            CHECK(2 == value.getNumber<size_t>());
+      }
+    }
   }
 
   // ensure scorers are deduplicated (expression)
   {
     std::string const queryString =
-      "LET obj = _NONDETERM_({ value : 0 })"
+      "LET obj = _NONDETERM_({ value : 2 }) "
       "FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
       "RETURN [ customscorer(d, obj.value+1), customscorer(d, obj.value+1) ] ";
 
@@ -714,6 +762,141 @@ TEST_CASE("IResearchQueryScorer", "[iresearch][iresearch-query]") {
         CHECK(static_cast<const void*>(var) == sub->getData());
       }
     }
+
+    // check execution
+    auto queryResult = arangodb::tests::executeQuery(vocbase, queryString);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(3 == resultIt.size());
+
+    for (;resultIt.valid(); resultIt.next()) {
+      auto const actualValue = resultIt.value();
+      REQUIRE(actualValue.isArray());
+
+      VPackArrayIterator scoreIt(actualValue);
+      CHECK(2 == scoreIt.size());
+
+      for (; scoreIt.valid(); scoreIt.next()) {
+        auto const value = scoreIt.value();
+        REQUIRE(value.isNumber());
+        CHECK(3 == value.getNumber<size_t>());
+      }
+    }
+  }
+
+  // ensure scorers are deduplicated (expression)
+  {
+    std::string const queryString =
+      "LET obj = _NONDETERM_([ 2, 5 ]) "
+      "FOR d IN testView SEARCH d.name >= 'A' AND d.name <= 'C' "
+      "RETURN [ customscorer(d, obj[1]), customscorer(d, obj[1]) ] ";
+
+    CHECK(arangodb::tests::assertRules(
+      vocbase, queryString, {
+        arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
+      }
+    ));
+
+    arangodb::aql::Query query(
+      false,
+      vocbase,
+      arangodb::aql::QueryString(queryString),
+      std::shared_ptr<arangodb::velocypack::Builder>(),
+      arangodb::velocypack::Parser::fromJson("{}"),
+      arangodb::aql::PART_MAIN
+    );
+
+    query.prepare(arangodb::QueryRegistryFeature::registry());
+    auto* plan = query.plan();
+    REQUIRE(plan);
+
+    arangodb::SmallVector<arangodb::aql::ExecutionNode*>::allocator_type::arena_type a;
+    arangodb::SmallVector<arangodb::aql::ExecutionNode*> nodes{a};
+
+    // only one scorer
+    plan->findNodesOfType(nodes, arangodb::aql::ExecutionNode::ENUMERATE_IRESEARCH_VIEW, true);
+    REQUIRE(1 == nodes.size());
+    auto* viewNode = arangodb::aql::ExecutionNode::castTo<arangodb::iresearch::IResearchViewNode*>(nodes.front());
+    REQUIRE(viewNode);
+    auto& scorers = viewNode->scorers();
+    REQUIRE(1 == scorers.size());
+    auto* var = scorers.front().var;
+    REQUIRE(var);
+
+    // check scorer
+    {
+      auto* expr = scorers.front().node;
+      REQUIRE(expr);
+      REQUIRE(arangodb::aql::NODE_TYPE_FCALL == expr->type);
+      auto* fn = static_cast<arangodb::aql::Function*>(expr->getData());
+      REQUIRE(fn);
+      REQUIRE(arangodb::iresearch::isScorer(*fn));
+      CHECK("CUSTOMSCORER" == fn->name);
+
+      REQUIRE(1 == expr->numMembers());
+      auto* args = expr->getMember(0);
+      REQUIRE(args);
+      REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == args->type);
+      REQUIRE(2 == args->numMembers());
+      auto* arg0 = args->getMember(0); // reference to d
+      REQUIRE(arg0);
+      REQUIRE(static_cast<void const*>(&viewNode->outVariable()) == arg0->getData());
+      auto* arg1 = args->getMember(1);
+      REQUIRE(arg1);
+      REQUIRE(arangodb::aql::NODE_TYPE_INDEXED_ACCESS == arg1->type);
+    }
+
+    // and 2 references
+    nodes.clear();
+    plan->findNodesOfType(nodes, arangodb::aql::ExecutionNode::CALCULATION, true);
+    REQUIRE(2 == nodes.size());
+    for (auto const* node : nodes) {
+      auto* calcNode = arangodb::aql::ExecutionNode::castTo<arangodb::aql::CalculationNode const*>(node);
+      REQUIRE(calcNode);
+      REQUIRE(calcNode->expression());
+
+      if (calcNode->outVariable()->name == "obj") {
+        continue;
+      }
+
+      auto* exprNode = calcNode->expression()->node();
+      REQUIRE(exprNode);
+      REQUIRE(arangodb::aql::NODE_TYPE_ARRAY == exprNode->type);
+      REQUIRE(2 == exprNode->numMembers());
+      for (size_t i = 0; i < exprNode->numMembers(); ++i) {
+        auto* sub = exprNode->getMember(i);
+        REQUIRE(arangodb::aql::NODE_TYPE_REFERENCE == sub->type);
+        CHECK(static_cast<const void*>(var) == sub->getData());
+      }
+    }
+
+    // check execution
+    auto queryResult = arangodb::tests::executeQuery(vocbase, queryString);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(3 == resultIt.size());
+
+    for (;resultIt.valid(); resultIt.next()) {
+      auto const actualValue = resultIt.value();
+      REQUIRE(actualValue.isArray());
+
+      VPackArrayIterator scoreIt(actualValue);
+      CHECK(2 == scoreIt.size());
+
+      for (; scoreIt.valid(); scoreIt.next()) {
+        auto const value = scoreIt.value();
+        REQUIRE(value.isNumber());
+        CHECK(5 == value.getNumber<size_t>());
+      }
+    }
   }
 
   // con't deduplicate scorers with default values