fixed issue #1942

arangod/Aql/IndexNode.h

@@ -79,6 +79,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&,
                           bool) const override final;

arangod/Aql/OptimizerRules.cpp

@@ -1881,6 +1881,7 @@ struct SortToIndexNode final : public WalkerWorker<ExecutionNode> {
               (isSorted || fields.size() == sortCondition.numAttributes())) {
             // no need to sort
             _plan->unlinkNode(_plan->getNodeById(_sortNode->id()));
+            indexNode->reverse(sortCondition.isDescending());
             _modified = true;
           }
         }

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());
 

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 
+        return { endpoint : options.server,
-               , url : options.server.replace("tcp","http")
+                 url : options.server.replace("tcp", "http"),
-               , arangods : []
+                 arangods : []
                };
     }
     else if (options.cluster) {

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,
+                countXPect, "Has " + countXPect +  " CalculationNode");
-                "Has " + countXPect +  " CalculationNode");
   };
   var hasIndexNode = function (plan) {
     var rn = findExecutionNodes(plan, "IndexNode");
@@ -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,7 +610,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]) + "'"
@@ -626,14 +618,11 @@ function optimizerRuleTestSuite() {
       }
     },
     
-
-
-    
     ////////////////////////////////////////////////////////////////////////////////
     /// @brief test in detail that an index range can be used for an equality filter.
     ////////////////////////////////////////////////////////////////////////////////
-    testRangeEquals: function () {
     
+    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();
-