Merge branch 'devel' of https://github.com/triAGENS/ArangoDB into devel

arangod/Aql/Collection.cpp

@@ -111,6 +111,23 @@ std::vector<std::string> Collection::shardIds () const {
   return ids;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief returns the shard keys of a collection
+////////////////////////////////////////////////////////////////////////////////
+
+std::vector<std::string> Collection::shardKeys () const {
+  auto clusterInfo = triagens::arango::ClusterInfo::instance();
+  auto collectionInfo = clusterInfo->getCollection(std::string(vocbase->_name), name);
+  if (collectionInfo.get() == nullptr) {
+    THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "collection not found");
+  }
+
+  std::vector<std::string> keys;
+  for (auto const& x : collectionInfo.get()->shardKeys()) {
+    keys.emplace_back(x);
+  }
+  return keys;
+}
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief returns the indexes of the collection
 ////////////////////////////////////////////////////////////////////////////////

arangod/Aql/Collection.h

@@ -145,6 +145,12 @@ namespace triagens {
 
       std::vector<std::string> shardIds () const;
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief returns the shard keys of a collection
+////////////////////////////////////////////////////////////////////////////////
+
+      std::vector<std::string> shardKeys () const;
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief returns the indexes of the collection
 ////////////////////////////////////////////////////////////////////////////////

arangod/Aql/ExecutionNode.cpp

@@ -2448,7 +2448,6 @@ void DistributeNode::toJsonHelper (triagens::basics::Json& nodes,
       ("collection", triagens::basics::Json(_collection->getName()))
       ("varId", triagens::basics::Json(static_cast<int>(_varId)));
 
-
   // And add it:
   nodes(json);
 }

arangod/Aql/OptimizerRules.cpp

@@ -1689,7 +1689,6 @@ int triagens::aql::scatterInCluster (Optimizer* opt,
       // re-link with the remote node
       node->addDependency(remoteNode);
 
-
       // insert another remote node
       remoteNode = new RemoteNode(plan, plan->nextId(), vocbase, collection, "", "", "");
       plan->registerNode(remoteNode);
@@ -1742,21 +1741,30 @@ int triagens::aql::distributeInCluster (Optimizer* opt,
     auto const nodeType = node->getType();
     
     if (nodeType != ExecutionNode::INSERT  &&
-        nodeType != ExecutionNode::UPDATE  &&
-        nodeType != ExecutionNode::REPLACE &&
         nodeType != ExecutionNode::REMOVE) {
       opt->addPlan(plan, rule->level, wasModified);
       return TRI_ERROR_NO_ERROR;
     }
+    
+    Collection const* collection = static_cast<ModificationNode*>(node)->collection();
+    
+    if (nodeType == ExecutionNode::REMOVE) {
+      // check if collection shard keys are only _key
+      std::vector<std::string> shardKeys = collection->shardKeys();
+      if (shardKeys.size() != 1 || shardKeys[0] != "_key") {
+        opt->addPlan(plan, rule->level, wasModified);
+        return TRI_ERROR_NO_ERROR;
+      }
+    }
+
     auto deps = node->getDependencies();
     TRI_ASSERT(deps.size() == 1);
 
     // unlink the node
     plan->unlinkNode(node, true);
 
-    // extract database and collection from plan node
+    // extract database from plan node
     TRI_vocbase_t* vocbase = static_cast<ModificationNode*>(node)->vocbase();
-    Collection const* collection = static_cast<ModificationNode*>(node)->collection();
 
     // insert a distribute node
     TRI_ASSERT(node->getVariablesUsedHere().size() == 1);
@@ -1782,6 +1790,7 @@ int triagens::aql::distributeInCluster (Optimizer* opt,
   opt->addPlan(plan, rule->level, wasModified);
   return TRI_ERROR_NO_ERROR;
 }
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief move filters up into the cluster distribution part of the plan
 /// this rule modifies the plan in place

js/apps/system/aardvark/frontend/js/views/collectionsView.js

@@ -46,7 +46,13 @@
         $('#collectionsToggle').addClass('activated');
       }
 
-      var length = searchOptions.searchPhrase.length;
+      var length;
+
+      try {
+        length = searchOptions.searchPhrase.length;
+      }
+      catch (ignore) {
+      }
       $('#searchInput').val(searchOptions.searchPhrase);
       $('#searchInput').focus();
       $('#searchInput')[0].setSelectionRange(length, length);

js/server/tests/aql-optimizer-rule-distribute-in-cluster.js

@@ -80,7 +80,7 @@ function optimizerRuleTestSuite () {
     },
 
     ////////////////////////////////////////////////////////////////////////////////
-    /// @brief test that rule does not fire when it is not enabled 
+    /// @brief test that the rule fires when it is enabled
     ////////////////////////////////////////////////////////////////////////////////
 
     testThisRuleEnabled : function () {
@@ -89,10 +89,71 @@ function optimizerRuleTestSuite () {
         [ "FOR d IN " + cn1 + " REMOVE d._key in " + cn1, 1],
         [ "FOR d IN " + cn1 + " INSERT d in " + cn2, 2],
         [ "FOR d IN " + cn1 + " INSERT d._key in " + cn2, 3],
-        [ "FOR d IN " + cn1 + " REPLACE d in " + cn1, 4],
+      ];
-        [ "FOR d IN " + cn1 + " REPLACE d._key in " + cn1, 5],
+
-        [ "FOR d IN " + cn1 + " UPDATE d in " + cn1 , 6],
+      var expectedRules = [
-        [ "FOR d IN " + cn1 + " UPDATE d._key in " + cn1 , 7]
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                            ], 
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                              "distribute-filtercalc-to-cluster"
+                            ]
+                          ];
+
+      var expectedNodes = [ 
+                            [
+                              "SingletonNode",
+                              "ScatterNode", 
+                              "RemoteNode", 
+                              "EnumerateCollectionNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ],
+                            [
+                              "SingletonNode",
+                              "ScatterNode", 
+                              "RemoteNode", 
+                              "EnumerateCollectionNode", 
+                              "CalculationNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ]
+                          ];
+
+      var finalNodes = [ 
+                        "RemoveNode", "RemoveNode", 
+                        "InsertNode", "InsertNode"
+                       ];
+
+      queries.forEach(function(query) {
+        // can't turn this rule off so should always get the same answer
+        var i = query[1] % 2;
+        var result = AQL_EXPLAIN(query[0], { }, thisRuleEnabled);
+        assertEqual(expectedRules[i], result.plan.rules, query);
+        expectedNodes[i].push(finalNodes[query[1]]);
+        assertEqual(expectedNodes[i], explain(result), query);
+        expectedNodes[i].pop();
+        
+      });
+    },
+
+    ////////////////////////////////////////////////////////////////////////////////
+    /// @brief test that rule fires when it is disabled (i.e. it can't be disabled)
+    ////////////////////////////////////////////////////////////////////////////////
+
+    testThisRuleDisabled : function () {
+      var queries = [ 
+        [ "FOR d IN " + cn1 + " REMOVE d in " + cn1, 0],
+        [ "FOR d IN " + cn1 + " REMOVE d._key in " + cn1, 1],
+        [ "FOR d IN " + cn1 + " INSERT d in " + cn2, 2],
+        [ "FOR d IN " + cn1 + " INSERT d._key in " + cn2, 3],
       ];
 
       var expectedRules = [
@@ -131,12 +192,74 @@ function optimizerRuleTestSuite () {
 
       var finalNodes = [ 
                         "RemoveNode", "RemoveNode", 
-                        "InsertNode", "InsertNode",
+                        "InsertNode", "InsertNode"
-                        "ReplaceNode", "ReplaceNode", 
-                        "UpdateNode", "UpdateNode"
                        ];
 
       queries.forEach(function(query) {
+        // can't turn this rule off so should always get the same answer
+        var i = query[1] % 2;
+        var result = AQL_EXPLAIN(query[0], { }, rulesAll);
+        assertEqual(expectedRules[i], result.plan.rules, query);
+        expectedNodes[i].push(finalNodes[query[1]]);
+        result = AQL_EXPLAIN(query[0], { }, thisRuleDisabled);
+        assertEqual(expectedNodes[i], explain(result), query);
+        expectedNodes[i].pop();
+      });
+    },
+
+    ////////////////////////////////////////////////////////////////////////////////
+    /// @brief test that rule does not fire when it is not enabled 
+    ////////////////////////////////////////////////////////////////////////////////
+
+    testRulesAll : function () {
+      var queries = [ 
+        [ "FOR d IN " + cn1 + " REMOVE d in " + cn1, 0],
+        [ "FOR d IN " + cn1 + " REMOVE d._key in " + cn1, 1],
+        [ "FOR d IN " + cn1 + " INSERT d in " + cn2, 2],
+        [ "FOR d IN " + cn1 + " INSERT d._key in " + cn2, 3],
+      ];
+
+      var expectedRules = [
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                              "remove-unnecessary-remote-scatter" 
+                            ], 
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                              "distribute-filtercalc-to-cluster", 
+                              "remove-unnecessary-remote-scatter" 
+                            ]
+                          ];
+
+      var expectedNodes = [ 
+                            [
+                              "SingletonNode", 
+                              "EnumerateCollectionNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ],
+                            [
+                              "SingletonNode",
+                              "EnumerateCollectionNode", 
+                              "CalculationNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ]
+                          ];
+
+      var finalNodes = [ 
+                        "RemoveNode", "RemoveNode", 
+                        "InsertNode", "InsertNode"
+                       ];
+
+      queries.forEach(function(query) {
+        // can't turn this rule off so should always get the same answer
         var i = query[1] % 2;
         var result = AQL_EXPLAIN(query[0], { }, rulesAll);
         assertEqual(expectedRules[i], result.plan.rules, query);
@@ -146,6 +269,71 @@ function optimizerRuleTestSuite () {
       });
     },
 
+    ////////////////////////////////////////////////////////////////////////////////
+    /// @brief test that rule does not fire when it is not enabled 
+    ////////////////////////////////////////////////////////////////////////////////
+
+    testRulesNone : function () {
+      var queries = [ 
+        [ "FOR d IN " + cn1 + " REMOVE d in " + cn1, 0],
+        [ "FOR d IN " + cn1 + " REMOVE d._key in " + cn1, 1],
+        [ "FOR d IN " + cn1 + " INSERT d in " + cn2, 2],
+        [ "FOR d IN " + cn1 + " INSERT d._key in " + cn2, 3],
+      ];
+
+      var expectedRules = [
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                            ], 
+                            [ 
+                              "distribute-in-cluster", 
+                              "scatter-in-cluster", 
+                              "distribute-filtercalc-to-cluster", 
+                            ]
+                          ];
+
+      var expectedNodes = [ 
+                            [
+                              "SingletonNode",
+                              "ScatterNode", 
+                              "RemoteNode", 
+                              "EnumerateCollectionNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ],
+                            [
+                              "SingletonNode",
+                              "ScatterNode", 
+                              "RemoteNode", 
+                              "EnumerateCollectionNode", 
+                              "CalculationNode", 
+                              "RemoteNode", 
+                              "GatherNode",
+                              "DistributeNode",
+                              "RemoteNode"
+                            ]
+                          ];
+
+      var finalNodes = [ 
+                        "RemoveNode", "RemoveNode", 
+                        "InsertNode", "InsertNode"
+                       ];
+
+      queries.forEach(function(query) {
+        // can't turn this rule off so should always get the same answer
+        var i = query[1] % 2;
+        var result = AQL_EXPLAIN(query[0], { }, rulesNone);
+        assertEqual(expectedRules[i], result.plan.rules, query);
+        expectedNodes[i].push(finalNodes[query[1]]);
+        assertEqual(expectedNodes[i], explain(result), query);
+        expectedNodes[i].pop();
+        
+      });
+    },
+
     ////////////////////////////////////////////////////////////////////////////////
     /// @brief test that rule has no effect
     ////////////////////////////////////////////////////////////////////////////////
@@ -153,6 +341,10 @@ function optimizerRuleTestSuite () {
     testRuleNoEffect : function () {
        var queries = [ 
          "FOR d IN " + cn1 +  " RETURN d",
+         "FOR d IN " + cn1 + " REPLACE d in " + cn1, 
+         "FOR d IN " + cn1 + " REPLACE d._key in " + cn1,
+         "FOR d IN " + cn1 + " UPDATE d in " + cn1,
+         "FOR d IN " + cn1 + " UPDATE d._key in " + cn1 ,
          "FOR i IN 1..10 RETURN i" ];
 
       queries.forEach(function(query) {