diff --git a/arangod/Aql/IndexNode.h b/arangod/Aql/IndexNode.h index d08f4e3d00..b0c118de98 100644 --- a/arangod/Aql/IndexNode.h +++ b/arangod/Aql/IndexNode.h @@ -78,6 +78,9 @@ class IndexNode : public ExecutionNode { /// @brief whether or not all indexes are accessed in reverse order bool reverse() const { return _reverse; } + + /// @brief set reverse mode + void reverse(bool value) { _reverse = value; } /// @brief export to VelocyPack void toVelocyPackHelper(arangodb::velocypack::Builder&, diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 22a75fbdcd..543d94920d 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1881,6 +1881,7 @@ struct SortToIndexNode final : public WalkerWorker { (isSorted || fields.size() == sortCondition.numAttributes())) { // no need to sort _plan->unlinkNode(_plan->getNodeById(_sortNode->id())); + indexNode->reverse(sortCondition.isDescending()); _modified = true; } } diff --git a/arangod/Utils/Transaction.cpp b/arangod/Utils/Transaction.cpp index d9f5db7b9b..f9aac39d3c 100644 --- a/arangod/Utils/Transaction.cpp +++ b/arangod/Utils/Transaction.cpp @@ -766,7 +766,6 @@ VPackSlice Transaction::extractKeyFromDocument(VPackSlice slice) { // however this method may also be called for remove markers, which only // have _key and _rev. therefore the only assertion that we can make // here is that the document at least has two attributes - TRI_ASSERT(slice.length() >= 2); uint8_t const* p = slice.begin() + slice.findDataOffset(slice.head()); diff --git a/js/client/modules/@arangodb/testing.js b/js/client/modules/@arangodb/testing.js index cf0543f0f2..92ab39d96d 100644 --- a/js/client/modules/@arangodb/testing.js +++ b/js/client/modules/@arangodb/testing.js @@ -1475,9 +1475,9 @@ function startInstance (protocol, options, addArgs, testname, tmpDir) { const startTime = time(); try { if(options.hasOwnProperty("server")){ - return { endpoint : options.server - , url : options.server.replace("tcp","http") - , arangods : [] + return { endpoint : options.server, + url : options.server.replace("tcp", "http"), + arangods : [] }; } else if (options.cluster) { diff --git a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js index c0fb498575..c8c01b7702 100644 --- a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js +++ b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js @@ -1,5 +1,5 @@ /*jshint globalstrict:false, strict:false, maxlen: 500 */ -/*global assertEqual, assertTrue, assertNotEqual, AQL_EXPLAIN, AQL_EXECUTE */ +/*global assertEqual, assertFalse, assertTrue, assertNotEqual, AQL_EXPLAIN, AQL_EXECUTE */ //////////////////////////////////////////////////////////////////////////////// /// @brief tests for optimizer rules @@ -75,18 +75,17 @@ function optimizerRuleTestSuite() { assertEqual(findExecutionNodes(plan, "SortNode").length, 1, "Has SortNode"); }; var hasNoSortNode = function (plan) { - assertEqual(findExecutionNodes(plan, "SortNode").length, 0, "Has NO SortNode"); + assertEqual(findExecutionNodes(plan, "SortNode").length, 0, "Has no SortNode"); }; var hasNoIndexNode = function (plan) { - assertEqual(findExecutionNodes(plan, "IndexNode").length, 0, "Has NO IndexNode"); + assertEqual(findExecutionNodes(plan, "IndexNode").length, 0, "Has no IndexNode"); }; var hasNoResultsNode = function (plan) { assertEqual(findExecutionNodes(plan, "NoResultsNode").length, 1, "Has NoResultsNode"); }; var hasCalculationNodes = function (plan, countXPect) { assertEqual(findExecutionNodes(plan, "CalculationNode").length, - countXPect, - "Has " + countXPect + " CalculationNode"); + countXPect, "Has " + countXPect + " CalculationNode"); }; var hasIndexNode = function (plan) { var rn = findExecutionNodes(plan, "IndexNode"); @@ -119,8 +118,8 @@ function optimizerRuleTestSuite() { for (i = 1; i <= loopto; ++i) { skiplist.save({ "a" : i, "b": j , "c": j, "d": i, "e": i, "joinme" : "aoeu " + j}); } - skiplist.save( { "a" : i, "c": j, "d": i, "e": i, "joinme" : "aoeu " + j}); - skiplist.save( { "c": j, "joinme" : "aoeu " + j}); + skiplist.save( { "a" : i, "c": j, "d": i, "e": i, "joinme" : "aoeu " + j}); + skiplist.save( { "c": j, "joinme" : "aoeu " + j}); } skiplist.ensureSkiplist("a", "b"); @@ -133,8 +132,8 @@ function optimizerRuleTestSuite() { for (i = 1; i <= loopto; ++i) { skiplist2.save({ "f" : i, "g": j , "h": j, "i": i, "j": i, "joinme" : "aoeu " + j}); } - skiplist2.save( { "f" : i, "g": j, "i": i, "j": i, "joinme" : "aoeu " + j}); - skiplist2.save( { "h": j, "joinme" : "aoeu " + j}); + skiplist2.save( { "f" : i, "g": j, "i": i, "j": i, "joinme" : "aoeu " + j}); + skiplist2.save( { "h": j, "joinme" : "aoeu " + j}); } skiplist2.ensureSkiplist("f", "g"); skiplist2.ensureSkiplist("i"); @@ -229,7 +228,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query[0] + + "while execution of '" + query[0] + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -237,10 +236,8 @@ function optimizerRuleTestSuite() { } } }); - }, - //////////////////////////////////////////////////////////////////////////////// /// @brief test that rule has an effect //////////////////////////////////////////////////////////////////////////////// @@ -273,7 +270,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -311,7 +308,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -319,12 +316,8 @@ function optimizerRuleTestSuite() { } i++; }); - }, - - - //////////////////////////////////////////////////////////////////////////////// /// @brief this sort is replaceable by an index. //////////////////////////////////////////////////////////////////////////////// @@ -372,7 +365,7 @@ function optimizerRuleTestSuite() { // This plan didn't sort by the index, so we need to re-sort the result by v.a and v.b assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -382,7 +375,7 @@ function optimizerRuleTestSuite() { else { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -456,7 +449,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -536,7 +529,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -550,7 +543,6 @@ function optimizerRuleTestSuite() { //////////////////////////////////////////////////////////////////////////////// testRangeSupersedesSort2: function () { - var query = "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a, v.b RETURN [v.a, v.b, v.c]"; var XPresult; var QResults=[]; @@ -618,22 +610,19 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" ); } }, - - - //////////////////////////////////////////////////////////////////////////////// /// @brief test in detail that an index range can be used for an equality filter. //////////////////////////////////////////////////////////////////////////////// + testRangeEquals: function () { - var query = "FOR v IN " + colName + " FILTER v.a == 1 RETURN [v.a, v.b]"; var XPresult; @@ -660,7 +649,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -668,10 +657,10 @@ function optimizerRuleTestSuite() { } }, + //////////////////////////////////////////////////////////////////////////////// + /// @brief test in detail that an index range can be used for a less than filter + //////////////////////////////////////////////////////////////////////////////// - //////////////////////////////////////////////////////////////////////////////// - /// @brief test in detail that an index range can be used for a less than filter. - //////////////////////////////////////////////////////////////////////////////// testRangeLessThan: function () { var query = "FOR v IN " + colName + " FILTER v.a < 5 RETURN [v.a, v.b]"; @@ -697,7 +686,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -733,7 +722,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -745,6 +734,7 @@ function optimizerRuleTestSuite() { /// @brief test in detail that an index range can be used for an and combined /// greater than + less than filter spanning a range. //////////////////////////////////////////////////////////////////////////////// + testRangeBandpass: function () { var query = "FOR v IN " + colName + " FILTER v.a > 4 && v.a < 10 RETURN [v.a, v.b]"; var XPresult; @@ -769,7 +759,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0].json.sort(sortArray), allresults.results[j].json.sort(sortArray)), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -811,7 +801,7 @@ function optimizerRuleTestSuite() { for (j = 1; j < allresults.results.length; j++) { assertTrue(isEqual(allresults.results[0], allresults.results[j]), - "whether the execution of '" + query + + "while execution of '" + query + "' this plan gave the wrong results: " + JSON.stringify(allresults.plans[j]) + " Should be: '" + JSON.stringify(allresults.results[0]) + "' but Is: " + JSON.stringify(allresults.results[j]) + "'" @@ -819,7 +809,6 @@ function optimizerRuleTestSuite() { } }, - //////////////////////////////////////////////////////////////////////////////// /// @brief test in detail that an index range can be used for an or combined /// greater than + less than filter spanning a range. @@ -896,6 +885,16 @@ function optimizerRuleTestSuite() { var rules = AQL_EXPLAIN("FOR v IN " + colName + " SORT v.d ASC RETURN v").plan.rules; assertNotEqual(-1, rules.indexOf(ruleName)); + + var nodes = AQL_EXPLAIN("FOR v IN " + colName + " SORT v.d ASC RETURN v").plan.nodes; + var seen = false; + nodes.forEach(function(node) { + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); + } + }); + assertTrue(seen); }, testSortDescEmptyCollection : function () { @@ -908,8 +907,169 @@ function optimizerRuleTestSuite() { var rules = AQL_EXPLAIN("FOR v IN " + colName + " SORT v.d DESC RETURN v").plan.rules; assertNotEqual(-1, rules.indexOf(ruleName)); - } + + var nodes = AQL_EXPLAIN("FOR v IN " + colName + " SORT v.d DESC RETURN v").plan.nodes; + var seen = false; + nodes.forEach(function(node) { + if (node.type === "IndexNode") { + seen = true; + assertTrue(node.reverse); + } + }); + assertTrue(seen); + }, + testSortAscWithFilter : function () { + var query = "FOR v IN " + colName + " FILTER v.d == 123 SORT v.d ASC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); + } + }); + assertTrue(seen); + }, + + testSortAscWithFilterMulti : function () { + var query = "FOR v IN " + colName + " FILTER v.a == 123 SORT v.b ASC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); + } + }); + assertTrue(seen); + }, + + testSortAscWithFilterNonConst : function () { + var query = "FOR v IN " + colName + " FILTER v.d == NOOPT(123) SORT v.d ASC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); + } + }); + assertTrue(seen); + }, + + testSortAscWithFilterNonConstMulti : function () { + var query = "FOR v IN " + colName + " FILTER v.a == NOOPT(123) SORT v.b ASC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); + } + }); + assertTrue(seen); + }, + + testSortDescWithFilter : function () { + var query = "FOR v IN " + colName + " FILTER v.d == 123 SORT v.d DESC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); // forward or backward does not matter here + } + }); + assertTrue(seen); + }, + + testSortDescWithFilterMulti : function () { + var query = "FOR v IN " + colName + " FILTER v.a == 123 SORT v.b DESC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertTrue(node.reverse); + } + }); + assertTrue(seen); + }, + + testSortDescWithFilterNonConst : function () { + var query = "FOR v IN " + colName + " FILTER v.d == NOOPT(123) SORT v.d DESC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertFalse(node.reverse); // forward or backward does not matter here + } + }); + assertTrue(seen); + }, + + testSortDescWithFilterNonConstMulti : function () { + var query = "FOR v IN " + colName + " FILTER v.a == NOOPT(123) SORT v.b DESC RETURN v"; + var rules = AQL_EXPLAIN(query).plan.rules; + assertNotEqual(-1, rules.indexOf(ruleName)); + assertNotEqual(-1, rules.indexOf(secondRuleName)); + assertNotEqual(-1, rules.indexOf("remove-filter-covered-by-index")); + + var nodes = AQL_EXPLAIN(query).plan.nodes; + var seen = false; + nodes.forEach(function(node) { + assertNotEqual("SortNode", node.type); + if (node.type === "IndexNode") { + seen = true; + assertTrue(node.reverse); + } + }); + assertTrue(seen); + } }; } @@ -920,4 +1080,3 @@ function optimizerRuleTestSuite() { jsunity.run(optimizerRuleTestSuite); return jsunity.done(); -