Fixed some minor bugs in breadth-first-search for AQL traversal. Also added another test case for it.

arangod/Cluster/ClusterTraverser.cpp

@@ -280,12 +280,71 @@ void ClusterTraverser::ClusterEdgeGetter::getEdge(
 }
 
 void ClusterTraverser::ClusterEdgeGetter::getAllEdges(
-    std::string const& startVertex, std::vector<std::string>& result) {
+    std::string const& startVertex, std::vector<std::string>& result,
-  size_t* last = nullptr;
+    size_t depth) {
-  size_t idx = 0;
+  std::string collName;
-  do {
+  TRI_edge_direction_e dir;
-    getEdge(startVertex, result, last, idx);
+  size_t eColIdx = 0;
-  } while (last != nullptr);
+  std::vector<TraverserExpression*> expEdges;
+  auto found = _traverser->_expressions->find(depth);
+  if (found != _traverser->_expressions->end()) {
+    expEdges = found->second;
+  }
+
+  arangodb::GeneralResponse::ResponseCode responseCode;
+  VPackBuilder resultEdges;
+  std::unordered_set<std::string> verticesToFetch;
+  while (_traverser->_opts.getCollection(eColIdx++, collName, dir)) {
+    resultEdges.clear();
+    resultEdges.openObject();
+    int res = getFilteredEdgesOnCoordinator(
+        _traverser->_dbname, collName, startVertex, dir,
+        expEdges, responseCode, resultEdges);
+    if (res != TRI_ERROR_NO_ERROR) {
+      THROW_ARANGO_EXCEPTION(res);
+    }
+    resultEdges.close();
+    VPackSlice resSlice = resultEdges.slice();
+    VPackSlice edgesSlice = resSlice.get("edges");
+    VPackSlice statsSlice = resSlice.get("stats");
+
+    size_t read = arangodb::basics::VelocyPackHelper::getNumericValue<size_t>(
+        statsSlice, "scannedIndex", 0);
+    size_t filter = arangodb::basics::VelocyPackHelper::getNumericValue<size_t>(
+        statsSlice, "filtered", 0);
+    _traverser->_readDocuments += read;
+    _traverser->_filteredPaths += filter;
+    if (edgesSlice.isNone() || edgesSlice.length() == 0) {
+      // No edges found here
+      continue;
+    }
+    for (auto const& edge : VPackArrayIterator(edgesSlice)) {
+      std::string edgeId = arangodb::basics::VelocyPackHelper::getStringValue(
+          edge, StaticStrings::IdString.c_str(), "");
+      if (_traverser->_opts.uniqueEdges ==
+          TraverserOptions::UniquenessLevel::GLOBAL) {
+        // DO not push this edge on the stack.
+        if (_traverser->_edges.find(edgeId) != _traverser->_edges.end()) {
+          continue;
+        }
+      }
+      std::string fromId = arangodb::basics::VelocyPackHelper::getStringValue(
+          edge, StaticStrings::FromString.c_str(), "");
+      if (_traverser->_vertices.find(fromId) == _traverser->_vertices.end()) {
+        verticesToFetch.emplace(std::move(fromId));
+      }
+      std::string toId = arangodb::basics::VelocyPackHelper::getStringValue(
+          edge, StaticStrings::ToString.c_str(), "");
+      if (_traverser->_vertices.find(toId) == _traverser->_vertices.end()) {
+        verticesToFetch.emplace(std::move(toId));
+      }
+      VPackBuilder tmpBuilder;
+      tmpBuilder.add(edge);
+      _traverser->_edges.emplace(edgeId, tmpBuilder.steal());
+      result.emplace_back(std::move(edgeId));
+    }
+  }
+  _traverser->fetchVertices(verticesToFetch, depth + 1);
 }
 
 void ClusterTraverser::setStartVertex(std::string const& id) {
@@ -398,6 +457,27 @@ arangodb::traverser::TraversalPath* ClusterTraverser::next() {
 
   size_t countEdges = path.edges.size();
 
+  if (_opts.useBreadthFirst &&
+      _opts.uniqueVertices == TraverserOptions::UniquenessLevel::NONE &&
+      _opts.uniqueEdges == TraverserOptions::UniquenessLevel::PATH) {
+    // Only if we use breadth first
+    // and vertex uniqueness is not guaranteed
+    // We have to validate edges on path uniquness.
+    // Otherwise this situation cannot occur.
+    // If two edges are identical than at least their start or end vertex
+    // is on the path twice: A -> B <- A
+    for (size_t i = 0; i < countEdges; ++i) {
+      for (size_t j = i + 1; j < countEdges; ++j) {
+        if (path.edges[i] == path.edges[j]) {
+          // We found two idential edges. Prune.
+          // Next
+          _pruneNext = true;
+          return next();
+        }
+      }
+    }
+  }
+
   auto p = std::make_unique<ClusterTraversalPath>(this, path);
   if (countEdges < _opts.minDepth) {
     return next();

arangod/Cluster/ClusterTraverser.h

@@ -108,7 +108,7 @@ class ClusterTraverser : public Traverser {
     void getEdge(std::string const&, std::vector<std::string>&, size_t*&,
                  size_t&) override;
 
-    void getAllEdges(std::string const&, std::vector<std::string>&) override;
+    void getAllEdges(std::string const&, std::vector<std::string>&, size_t depth) override;
 
    private:
     ClusterTraverser* _traverser;

arangod/V8Server/v8-vocbase.cpp

@@ -1815,220 +1815,6 @@ static v8::Handle<v8::Value> VertexIdsToV8(v8::Isolate* isolate,
   return scope.Escape<v8::Value>(vertices);
 }
 
-
-////////////////////////////////////////////////////////////////////////////////
-/// @brief Executes a Neighbors computation
-////////////////////////////////////////////////////////////////////////////////
-
-static void JS_QueryNeighbors(v8::FunctionCallbackInfo<v8::Value> const& args) {
-  TRI_V8_TRY_CATCH_BEGIN(isolate);
-  v8::HandleScope scope(isolate);
-
-  if (args.Length() < 3 || args.Length() > 4) {
-    TRI_V8_THROW_EXCEPTION_USAGE(
-        "CPP_NEIGHBORS(<vertexcollections[]>, <edgecollections[]>, <start>, "
-        "<options>)");
-  }
-
-  // get the vertex collections
-  if (!args[0]->IsArray()) {
-    TRI_V8_THROW_TYPE_ERROR("expecting array for <vertexcollections[]>");
-  }
-  std::unordered_set<std::string> vertexCollectionNames;
-  V8ArrayToStrings(args[0], vertexCollectionNames);
-
-  // get the edge collections
-  if (!args[1]->IsArray()) {
-    TRI_V8_THROW_TYPE_ERROR("expecting array for <edgecollections[]>");
-  }
-  std::unordered_set<std::string> edgeCollectionNames;
-  V8ArrayToStrings(args[1], edgeCollectionNames);
-
-  TRI_vocbase_t* vocbase = GetContextVocBase(isolate);
-
-  if (vocbase == nullptr) {
-    TRI_V8_THROW_EXCEPTION(TRI_ERROR_ARANGO_DATABASE_NOT_FOUND);
-  }
-
-  std::vector<std::string> startVertices;
-  if (args[2]->IsString()) {
-    startVertices.emplace_back(TRI_ObjectToString(args[2]));
-  } else if (args[2]->IsArray()) {
-    auto list = v8::Handle<v8::Array>::Cast(args[2]);
-    for (uint32_t i = 0; i < list->Length(); i++) {
-      if (list->Get(i)->IsString()) {
-        startVertices.emplace_back(TRI_ObjectToString(list->Get(i)));
-      } else {
-        TRI_V8_THROW_TYPE_ERROR("expecting array of IDs for <startVertex>");
-      }
-    }
-  } else {
-    TRI_V8_THROW_TYPE_ERROR("expecting string ID for <startVertex>");
-  }
-
-  std::vector<std::string> readCollections;
-  std::vector<std::string> writeCollections;
-
-  auto transactionContext =
-      std::make_shared<V8TransactionContext>(vocbase, true);
-
-  int res = TRI_ERROR_NO_ERROR;
-
-  for (auto const& it : edgeCollectionNames) {
-    readCollections.emplace_back(it);
-  }
-  for (auto const& it : vertexCollectionNames) {
-    readCollections.emplace_back(it);
-  }
-
-  std::unique_ptr<ExplicitTransaction> trx;
-
-  try {
-    trx.reset(BeginTransaction(transactionContext, readCollections,
-                               writeCollections));
-  } catch (Exception& e) {
-    TRI_V8_THROW_EXCEPTION(e.code());
-  }
-
-  traverser::NeighborsOptions opts(trx.get());
-  bool includeData = false;
-  v8::Handle<v8::Value> edgeExample;
-  v8::Handle<v8::Value> vertexExample;
-
-  if (args.Length() == 4) {
-    if (!args[3]->IsObject()) {
-      TRI_V8_THROW_TYPE_ERROR("expecting json for <options>");
-    }
-    v8::Handle<v8::Object> options = args[3]->ToObject();
-
-    // Parse direction
-    v8::Local<v8::String> keyDirection = TRI_V8_ASCII_STRING("direction");
-    if (options->Has(keyDirection)) {
-      std::string dir = TRI_ObjectToString(options->Get(keyDirection));
-      if (dir == "outbound") {
-        opts.direction = TRI_EDGE_OUT;
-      } else if (dir == "inbound") {
-        opts.direction = TRI_EDGE_IN;
-      } else if (dir == "any") {
-        opts.direction = TRI_EDGE_ANY;
-      } else {
-        TRI_V8_THROW_TYPE_ERROR(
-            "expecting direction to be 'outbound', 'inbound' or 'any'");
-      }
-    }
-
-    // Parse includeData
-    v8::Local<v8::String> keyIncludeData = TRI_V8_ASCII_STRING("includeData");
-    if (options->Has(keyIncludeData)) {
-      includeData = TRI_ObjectToBoolean(options->Get(keyIncludeData));
-    }
-
-    // Parse filterEdges
-    v8::Local<v8::String> keyFilterEdges = TRI_V8_ASCII_STRING("filterEdges");
-    if (options->Has(keyFilterEdges)) {
-      opts.useEdgeFilter = true;
-      edgeExample = options->Get(keyFilterEdges);
-    }
-
-    // Parse vertexFilter
-    v8::Local<v8::String> keyFilterVertices =
-        TRI_V8_ASCII_STRING("filterVertices");
-    if (options->Has(keyFilterVertices)) {
-      opts.useVertexFilter = true;
-      // note: only works with vertex examples and not with user-defined AQL
-      // functions
-      vertexExample =
-          v8::Handle<v8::Object>::Cast(options->Get(keyFilterVertices));
-    }
-
-    // Parse minDepth
-    v8::Local<v8::String> keyMinDepth = TRI_V8_ASCII_STRING("minDepth");
-    if (options->Has(keyMinDepth)) {
-      opts.minDepth = TRI_ObjectToUInt64(options->Get(keyMinDepth), false);
-    }
-
-    // Parse maxDepth
-    v8::Local<v8::String> keyMaxDepth = TRI_V8_ASCII_STRING("maxDepth");
-    if (options->Has(keyMaxDepth)) {
-      opts.maxDepth = TRI_ObjectToUInt64(options->Get(keyMaxDepth), false);
-    }
-  }
-
-  std::vector<EdgeCollectionInfo*> edgeCollectionInfos;
-
-  arangodb::basics::ScopeGuard guard{[]() -> void {},
-                                     [&edgeCollectionInfos]() -> void {
-                                       for (auto& p : edgeCollectionInfos) {
-                                         delete p;
-                                       }
-                                     }};
-
-  for (auto const& it : edgeCollectionNames) {
-    edgeCollectionInfos.emplace_back(
-        new arangodb::traverser::EdgeCollectionInfo(
-            trx.get(), it, opts.direction, "", 1));
-    TRI_IF_FAILURE("EdgeCollectionDitchOOM") {
-      THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
-    }
-  }
-
-  for (auto const& it : vertexCollectionNames) {
-    opts.addCollectionRestriction(it);
-  }
-
-  if (opts.useEdgeFilter) {
-    std::string errorMessage;
-    for (auto const& it : edgeCollectionInfos) {
-      try {
-        opts.addEdgeFilter(isolate, edgeExample, it->getName(), errorMessage);
-      } catch (Exception& e) {
-        // ELEMENT not found is expected, if there is no shape of this type in
-        // this collection
-        if (e.code() != TRI_RESULT_ELEMENT_NOT_FOUND) {
-          TRI_V8_THROW_EXCEPTION(e.code());
-        }
-      }
-    }
-  }
-
-  if (opts.useVertexFilter) {
-    std::string errorMessage;
-    for (auto const& it : vertexCollectionNames) {
-      try {
-        opts.addVertexFilter(isolate, vertexExample, trx.get(), it,
-                             errorMessage);
-      } catch (Exception& e) {
-        // ELEMENT not found is expected, if there is no shape of this type in
-        // this collection
-        if (e.code() != TRI_RESULT_ELEMENT_NOT_FOUND) {
-          TRI_V8_THROW_EXCEPTION(e.code());
-        }
-      }
-    }
-  }
-
-  std::vector<VPackSlice> neighbors;
-  std::unordered_set<VPackSlice,
-                     arangodb::basics::VelocyPackHelper::VPackStringHash,
-                     arangodb::basics::VelocyPackHelper::VPackStringEqual> visited;
-  for (auto const& startVertex : startVertices) {
-    opts.setStart(startVertex);
-    try {
-      TRI_RunNeighborsSearch(edgeCollectionInfos, opts, visited, neighbors);
-    } catch (Exception& e) {
-      trx->finish(e.code());
-      TRI_V8_THROW_EXCEPTION(e.code());
-    }
-  }
-
-  auto result = VertexIdsToV8(isolate, trx.get(), neighbors, includeData);
-
-  trx->finish(res);
-
-  TRI_V8_RETURN(result);
-  TRI_V8_TRY_CATCH_END
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief sleeps and checks for query abortion in between
 ////////////////////////////////////////////////////////////////////////////////
@@ -3233,10 +3019,6 @@ void TRI_InitV8VocBridge(v8::Isolate* isolate, v8::Handle<v8::Context> context,
       isolate, context, TRI_V8_ASCII_STRING("THROW_COLLECTION_NOT_LOADED"),
       JS_ThrowCollectionNotLoaded, true);
 
-  TRI_AddGlobalFunctionVocbase(isolate, context,
-                               TRI_V8_ASCII_STRING("CPP_NEIGHBORS"),
-                               JS_QueryNeighbors, true);
-
   TRI_InitV8Replication(isolate, context, server, vocbase, threadNumber, v8g);
 
   TRI_AddGlobalFunctionVocbase(isolate, context,

arangod/VocBase/SingleServerTraverser.cpp

@@ -23,6 +23,7 @@
 
 #include "SingleServerTraverser.h"
 #include "Utils/OperationCursor.h"
+#include "VocBase/MasterPointer.h"
 #include "VocBase/SingleServerTraversalPath.h"
 
 using namespace arangodb::traverser;
@@ -268,7 +269,28 @@ TraversalPath* SingleServerTraverser::next() {
       return next();
     }
   }
+
   size_t countEdges = path.edges.size();
+  if (_opts.useBreadthFirst &&
+      _opts.uniqueVertices == TraverserOptions::UniquenessLevel::NONE &&
+      _opts.uniqueEdges == TraverserOptions::UniquenessLevel::PATH) {
+    // Only if we use breadth first
+    // and vertex uniqueness is not guaranteed
+    // We have to validate edges on path uniquness.
+    // Otherwise this situation cannot occur.
+    // If two edges are identical than at least their start or end vertex
+    // is on the path twice: A -> B <- A
+    for (size_t i = 0; i < countEdges; ++i) {
+      for (size_t j = i + 1; j < countEdges; ++j) {
+        if (path.edges[i] == path.edges[j]) {
+          // We found two idential edges. Prune.
+          // Next
+          _pruneNext = true;
+          return next();
+        }
+      }
+    }
+  }
 
   auto p = std::make_unique<SingleServerTraversalPath>(path, this);
   if (countEdges < _opts.minDepth) {
@@ -358,6 +380,12 @@ void SingleServerTraverser::EdgeGetter::nextEdge(
     }
     edge = edge.at(*last);
     if (!_traverser->edgeMatchesConditions(edge, edges.size())) {
+      if (_opts.uniqueEdges == TraverserOptions::UniquenessLevel::GLOBAL) {
+        // Insert a dummy to please the uniqueness
+        _traverser->_edges.emplace(_trx->extractIdString(edge), nullptr);
+      }
+
+      ++_traverser->_filteredPaths; 
       TRI_ASSERT(last != nullptr);
       (*last)++;
       continue;
@@ -403,11 +431,47 @@ void SingleServerTraverser::EdgeGetter::getEdge(std::string const& startVertex,
   nextEdge(startVertex, eColIdx, last, edges);
 }
 
-void SingleServerTraverser::EdgeGetter::getAllEdges(std::string const& startVertex,
+void SingleServerTraverser::EdgeGetter::getAllEdges(
-                                                std::vector<std::string>& edges) {
+    std::string const& startVertex, std::vector<std::string>& edges,
-  VPackValueLength* last = nullptr;
+    size_t depth) {
-  size_t idx = 0;
+  size_t idxId = 0;
-  do {
+  std::string eColName;
-    getEdge(startVertex, edges, last, idx);
+  arangodb::Transaction::IndexHandle indexHandle;
-  } while (last != nullptr);
+  std::vector<TRI_doc_mptr_t*> mptrs;
+
+  // We iterate over all index ids. note idxId++
+  while (_opts.getCollectionAndSearchValue(idxId++, startVertex, eColName,
+                                           indexHandle, _builder)) {
+    std::shared_ptr<OperationCursor> cursor = _trx->indexScan(
+        eColName, arangodb::Transaction::CursorType::INDEX, indexHandle,
+        _builder.slice(), 0, UINT64_MAX, Transaction::defaultBatchSize(), false);
+    if (cursor->failed()) {
+      // Some error, ignore and go to next
+      continue;
+    }
+    while (cursor->hasMore()) {
+      mptrs.clear();
+      cursor->getMoreMptr(mptrs, UINT64_MAX);
+
+      _traverser->_readDocuments += static_cast<size_t>(mptrs.size());
+      for (auto const& mptr : mptrs) {
+        VPackSlice edge(mptr->vpack());
+        if (!_traverser->edgeMatchesConditions(edge, depth)) {
+          if (_opts.uniqueEdges == TraverserOptions::UniquenessLevel::GLOBAL) {
+            // Insert a dummy to please the uniqueness
+            _traverser->_edges.emplace(_trx->extractIdString(edge), nullptr);
+          }
+          ++_traverser->_filteredPaths; 
+          continue;
+        }
+        std::string id = _trx->extractIdString(edge);
+
+        // TODO Optimize. May use VPack everywhere.
+        VPackBuilder tmpBuilder = VPackBuilder::clone(edge);
+        _traverser->_edges.emplace(id, tmpBuilder.steal());
+
+        edges.emplace_back(id);
+      }
+    }
+  }
 }

arangod/VocBase/SingleServerTraverser.h

@@ -91,7 +91,7 @@ class SingleServerTraverser : public Traverser {
     void getEdge(std::string const&, std::vector<std::string>&,
                  arangodb::velocypack::ValueLength*&, size_t&) override;
 
-    void getAllEdges(std::string const&, std::vector<std::string>&) override;
+    void getAllEdges(std::string const&, std::vector<std::string>&, size_t) override;
 
 
    private:

js/server/tests/aql/aql-graph.js

@@ -613,7 +613,7 @@ function ahuacatlQueryNeighborsTestSuite () {
       var v6 = "UnitTestsAhuacatlVertex/v6";
       var v7 = "UnitTestsAhuacatlVertex/v7";
       var createQuery = function (start, filter) {
-        return `FOR n, e IN OUTBOUND "${start}" UnitTestsAhuacatlEdge ${filter} SORT n._id RETURN n._id`;
+        return `FOR n, e IN OUTBOUND "${start}" UnitTestsAhuacatlEdge OPTIONS {bfs: true} ${filter} SORT n._id RETURN n._id`;
       };
 
       // An empty filter should let all edges through
@@ -641,7 +641,83 @@ function ahuacatlQueryNeighborsTestSuite () {
       actual = getQueryResults(createQuery(v3, `FILTER e._to == "${v4}"`));
       assertEqual(actual, [ v4 ]);
     }
+  };
+}
 
+function ahuacatlQueryBreadthFirstTestSuite () {
+  let vertex = null;
+  let edge   = null;
+  const vn = "UnitTestsAhuacatlVertex";
+  const en = "UnitTestsAhuacatlEdge";
+  const center = vn + "/A";
+
+  let cleanUp = function () {
+    db._drop(vn);
+    db._drop(en);
+  };
+
+  return {
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief set up
+///
+///
+/// Graph Under Test:
+///  +---------+---------+
+/// \|/        |        \|/
+///  D <- B <- A -> E -> F
+///       |         |
+///       +--> C <--+
+////////////////////////////////////////////////////////////////////////////////
+
+    setUp : function () {
+      cleanUp();
+
+      vertex = db._create(vn);
+      edge = db._createEdgeCollection(en);
+      
+      vertex.save({_key: "A"});
+      vertex.save({_key: "B"});
+      vertex.save({_key: "C"});
+      vertex.save({_key: "D"});
+      vertex.save({_key: "E"});
+      vertex.save({_key: "F"});
+
+      let makeEdge = function(from, to) {
+        edge.save({
+          _from: vn + "/" + from,
+          _to: vn + "/" + to, 
+          _key: from + "" + to
+        });
+      };
+
+      makeEdge("A", "B");
+      makeEdge("A", "D");
+      makeEdge("A", "E");
+      makeEdge("A", "F");
+
+      makeEdge("B", "C");
+      makeEdge("B", "D");
+
+      makeEdge("E", "C");
+      makeEdge("E", "F");
+    },
+
+    tearDown : cleanUp,
+
+    testUniqueVerticesMinDepth2 : function () {
+      var query = `
+        FOR n IN 2..2 OUTBOUND "${center}" ${en}
+        OPTIONS {bfs: true, uniqueVertices: 'global'}
+        SORT n._key RETURN n._key`;
+      var actual;
+
+      // A is directly connected to every other vertex accept "C"
+      // So we expect only C to be returned.
+      actual = getQueryResults(query);
+      assertEqual(actual.length, 1);
+      assertEqual(actual, [ "C" ]);
+    }
   };
 }
 
@@ -993,6 +1069,7 @@ function ahuacatlQueryShortestpathErrorsSuite () {
 
 jsunity.run(ahuacatlQueryEdgesTestSuite);
 jsunity.run(ahuacatlQueryNeighborsTestSuite);
+jsunity.run(ahuacatlQueryBreadthFirstTestSuite);
 jsunity.run(ahuacatlQueryShortestPathTestSuite);
 if (internal.debugCanUseFailAt() && ! cluster.isCluster()) {
   jsunity.run(ahuacatlQueryNeighborsErrorsSuite);

lib/Basics/Traverser.h

@@ -58,7 +58,8 @@ struct EdgeGetter {
   virtual void getEdge(vertexIdentifier const&, std::vector<edgeIdentifier>&,
                        edgeItem*&, size_t&) = 0;
 
-  virtual void getAllEdges(vertexIdentifier const&, std::vector<edgeIdentifier>&) = 0;
+  virtual void getAllEdges(vertexIdentifier const&,
+                           std::vector<edgeIdentifier>&, size_t) = 0;
 };
 
 
@@ -395,7 +396,7 @@ class BreadthFirstEnumerator : public PathEnumerator<edgeIdentifier, vertexIdent
 
       _tmpEdges.clear();
       auto& next = _toSearch[_toSearchPos++];
-      this->_edgeGetter->getAllEdges(next->vertex, _tmpEdges);
+      this->_edgeGetter->getAllEdges(next->vertex, _tmpEdges, _currentDepth);
       if (!_tmpEdges.empty()) {
         bool didInsert = false;
         for (auto const& e : _tmpEdges) {