diff --git a/arangod/Aql/Collection.cpp b/arangod/Aql/Collection.cpp index 5aa9dc0328..77e8f3b410 100644 --- a/arangod/Aql/Collection.cpp +++ b/arangod/Aql/Collection.cpp @@ -111,6 +111,23 @@ std::vector Collection::shardIds () const { return ids; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief returns the shard keys of a collection +//////////////////////////////////////////////////////////////////////////////// + +std::vector 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 keys; + for (auto const& x : collectionInfo.get()->shardKeys()) { + keys.emplace_back(x); + } + return keys; +} //////////////////////////////////////////////////////////////////////////////// /// @brief returns the indexes of the collection //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/Collection.h b/arangod/Aql/Collection.h index bd932b0f00..de34f007a2 100644 --- a/arangod/Aql/Collection.h +++ b/arangod/Aql/Collection.h @@ -145,6 +145,12 @@ namespace triagens { std::vector shardIds () const; +//////////////////////////////////////////////////////////////////////////////// +/// @brief returns the shard keys of a collection +//////////////////////////////////////////////////////////////////////////////// + + std::vector shardKeys () const; + //////////////////////////////////////////////////////////////////////////////// /// @brief returns the indexes of the collection //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index 4cf6d139a3..9369129579 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/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(_varId))); - // And add it: nodes(json); } diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 93280898c2..51a871769b 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1688,7 +1688,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, "", "", ""); @@ -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(node)->collection(); + + if (nodeType == ExecutionNode::REMOVE) { + // check if collection shard keys are only _key + std::vector 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(node)->vocbase(); - Collection const* collection = static_cast(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 diff --git a/js/apps/system/aardvark/frontend/js/views/collectionsView.js b/js/apps/system/aardvark/frontend/js/views/collectionsView.js index 78c3584ddc..c5f576b87f 100644 --- a/js/apps/system/aardvark/frontend/js/views/collectionsView.js +++ b/js/apps/system/aardvark/frontend/js/views/collectionsView.js @@ -46,9 +46,15 @@ $('#collectionsToggle').addClass('activated'); } - var length = searchOptions.searchPhrase.length; + var length; + + try { + length = searchOptions.searchPhrase.length; + } + catch (ignore) { + } $('#searchInput').val(searchOptions.searchPhrase); - $('#searchInput').focus(); + $('#searchInput').focus(); $('#searchInput')[0].setSelectionRange(length, length); arangoHelper.fixTooltips(".icon_arangodb, .arangoicon", "left"); diff --git a/js/server/tests/aql-optimizer-rule-distribute-in-cluster.js b/js/server/tests/aql-optimizer-rule-distribute-in-cluster.js index 77267d3244..5aeb9db4ad 100644 --- a/js/server/tests/aql-optimizer-rule-distribute-in-cluster.js +++ b/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], - [ "FOR d IN " + cn1 + " UPDATE d._key in " + cn1 , 7] + ]; + + 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], { }, 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", - "ReplaceNode", "ReplaceNode", - "UpdateNode", "UpdateNode" + "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); + 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) {