diff --git a/arangod/Aql/Aggregator.cpp b/arangod/Aql/Aggregator.cpp index 116329600e..4d6589daf7 100644 --- a/arangod/Aql/Aggregator.cpp +++ b/arangod/Aql/Aggregator.cpp @@ -226,14 +226,16 @@ struct AggregatorMax final : public Aggregator { struct AggregatorSum final : public Aggregator { explicit AggregatorSum(transaction::Methods* trx) - : Aggregator(trx), sum(0.0), invalid(false) {} + : Aggregator(trx), sum(0.0), invalid(false), invoked(false) {} void reset() override { sum = 0.0; invalid = false; + invoked = false; } void reduce(AqlValue const& cmpValue) override { + invoked = true; if (!invalid) { if (cmpValue.isNull(true)) { // ignore `null` values here @@ -253,7 +255,7 @@ struct AggregatorSum final : public Aggregator { } AqlValue stealValue() override { - if (invalid || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { + if (invalid || !invoked || std::isnan(sum) || sum == HUGE_VAL || sum == -HUGE_VAL) { return AqlValue(AqlValueHintNull()); } @@ -266,6 +268,7 @@ struct AggregatorSum final : public Aggregator { double sum; bool invalid; + bool invoked; }; /// @brief the single-server variant of AVERAGE diff --git a/arangod/Aql/CollectBlock.cpp b/arangod/Aql/CollectBlock.cpp index d4634aa0b7..e50b5375b7 100644 --- a/arangod/Aql/CollectBlock.cpp +++ b/arangod/Aql/CollectBlock.cpp @@ -511,11 +511,8 @@ void SortedCollectBlock::emitGroup(AqlItemBlock const* cur, AqlItemBlock* res, ++r) { it->reduce(getValueForRegister(cur, r, reg)); } - res->setValue(row, _aggregateRegisters[j].first, it->stealValue()); - } else { - res->emplaceValue(row, _aggregateRegisters[j].first, - AqlValueHintNull()); } + res->setValue(row, _aggregateRegisters[j].first, it->stealValue()); ++j; } diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 56353be252..3977eefb2b 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2128,10 +2128,10 @@ struct SortToIndexNode final : public WalkerWorker { IndexIteratorOptions opts; opts.ascending = sortCondition.isAscending(); - std::unique_ptr newNode(new IndexNode( + auto newNode = std::make_unique( _plan, _plan->nextId(), enumerateCollectionNode->collection(), outVariable, usedIndexes, - std::move(condition), opts)); + std::move(condition), opts); auto n = newNode.release(); @@ -2142,6 +2142,7 @@ struct SortToIndexNode final : public WalkerWorker { if (coveredAttributes == sortCondition.numAttributes()) { // if the index covers the complete sort condition, we can also remove // the sort node + n->needsGatherNodeSort(true); _plan->unlinkNode(_plan->getNodeById(_sortNode->id())); } } diff --git a/js/common/modules/@arangodb/aql/explainer.js b/js/common/modules/@arangodb/aql/explainer.js index ef8677496b..2d822c41f3 100644 --- a/js/common/modules/@arangodb/aql/explainer.js +++ b/js/common/modules/@arangodb/aql/explainer.js @@ -332,7 +332,7 @@ function printIndexes (indexes) { var ranges; if (indexes[i].hasOwnProperty('condition')) { ranges = indexes[i].condition; - } else { + } else { ranges = '[ ' + indexes[i].ranges + ' ]'; } @@ -982,11 +982,11 @@ function processQuery (query, explain) { if (node.hasOwnProperty('condition') && node.condition.type && node.condition.type === 'n-ary or') { idx.condition = buildExpression(node.condition.subNodes[i]); } else { - if (variable !== false) { + if (variable !== false && variable !== undefined) { idx.condition = variable; } } - if (idx.condition === '') { + if (idx.condition === '' || idx.condition === undefined) { idx.condition = '*'; // empty condition. this is likely an index used for sorting or scanning only } indexes.push(idx); diff --git a/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-cluster.js b/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-cluster.js new file mode 100644 index 0000000000..2243dcc0a5 --- /dev/null +++ b/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-cluster.js @@ -0,0 +1,308 @@ +/*jshint globalstrict:false, strict:false, maxlen: 500 */ +/*global assertEqual, assertFalse, assertTrue, assertNotEqual, AQL_EXPLAIN, AQL_EXECUTE */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tests for optimizer rules +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2010-2012 triagens GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is triAGENS GmbH, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2012, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +var internal = require("internal"); +var db = internal.db; +var jsunity = require("jsunity"); +var helper = require("@arangodb/aql-helper"); +var isEqual = helper.isEqual; +var findExecutionNodes = helper.findExecutionNodes; +var findReferencedNodes = helper.findReferencedNodes; +var getQueryMultiplePlansAndExecutions = helper.getQueryMultiplePlansAndExecutions; +var removeAlwaysOnClusterRules = helper.removeAlwaysOnClusterRules; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test suite +//////////////////////////////////////////////////////////////////////////////// + +function optimizerRuleTestSuite() { + const ruleName = "use-index-for-sort"; + const colName = "UnitTestsUseIndexForSort"; + let c; + + return { + + setUp : function () { + internal.db._drop(colName); + c = internal.db._create(colName, {numberOfShards: 5}); + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + c.ensureIndex({ type: "skiplist", fields: [ "value" ] }); + }, + + //////////////////////////////////////////////////////////////////////////////// + /// @brief tear down + //////////////////////////////////////////////////////////////////////////////// + + tearDown : function () { + internal.db._drop(colName); + }, + + testSortAscKeepGather : function () { + let query = "FOR doc IN " + colName + " SORT doc.value ASC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertEqual("doc", nodes[0].elements[0].inVariable.name); + assertEqual(["value"], nodes[0].elements[0].path); + assertTrue(nodes[0].elements[0].ascending); + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value > last); + last = doc.value; + }); + }, + + testSortDescKeepGather : function () { + let query = "FOR doc IN " + colName + " SORT doc.value DESC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertEqual("doc", nodes[0].elements[0].inVariable.name); + assertEqual(["value"], nodes[0].elements[0].path); + assertFalse(nodes[0].elements[0].ascending); + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value < last); + last = doc.value; + }); + }, + + testSortAscKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " SORT doc.value ASC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertEqual("doc", nodes[0].elements[0].inVariable.name); + assertEqual(["value"], nodes[0].elements[0].path); + assertTrue(nodes[0].elements[0].ascending); + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(4000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value >= last); + last = doc.value; + }); + }, + + testCollectHashKeepGather : function () { + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'hash' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(2, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("hash", nodes[0].collectOptions.method); + assertEqual("SUM", nodes[1].aggregates[0].type); + assertFalse(nodes[1].count); + assertEqual("hash", nodes[1].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertEqual(0, nodes[0].elements.length); + + assertEqual(-1, plan.rules.indexOf(ruleName)); + assertNotEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(1, doc.l, doc); + }); + }, + + testCollectSortedKeepGather : function () { + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'sorted' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(2, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + assertEqual("SUM", nodes[1].aggregates[0].type); + assertFalse(nodes[1].count); + assertEqual("sorted", nodes[1].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertTrue(nodes[0].elements[0].ascending); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + assertNotEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value >= last); + last = doc.value; + assertEqual(1, doc.l, doc); + }); + }, + + testCollectIntoSortedKeepGather : function () { + let query = "FOR doc IN " + colName + " COLLECT value = doc.value INTO g OPTIONS { method: 'sorted' } RETURN { value, g }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertFalse(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertTrue(nodes[0].elements[0].ascending); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + assertEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value >= last); + last = doc.value; + assertEqual(1, doc.g.length); + assertEqual(doc.value, doc.g[0].doc.value); + }); + }, + + testCollectIntoSortedKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " COLLECT value = doc.value INTO g OPTIONS { method: 'sorted' } RETURN { value, g }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertFalse(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertTrue(nodes[0].elements[0].ascending); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + assertEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value >= last); + last = doc.value; + assertEqual(2, doc.g.length); + assertEqual(doc.value, doc.g[0].doc.value); + assertEqual(doc.value, doc.g[1].doc.value); + }); + }, + + testCollectHashKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'hash' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(2, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("hash", nodes[0].collectOptions.method); + assertEqual("SUM", nodes[1].aggregates[0].type); + assertFalse(nodes[1].count); + assertEqual("hash", nodes[1].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + + assertEqual(-1, plan.rules.indexOf(ruleName)); + assertNotEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(2, doc.l, doc); + }); + }, + + testCollectSortedKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'sorted' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(2, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + assertEqual("SUM", nodes[1].aggregates[0].type); + assertFalse(nodes[1].count); + assertEqual("sorted", nodes[1].collectOptions.method); + + nodes = plan.nodes.filter(function(n) { return n.type === 'GatherNode'; }); + assertEqual(1, nodes.length); + assertTrue(nodes[0].elements[0].ascending); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + assertNotEqual(-1, plan.rules.indexOf("collect-in-cluster")); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(2, doc.l, doc); + }); + } + + }; +} + +jsunity.run(optimizerRuleTestSuite); + +return jsunity.done(); diff --git a/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-noncluster.js b/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-noncluster.js new file mode 100644 index 0000000000..52b924a92f --- /dev/null +++ b/tests/js/server/aql/aql-optimizer-rule-use-index-for-sort-noncluster.js @@ -0,0 +1,200 @@ +/*jshint globalstrict:false, strict:false, maxlen: 500 */ +/*global assertEqual, assertFalse, assertTrue, assertNotEqual, AQL_EXPLAIN, AQL_EXECUTE */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tests for optimizer rules +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2010-2012 triagens GmbH, Cologne, Germany +/// +/// Licensed under the Apache License, Version 2.0 (the "License"); +/// you may not use this file except in compliance with the License. +/// You may obtain a copy of the License at +/// +/// http://www.apache.org/licenses/LICENSE-2.0 +/// +/// Unless required by applicable law or agreed to in writing, software +/// distributed under the License is distributed on an "AS IS" BASIS, +/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +/// See the License for the specific language governing permissions and +/// limitations under the License. +/// +/// Copyright holder is triAGENS GmbH, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2012, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +var internal = require("internal"); +var db = internal.db; +var jsunity = require("jsunity"); +var helper = require("@arangodb/aql-helper"); +var isEqual = helper.isEqual; +var findExecutionNodes = helper.findExecutionNodes; +var findReferencedNodes = helper.findReferencedNodes; +var getQueryMultiplePlansAndExecutions = helper.getQueryMultiplePlansAndExecutions; +var removeAlwaysOnClusterRules = helper.removeAlwaysOnClusterRules; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test suite +//////////////////////////////////////////////////////////////////////////////// + +function optimizerRuleTestSuite() { + const ruleName = "use-index-for-sort"; + const colName = "UnitTestsUseIndexForSort"; + let c; + + return { + + setUp : function () { + internal.db._drop(colName); + c = internal.db._create(colName, {numberOfShards: 5}); + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + c.ensureIndex({ type: "skiplist", fields: [ "value" ] }); + }, + + //////////////////////////////////////////////////////////////////////////////// + /// @brief tear down + //////////////////////////////////////////////////////////////////////////////// + + tearDown : function () { + internal.db._drop(colName); + }, + + testSortAscKeepGather : function () { + let query = "FOR doc IN " + colName + " SORT doc.value ASC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value > last); + last = doc.value; + }); + }, + + testSortDescKeepGather : function () { + let query = "FOR doc IN " + colName + " SORT doc.value DESC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value < last); + last = doc.value; + }); + }, + + testSortAscKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " SORT doc.value ASC RETURN doc"; + let plan = AQL_EXPLAIN(query).plan; + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(4000, results.length); + let last = null; + results.forEach(function(doc) { + assertTrue(last === null || doc.value >= last); + last = doc.value; + }); + }, + + testCollectHashKeepGather : function () { + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'hash' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("hash", nodes[0].collectOptions.method); + + assertEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(1, doc.l, doc); + }); + }, + + testCollectSortedKeepGather : function () { + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'sorted' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(1, doc.l, doc); + }); + }, + + testCollectHashKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'hash' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("hash", nodes[0].collectOptions.method); + + assertEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(2, doc.l, doc); + }); + }, + + testCollectSortedKeepGatherNonUnique : function () { + // add the same values again + for (let i = 0; i < 2000; ++i) { + c.insert({ value: i }); + } + let query = "FOR doc IN " + colName + " COLLECT value = doc.value WITH COUNT INTO l OPTIONS { method: 'sorted' } RETURN { value, l }"; + let plan = AQL_EXPLAIN(query).plan; + let nodes = plan.nodes.filter(function(n) { return n.type === 'CollectNode'; }); + assertEqual(1, nodes.length); + assertEqual([], nodes[0].aggregates); + assertTrue(nodes[0].count); + assertEqual("sorted", nodes[0].collectOptions.method); + + assertNotEqual(-1, plan.rules.indexOf(ruleName)); + + let results = AQL_EXECUTE(query).json; + assertEqual(2000, results.length); + results.forEach(function(doc) { + assertEqual(2, doc.l, doc); + }); + } + + }; +} + +jsunity.run(optimizerRuleTestSuite); + +return jsunity.done();