improve handling of FILTERs with constant expressions (#9942)

arangod/Aql/ExecutionPlan.cpp

@@ -1173,9 +1173,26 @@ ExecutionNode* ExecutionPlan::fromNodeFilter(ExecutionNode* previous, AstNode co
     en = registerNode(new FilterNode(this, nextId(), v));
   } else {
     // operand is some misc expression
-    auto calc = createTemporaryCalculation(expression, previous);
+    if (expression->isTrue()) {
-    en = registerNode(new FilterNode(this, nextId(), getOutVariable(calc)));
+      // filter expression is known to be always true, so 
-    previous = calc;
+      // remove the filter entirely
+      return previous;
+    }
+
+    // note: if isTrue() is false above, it is not necessarily the case that
+    // isFalse() is true next. The reason is that isTrue() and isFalse() only
+    // return true in case of absolulete certainty. this requires expressions
+    // to be based on query compile-time values only, but it will not work
+    // for expressions that need to be evaluated at query runtime
+    if (expression->isFalse()) {
+      // filter expression is known to be always false, so
+      // replace the FILTER with a NoResultsNode
+      en = registerNode(new NoResultsNode(this, nextId()));
+    } else {
+      auto calc = createTemporaryCalculation(expression, previous);
+      en = registerNode(new FilterNode(this, nextId(), getOutVariable(calc)));
+      previous = calc;
+    }
   }
 
   return addDependency(previous, en);

tests/js/server/aql/aql-optimizer-rule-remove-unnecessary-filters.js

@@ -1,5 +1,5 @@
 /*jshint globalstrict:false, strict:false, maxlen: 500 */
-/*global assertEqual, assertTrue, AQL_EXPLAIN, AQL_EXECUTE */
+/*global assertEqual, assertTrue, assertFalse, AQL_EXPLAIN, AQL_EXECUTE */
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief tests for optimizer rules
@@ -101,21 +101,30 @@ function optimizerRuleTestSuite () {
 
     testRuleHasEffect : function () {
       var queries = [ 
-        "FOR i IN 1..10 FILTER true RETURN i",
+        [ "FOR i IN 1..2 FILTER true RETURN i", true ],
-        "FOR i IN 1..10 FILTER 1 < 9 RETURN i",
+        [ "FOR i IN 1..2 FILTER 1 > 9 RETURN i", false ],
-        "FOR i IN 1..10 LET a = 1 FILTER a == 1 RETURN i",
+        [ "FOR i IN 1..2 FILTER 1 < 9 RETURN i", true ],
-        "FOR i IN 1..10 LET a = 1 LET b = 1 FILTER a == b RETURN i",
+        [ "FOR i IN 1..2 LET a = 1 FILTER a == 1 RETURN i", true ],
-        "FOR i IN 1..10 LET a = 1 LET b = 2 FILTER a != b RETURN i",
+        [ "FOR i IN 1..2 LET a = 1 LET b = 1 FILTER a == b RETURN i", true ],
-        "FOR i IN 1..10 FILTER false RETURN i",
+        [ "FOR i IN 1..2 LET a = 1 LET b = 1 FILTER a != b RETURN i", false ],
-        "FOR i IN 1..10 LET a = 1 FILTER a == 9 RETURN i",
+        [ "FOR i IN 1..2 LET a = 1 LET b = 2 FILTER a != b RETURN i", true ],
-        "FOR i IN 1..10 LET a = 1 FILTER a != 1 RETURN i",
+        [ "FOR i IN 1..2 LET a = 1 LET b = 2 FILTER a == b RETURN i", false ],
-        "FOR i IN 1..10 FILTER 1 == 1 && 2 == 2 RETURN 1",
+        [ "FOR i IN 1..2 FILTER false RETURN i", false ],
-        "FOR i IN 1..10 FILTER 1 != 1 && 2 != 2 RETURN 1"
+        [ "FOR i IN 1..2 LET a = 1 FILTER a == 9 RETURN i", false ],
+        [ "FOR i IN 1..2 LET a = 1 FILTER a != 1 RETURN i", false ],
+        [ "FOR i IN 1..2 FILTER 1 == 1 && 2 == 2 RETURN i", true ],
+        [ "FOR i IN 1..2 FILTER 1 != 1 && 2 != 2 RETURN i", false ],
       ];
 
       queries.forEach(function(query) {
-        var result = AQL_EXPLAIN(query, { }, paramEnabled);
+        var result = AQL_EXPLAIN(query[0], { }, paramEnabled);
-        assertEqual([ ruleName ], result.plan.rules);
+        assertEqual([ ], result.plan.rules, query);
+        result = AQL_EXECUTE(query[0], { }, paramEnabled).json;
+        if (query[1]) {
+          assertEqual([ 1, 2 ], result, query);
+        } else {
+          assertEqual([ ], result, query);
+        }
       });
     },
 
@@ -138,7 +147,8 @@ function optimizerRuleTestSuite () {
 
       plans.forEach(function(plan) {
         var result = AQL_EXPLAIN(plan[0], { }, paramMore);
-        assertTrue(result.plan.rules.indexOf(ruleName) !== -1, plan[0]);
+        // rule will not fire anymore for constant filters
+        assertFalse(result.plan.rules.indexOf(ruleName) !== -1, plan[0]);
         assertEqual(plan[1], helper.getCompactPlan(result).map(function(node) { return node.type; }), plan[0]);
       });
     },
@@ -169,7 +179,8 @@ function optimizerRuleTestSuite () {
         assertTrue(isEqual(resultDisabled, resultEnabled), query[0]);
 
         assertTrue(planDisabled.plan.rules.indexOf(ruleName) === -1, query[0]);
-        assertTrue(planEnabled.plan.rules.indexOf(ruleName) !== -1, query[0]);
+        // rule will not fire anymore for constant filters
+        assertTrue(planEnabled.plan.rules.indexOf(ruleName) === -1, query[0]);
 
         assertEqual(resultDisabled, query[1]);
         assertEqual(resultEnabled, query[1]);

tests/js/server/aql/aql-profiler.js

@@ -295,9 +295,7 @@ function ahuacatlProfilerTestSuite () {
       const genNodeList = (rows, batches) => [
         {type: SingletonBlock, calls: 1, items: 1},
         {type: CalculationBlock, calls: 1, items: 1},
-        {type: CalculationBlock, calls: 1, items: 1},
         {type: EnumerateListBlock, calls: batches, items: rows},
-        {type: FilterBlock, calls: batches, items: rows},
         {type: ReturnBlock, calls: batches, items: rows},
       ];
       profHelper.runDefaultChecks({query, genNodeList, options});
@@ -497,8 +495,8 @@ function ahuacatlProfilerTestSuite () {
       const genNodeList = () => [
         {type: SingletonBlock, calls: 0, items: 0},
         {type: CalculationBlock, calls: 0, items: 0},
+        {type: EnumerateListBlock, calls: 0, items: 0},
         {type: NoResultsBlock, calls: 1, items: 0},
-        {type: EnumerateListBlock, calls: 1, items: 0},
         {type: ReturnBlock, calls: 1, items: 0},
       ];