fix tests

arangodb · gnusi · Jan 10, 2019 · Dec 21, 2018 · Dec 23, 2018 · Dec 24, 2018
commit 31d273c3c2837f1592a7f186d2da231295927a16
diff --git a/arangod/IResearch/AqlHelper.cpp b/arangod/IResearch/AqlHelper.cpp
@@ -161,14 +161,19 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
     case aql::NODE_TYPE_ATTRIBUTE_ACCESS: {
       hash = fasthash64(static_cast<const void*>("access"), 6, hash);
 
+      // FIXME
+
       struct Visitor {
         explicit Visitor(size_t& hash) noexcept
           : hash(hash) {
         }
 
         bool attributeAccess(arangodb::aql::AstNode const& node) noexcept {
+          irs::string_ref value;
+          parseValue(value, node);
+
           hash = fasthash64(static_cast<const void*>("attribute"), 9, hash);
-          hash = iresearch::hash(&node, hash);
+          hash = fasthash64(value.c_str(), value.size(), hash);
           return true;
         }
 
@@ -189,6 +194,10 @@ size_t hash(aql::AstNode const* node, size_t hash /*= 0*/) noexcept {
       aql::AstNode const* head = nullptr;
       visitAttributeAccess(head, node, hasher);
 
+      if (head) {
+        hash = fasthash64(head, sizeof(head), hash);
+      }
+
       return hash;
     }
 

diff --git a/arangod/IResearch/AqlHelper.h b/arangod/IResearch/AqlHelper.h
@@ -443,74 +443,6 @@ bool visitAttributeAccess(
   }
 }
 
-////////////////////////////////////////////////////////////////////////////////
-/// @brief interprets the specified node as an attribute path description and
-///        visits the members in attribute path order calling the provided
-///        'visitor' on each path sub-index, expecting the following signatures:
-///          bool operator()(irs::string_ref) - string keys
-///          bool operator()(int64_t)         - array offsets
-///          bool operator()()                - any string key or numeric offset
-/// @return success and set head the the starting node of path (reference/value)
-////////////////////////////////////////////////////////////////////////////////
-template<typename T>
-bool visitAttributePath(
-  aql::AstNode const*& head,
-  aql::AstNode const& node,
-  T& visitor
-) {
-  if (node.numMembers() >= 2
-      && aql::NODE_TYPE_EXPANSION == node.type) { // [*]
-    auto* itr = node.getMemberUnchecked(0);
-    auto* ref = node.getMemberUnchecked(1);
-
-    if (itr && itr->numMembers() == 2) {
-      auto* root = itr->getMemberUnchecked(1);
-      auto* var = itr->getMemberUnchecked(0);
-
-      return ref
-        && aql::NODE_TYPE_ITERATOR == itr->type
-        && aql::NODE_TYPE_REFERENCE == ref->type
-        && root && var
-        && aql::NODE_TYPE_VARIABLE == var->type
-        && visitAttributePath(head, *root, visitor) // 1st visit root
-        && visitor(); // 2nd visit current node
-    }
-  } else if (node.numMembers() == 2
-             && aql::NODE_TYPE_INDEXED_ACCESS == node.type) { // [<something>]
-    auto* root = node.getMemberUnchecked(0);
-    auto* offset = node.getMemberUnchecked(1);
-
-    if (offset && offset->isIntValue()) {
-      return root
-        && offset->getIntValue() >= 0
-        && visitAttributePath(head, *root, visitor) // 1st visit root
-        && visitor(offset->getIntValue()); // 2nd visit current node
-    }
-
-    return root && offset && offset->isStringValue()
-      && visitAttributePath(head, *root, visitor) // 1st visit root
-      && visitor(iresearch::getStringRef(*offset)); // 2nd visit current node
-  } else if (node.numMembers() == 1
-             && aql::NODE_TYPE_ATTRIBUTE_ACCESS == node.type) {
-    auto* root = node.getMemberUnchecked(0);
-
-    return root
-      && aql::VALUE_TYPE_STRING == node.value.type
-      && visitAttributePath(head, *root, visitor) // 1st visit root
-      && visitor(iresearch::getStringRef(node)); // 2nd visit current node
-  } else if (!node.numMembers()) { // end of attribute path (base case)
-    head = &node;
-
-    return aql::NODE_TYPE_REFERENCE == node.type
-      || (aql::NODE_TYPE_VALUE == node.type
-          && aql::VALUE_TYPE_STRING == node.value.type
-          && visitor(iresearch::getStringRef(node))
-         );
-  }
-
-  return false;
-}
-
 struct NormalizedCmpNode {
   aql::AstNode const* attribute;
   aql::AstNode const* value;

diff --git a/arangod/IResearch/IResearchOrderFactory.cpp b/arangod/IResearch/IResearchOrderFactory.cpp
@@ -278,7 +278,7 @@ void ScorerReplacer::replace(aql::CalculationNode& node) {
         return node;
       }
 
-      Scorer const key(ref, node);
+      HashedScorer const key(ref, node);
 
       auto it = _dedup.find(key);
 

diff --git a/arangod/IResearch/IResearchOrderFactory.h b/arangod/IResearch/IResearchOrderFactory.h
@@ -148,20 +148,31 @@ class ScorerReplacer {
   }
 
  private:
+  struct HashedScorer : Scorer {
+    HashedScorer(
+      aql::Variable const* var,
+      aql::AstNode const* node
+    ) : Scorer(var, node),
+        hash(iresearch::hash(node)) {
+    }
+
+    size_t hash;
+  }; // HashedScorer
+
   struct ScorerHash {
-    size_t operator()(Scorer const& key) const noexcept {
-      return iresearch::hash(key.node);
+    size_t operator()(HashedScorer const& key) const noexcept {
+      return key.hash;
     }
   }; // ScorerHash
 
   struct ScorerEqualTo {
-    bool operator()(Scorer const& lhs, Scorer const& rhs) const {
+    bool operator()(HashedScorer const& lhs, HashedScorer const& rhs) const {
       return iresearch::equalTo(lhs.node, rhs.node);
     }
   }; // ScorerEqualTo
 
   typedef std::unordered_map<
-    Scorer, aql::Variable const*, ScorerHash, ScorerEqualTo
+    HashedScorer, aql::Variable const*, ScorerHash, ScorerEqualTo
   > DedupScorers;
 
   DedupScorers _dedup;

diff --git a/arangod/IResearch/IResearchViewOptimizerRules.cpp b/arangod/IResearch/IResearchViewOptimizerRules.cpp
@@ -217,7 +217,7 @@ void handleViewsRule(
     auto const funcName = iresearch::getFuncName(*scorer.node);
 
     THROW_ARANGO_EXCEPTION_FORMAT(
-      TRI_ERROR_QUERY_PARSE,
+      TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH,
       "Non ArangoSearch view variable '%s' is used in scorer function '%s'",
       scorer.var->name.c_str(),
       funcName.c_str()

diff --git a/tests/IResearch/IResearchQueryJoin-test.cpp b/tests/IResearch/IResearchQueryJoin-test.cpp
@@ -1362,12 +1362,6 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
   }
 
   // invalid reference in scorer
-  //
-  // FOR x IN 0..5
-  //   FOR d IN testView
-  //   SEARCH d.seq == x
-  //   SORT customscorer(x,x)
-  // RETURN d;
   {
     std::string const query = "FOR d IN testView FOR i IN 0..5 SORT tfidf(i) DESC RETURN d";
 
@@ -1376,7 +1370,7 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
     ));
 
     auto queryResult = arangodb::tests::executeQuery(vocbase, query);
-    REQUIRE(TRI_ERROR_NOT_IMPLEMENTED == queryResult.code);
+    REQUIRE(TRI_ERROR_QUERY_FUNCTION_ARGUMENT_TYPE_MISMATCH == queryResult.code);
   }
 
   // FOR i IN 1..5
@@ -1457,6 +1451,85 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
     CHECK(expectedDoc == expectedDocs.end());
   }
 
+  {
+    std::string const query = "LET attr = _NONDETERM_('seq') "
+                              "FOR i IN 1..5 "
+                              "  FOR x IN collection_1 FILTER x.seq == i "
+                              "    FOR d IN testView SEARCH d.seq == x.seq AND d.name == x.name "
+                              "      SORT customscorer(d, x[attr]) DESC "
+                              "RETURN d";
+
+    CHECK(arangodb::tests::assertRules(
+      vocbase, query,
+      {
+        arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
+      }
+    ));
+
+    std::vector<arangodb::velocypack::Slice> expectedDocs {
+      arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
+      arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
+    };
+
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(expectedDocs.size() == resultIt.size());
+
+    // Check documents
+    auto expectedDoc = expectedDocs.begin();
+    for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
+      auto const actualDoc = resultIt.value();
+      auto const resolved = actualDoc.resolveExternals();
+
+      CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
+    }
+    CHECK(expectedDoc == expectedDocs.end());
+  }
+
+  // FOR i IN 1..5
+  //  FOR x IN collection_0 SEARCH x.seq == i
+  //    FOR d IN  SEARCH d.seq == x.seq && d.name == x.name
+  // SORT customscorer(d, x.seq)
+  {
+    std::string const query = "FOR i IN 1..5 FOR x IN collection_1 FILTER x.seq == i FOR d IN testView SEARCH d.seq == x.seq AND d.name == x.name SORT customscorer(d, x['seq']) DESC RETURN d";
+
+    CHECK(arangodb::tests::assertRules(
+      vocbase, query,
+      {
+        arangodb::aql::OptimizerRule::handleArangoSearchViewsRule,
+      }
+    ));
+
+    std::vector<arangodb::velocypack::Slice> expectedDocs {
+      arangodb::velocypack::Slice(insertedDocsView[4].vpack()),
+      arangodb::velocypack::Slice(insertedDocsView[2].vpack()),
+    };
+
+    auto queryResult = arangodb::tests::executeQuery(vocbase, query);
+    REQUIRE(TRI_ERROR_NO_ERROR == queryResult.code);
+
+    auto result = queryResult.result->slice();
+    CHECK(result.isArray());
+
+    arangodb::velocypack::ArrayIterator resultIt(result);
+    REQUIRE(expectedDocs.size() == resultIt.size());
+
+    // Check documents
+    auto expectedDoc = expectedDocs.begin();
+    for (;resultIt.valid(); resultIt.next(), ++expectedDoc) {
+      auto const actualDoc = resultIt.value();
+      auto const resolved = actualDoc.resolveExternals();
+
+      CHECK((0 == arangodb::basics::VelocyPackHelper::compare(arangodb::velocypack::Slice(*expectedDoc), resolved, true)));
+    }
+    CHECK(expectedDoc == expectedDocs.end());
+  }
+
   // unable to retrieve `d.seq` from self-referenced variable
   // FOR i IN 1..5
   //  FOR d IN  SEARCH d.seq == i SORT customscorer(d, d.seq)
@@ -1687,4 +1760,4 @@ TEST_CASE("IResearchQueryTestJoin", "[iresearch][iresearch-query]") {
 
 // -----------------------------------------------------------------------------
 // --SECTION--                                                       END-OF-FILE
-// -----------------------------------------------------------------------------
+// -----------------------------------------------------------------------------