make replace-or-with-in rule fire in more cases

arangod/Aql/Ast.cpp

@@ -1702,6 +1702,11 @@ void Ast::validateAndOptimize() {
     if (node->type == NODE_TYPE_OPERATOR_TERNARY) {
       return this->optimizeTernaryOperator(node);
     }
+    
+    // attribute access
+    if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) {
+      return this->optimizeAttributeAccess(node);
+    }
 
     // passthru node
     if (node->type == NODE_TYPE_PASSTHRU) {
@@ -2670,6 +2675,42 @@ AstNode* Ast::optimizeTernaryOperator(AstNode* node) {
   return falsePart;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief optimizes an attribute access
+////////////////////////////////////////////////////////////////////////////////
+
+AstNode* Ast::optimizeAttributeAccess(AstNode* node) {
+  TRI_ASSERT(node != nullptr);
+  TRI_ASSERT(node->type == NODE_TYPE_ATTRIBUTE_ACCESS);
+  TRI_ASSERT(node->numMembers() == 1);
+
+  AstNode* what = node->getMember(0);
+
+  if (!what->isConstant()) {
+    return node;
+  }
+
+  if (what->type == NODE_TYPE_OBJECT) {
+    // accessing an attribute from an object
+    char const* name = node->getStringValue();
+    size_t const length = node->getStringLength();
+
+    size_t const n = what->numMembers();
+
+    for (size_t i = 0; i < n; ++i) {
+      AstNode const* member = what->getMember(0);
+      if (member->type == NODE_TYPE_OBJECT_ELEMENT &&
+          member->getStringLength() == length &&
+          memcmp(name, member->getStringValue(), length) == 0) {
+        // found matching member
+        return member->getMember(0); 
+      }
+    }
+  }
+
+  return node;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief optimizes a call to a built-in function
 ////////////////////////////////////////////////////////////////////////////////

arangod/Aql/Ast.h

@@ -748,6 +748,12 @@ class Ast {
 
   AstNode* optimizeTernaryOperator(AstNode*);
 
+  //////////////////////////////////////////////////////////////////////////////
+  /// @brief optimizes an attribute access
+  //////////////////////////////////////////////////////////////////////////////
+
+  AstNode* optimizeAttributeAccess(AstNode*);
+
   //////////////////////////////////////////////////////////////////////////////
   /// @brief optimizes a call to a built-in function
   //////////////////////////////////////////////////////////////////////////////

arangod/Aql/OptimizerRules.cpp

@@ -3110,81 +3110,155 @@ struct CommonNodeFinder {
 /// @brief auxilliary struct for the OR-to-IN conversion
 ////////////////////////////////////////////////////////////////////////////////
 
-struct OrToInConverter {
-  std::vector<AstNode const*> valueNodes;
-  CommonNodeFinder finder;
-  AstNode const* commonNode = nullptr;
-  std::string commonName;
+struct OrSimplifier {
+  Ast* ast;
 
-  AstNode* buildInExpression(Ast* ast) {
-    // the list of comparison values
-    auto list = ast->createNodeArray();
-    for (auto& x : valueNodes) {
-      list->addMember(x);
+  explicit OrSimplifier(Ast* ast) : ast(ast) {}
+  
+  std::string stringifyNode(AstNode const* node) const {
+    try {
+      return node->toString();
     }
-
-    // return a new IN operator node
-    return ast->createNodeBinaryOperator(NODE_TYPE_OPERATOR_BINARY_IN,
-                                         commonNode->clone(ast), list);
+    catch (...) {
+    }
+    return "";
   }
 
-  bool canConvertExpression(AstNode const* node) {
-    if (finder.find(node, NODE_TYPE_OPERATOR_BINARY_EQ, commonNode,
-                    commonName)) {
-      return canConvertExpressionWalker(node);
-    } else if (finder.find(node, NODE_TYPE_OPERATOR_BINARY_IN, commonNode,
-                           commonName)) {
-      return canConvertExpressionWalker(node);
+  bool qualifies(AstNode const* node, std::string& attributeName) const {
+    if (node->isConstant()) {
+      return false;
     }
+
+    if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS ||
+        node->type == NODE_TYPE_INDEXED_ACCESS ||
+        node->type == NODE_TYPE_REFERENCE) {
+      attributeName = stringifyNode(node);
+      return true;
+    }
+
     return false;
   }
 
-  bool canConvertExpressionWalker(AstNode const* node) {
-    if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) {
-      return (canConvertExpressionWalker(node->getMember(0)) &&
-              canConvertExpressionWalker(node->getMember(1)));
-    }
+  bool detect(AstNode const* node, bool preferRight, std::string& attributeName, AstNode const*& attr, AstNode const*& value) const {
+    attributeName.clear();
 
     if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) {
       auto lhs = node->getMember(0);
       auto rhs = node->getMember(1);
-
-      if (canConvertExpressionWalker(rhs) && !canConvertExpressionWalker(lhs)) {
-        valueNodes.emplace_back(lhs);
-        return true;
+      if (!preferRight && qualifies(lhs, attributeName)) {
+        if (rhs->isDeterministic() && !rhs->canThrow()) {
+          attr = lhs;
+          value = rhs;
+          return true;
+        }
       }
 
-      if (canConvertExpressionWalker(lhs) && !canConvertExpressionWalker(rhs)) {
-        valueNodes.emplace_back(rhs);
-        return true;
+      if (qualifies(rhs, attributeName)) {
+        if (lhs->isDeterministic() && !lhs->canThrow()) {
+          attr = rhs;
+          value = lhs;
+          return true;
+        }
       }
-      // if canConvertExpressionWalker(lhs) and canConvertExpressionWalker(rhs),
-      // then one of
-      // the equalities in the OR statement is of the form x == x
-      // fall-through intentional
-    } else if (node->type == NODE_TYPE_OPERATOR_BINARY_IN) {
+      // fallthrough intentional
+    }
+    else if (node->type == NODE_TYPE_OPERATOR_BINARY_IN) {
       auto lhs = node->getMember(0);
       auto rhs = node->getMember(1);
-
-      if (canConvertExpressionWalker(lhs) && !canConvertExpressionWalker(rhs) &&
-          rhs->isArray()) {
-        size_t const n = rhs->numMembers();
-
-        for (size_t i = 0; i < n; ++i) {
-          valueNodes.emplace_back(rhs->getMemberUnchecked(i));
+      if (rhs->isArray() && qualifies(lhs, attributeName)) {
+        if (rhs->isDeterministic() && !rhs->canThrow()) {
+          attr = lhs;
+          value = rhs;
+          return true;
         }
-        return true;
       }
-      // fall-through intentional
-    } else if (node->type == NODE_TYPE_REFERENCE ||
-               node->type == NODE_TYPE_ATTRIBUTE_ACCESS ||
-               node->type == NODE_TYPE_INDEXED_ACCESS) {
-      // get a string representation of the node for comparisons
-      return (node->toString() == commonName);
+      // fallthrough intentional
     }
 
     return false;
   }
+
+  AstNode* buildValues(AstNode const* attr, AstNode const* lhs, bool leftIsArray, AstNode const* rhs, bool rightIsArray) const {
+    auto values = ast->createNodeArray();
+    if (leftIsArray) {
+      size_t const n = lhs->numMembers();
+      for (size_t i = 0; i < n; ++i) {
+        values->addMember(lhs->getMemberUnchecked(i));
+      }
+    }
+    else {
+      values->addMember(lhs);
+    }
+    
+    if (rightIsArray) {
+      size_t const n = rhs->numMembers();
+      for (size_t i = 0; i < n; ++i) {
+        values->addMember(rhs->getMemberUnchecked(i));
+      }
+    }
+    else {
+      values->addMember(rhs);
+    }
+
+    return ast->createNodeBinaryOperator(NODE_TYPE_OPERATOR_BINARY_IN, attr, values);
+  }
+
+  AstNode* simplify(AstNode const* node) const {
+    if (node == nullptr) {
+      return nullptr;
+    }
+
+    if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) {
+      auto lhs = node->getMember(0);
+      auto rhs = node->getMember(1);
+      
+      auto lhsNew = simplify(lhs);
+      auto rhsNew = simplify(rhs);
+      
+      if (lhs != lhsNew || rhs != rhsNew) {
+        // create a modified node
+        node = ast->createNodeBinaryOperator(node->type, lhsNew, rhsNew);
+      }
+
+      if ((lhsNew->type == NODE_TYPE_OPERATOR_BINARY_EQ || lhsNew->type == NODE_TYPE_OPERATOR_BINARY_IN) && 
+          (rhsNew->type == NODE_TYPE_OPERATOR_BINARY_EQ || rhsNew->type == NODE_TYPE_OPERATOR_BINARY_IN)) {
+        std::string leftName;
+        std::string rightName;
+        AstNode const* leftAttr = nullptr;
+        AstNode const* rightAttr = nullptr;
+        AstNode const* leftValue = nullptr;
+        AstNode const* rightValue = nullptr;
+
+        for (size_t i = 0; i < 4; ++i) {
+          if (detect(lhsNew, i >= 2, leftName, leftAttr, leftValue) && 
+              detect(rhsNew, i % 2 == 0, rightName, rightAttr, rightValue) && 
+              leftName == rightName) {
+            return buildValues(leftAttr, leftValue, lhsNew->type == NODE_TYPE_OPERATOR_BINARY_IN, rightValue, rhsNew->type == NODE_TYPE_OPERATOR_BINARY_IN);
+          }
+        }
+      }
+
+      // return node as is
+      return const_cast<AstNode*>(node);
+    }
+    
+    if (node->type == NODE_TYPE_OPERATOR_BINARY_AND) {
+      auto lhs = node->getMember(0);
+      auto rhs = node->getMember(1);
+      
+      auto lhsNew = simplify(lhs);
+      auto rhsNew = simplify(rhs);
+
+      if (lhs != lhsNew || rhs != rhsNew) {
+        // return a modified node
+        return ast->createNodeBinaryOperator(node->type, lhsNew, rhsNew);
+      }
+
+      // fallthrough intentional
+    }
+      
+    return const_cast<AstNode*>(node);
+  }
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -3219,17 +3293,16 @@ void arangodb::aql::replaceOrWithInRule(Optimizer* opt, ExecutionPlan* plan,
     if (outVar.size() != 1 || outVar[0]->id != inVar[0]->id) {
       continue;
     }
-    if (cn->expression()->node()->type != NODE_TYPE_OPERATOR_BINARY_OR) {
-      continue;
-    }
+    
+    auto root = cn->expression()->node();
 
-    OrToInConverter converter;
-    if (converter.canConvertExpression(cn->expression()->node())) {
+    OrSimplifier simplifier(plan->getAst());
+    auto newRoot = simplifier.simplify(root);
+
+    if (newRoot != root) {
       ExecutionNode* newNode = nullptr;
-      auto inNode = converter.buildInExpression(plan->getAst());
-
-      Expression* expr = new Expression(plan->getAst(), inNode);
-
+      Expression* expr = new Expression(plan->getAst(), newRoot);
+      
       try {
         TRI_IF_FAILURE("OptimizerRules::replaceOrWithInRuleOom") {
           THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);

js/server/tests/aql/aql-optimizer-condition.js

@@ -184,17 +184,13 @@ function optimizerConditionsTestSuite () {
       assertEqual("value", left.subNodes[0].subNodes[1].type);
       assertEqual(1, left.subNodes[0].subNodes[1].value);
 
-      assertEqual("logical or", left.subNodes[1].type);
+      assertEqual("compare in", left.subNodes[1].type);
       assertEqual(2, left.subNodes[1].subNodes.length);
-      assertEqual("compare ==", left.subNodes[1].subNodes[0].type);
-      assertEqual("attribute access", left.subNodes[1].subNodes[0].subNodes[0].type);
-      assertEqual("c", left.subNodes[1].subNodes[0].subNodes[0].name);
-      assertEqual("value", left.subNodes[1].subNodes[0].subNodes[1].type);
-      assertEqual("a", left.subNodes[1].subNodes[0].subNodes[1].value);
-
-      assertEqual("compare ==", left.subNodes[1].subNodes[1].type);
-      assertEqual("attribute access", left.subNodes[1].subNodes[1].subNodes[0].type);
-      assertEqual("c", left.subNodes[1].subNodes[1].subNodes[0].name);
+      assertEqual("attribute access", left.subNodes[1].subNodes[0].type);
+      assertEqual("c", left.subNodes[1].subNodes[0].name);
+      assertEqual("array", left.subNodes[1].subNodes[1].type);
+      assertEqual("value", left.subNodes[1].subNodes[1].subNodes[0].type);
+      assertEqual("a", left.subNodes[1].subNodes[1].subNodes[0].value);
       assertEqual("value", left.subNodes[1].subNodes[1].subNodes[1].type);
       assertEqual("b", left.subNodes[1].subNodes[1].subNodes[1].value);
 

js/server/tests/aql/aql-optimizer-indexes-in-or.js

@@ -31,7 +31,6 @@
 var jsunity = require("jsunity");
 var db = require("@arangodb").db;
 
-
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief test suite
 ////////////////////////////////////////////////////////////////////////////////
@@ -602,9 +601,9 @@ function optimizerIndexesInOrTestSuite () {
         [ "FOR i IN " + c.name() + " FILTER i.value1 == 30 && i.value2 == 30 SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 30, 30 ] ], false ],
         [ "FOR i IN " + c.name() + " FILTER i.value1 IN [ 30, 31, 32 ] && i.value2 IN [ 30, 31, 32, 130 ] SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 30, 30 ], [ 30, 130 ], [ 31, 31 ], [ 32, 32 ] ], false ],
         [ "FOR i IN " + c.name() + " FILTER i.value1 IN [ 32, 32, 32, 32, 31, 31, 31, 31, 30, 30, 30, 32, 32, 31 ] && i.value2 IN [ 30, 31, 32, 130, 30, 31, 32, 30, 'foobar' ] SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 30, 30 ], [ 30, 130 ], [ 31, 31 ], [ 32, 32 ] ], false ],
-        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && (i.value2 == 32 || i.value2 == 29 || i.value2 == 30) SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 29, 29 ], [ 30, 30 ], [ 32, 32 ] ], true ],
-        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && (i.value2 == 32 || i.value2 == 29 || i.value2 == 30 || i.value2 == 35) SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 29, 29 ], [ 30, 30 ], [ 32, 32 ] ], true ],
-        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && i.value2 IN [ 32, 30, 45, 99, 12, 7 ] SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 30, 30 ], [ 32, 32 ] ], true ]
+        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && (i.value2 == 32 || i.value2 == 29 || i.value2 == 30) SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 29, 29 ], [ 30, 30 ], [ 32, 32 ] ], false ],
+        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && (i.value2 == 32 || i.value2 == 29 || i.value2 == 30 || i.value2 == 35) SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 29, 29 ], [ 30, 30 ], [ 32, 32 ] ], false ],
+        [ "FOR i IN " + c.name() + " FILTER (i.value1 == 30 || i.value1 == 29 || i.value1 == 32) && i.value2 IN [ 32, 30, 45, 99, 12, 7 ] SORT i.value1, i.value2 RETURN [ i.value1, i.value2 ]", [ [ 30, 30 ], [ 32, 32 ] ], false ]
       ];
 
       queries.forEach(function(query) {
@@ -615,14 +614,8 @@ function optimizerIndexesInOrTestSuite () {
 
         // ensure an index is used
         assertNotEqual(-1, nodeTypes.indexOf("IndexNode"), query);
-        if (query[2]) {
-          // we still need a sort...
-          assertNotEqual(-1, nodeTypes.indexOf("SortNode"), query);
-        }
-        else {
-          // we don't need a sort...
-          assertEqual(-1, nodeTypes.indexOf("SortNode"), query);
-        }
+        // we don't need a sort...
+        assertEqual(-1, nodeTypes.indexOf("SortNode"), query);
 
         var results = AQL_EXECUTE(query[0]);
         assertEqual(query[1].length, results.json.length, query); 

js/server/tests/aql/aql-optimizer-indexes-multi.js

@@ -464,21 +464,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -534,21 +519,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -604,21 +574,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -676,21 +631,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -746,21 +686,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -816,21 +741,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -886,21 +796,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -956,21 +851,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -1028,21 +908,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);
@@ -1098,21 +963,6 @@ function optimizerIndexesMultiTestSuite () {
                        "no index used for: " + query);
         assertEqual("ReturnNode", nodeTypes[nodeTypes.length - 1], query);
 
-        // This is somewhat fragile, we test whether the 3rd node is
-        // a calculation node and the 4th is a filter refering to it.
-        // Furthermore, we check the type of expression in the CalcNode
-        // and the number of subnodes:
-        if (filtercheck !== null) {
-          assertEqual("CalculationNode", plan.nodes[2].type, query);
-          assertEqual("FilterNode", plan.nodes[3].type, query);
-          assertEqual(plan.nodes[2].outVariable, plan.nodes[3].inVariable,
-                      query);
-          assertEqual(filtercheck.type, plan.nodes[2].expression.type, query);
-          assertEqual(filtercheck.nrSubs,
-                      plan.nodes[2].expression.subNodes.length,
-                      "Number of subnodes in filter expression, " + query);
-        }
-
         var results = AQL_EXECUTE(query);
         var correct = makeResult(maker).map(function(x) { return x.a; });
         assertEqual(correct, results.json, query);

js/server/tests/aql/aql-optimizer-rule-replace-or-with-in.js

@@ -429,13 +429,20 @@ function NewAqlReplaceORWithINTestSuite () {
 
       ruleIsNotUsed(query, {});
     },
+    
+    testDudCommonConstant3: function () {
+      var query = "LET x = NOOPT({a:1}) FOR v IN " + replace.name() 
+        + " FILTER x.a == v.value || x.a == v._key RETURN v._key";
+
+      isRuleUsed(query, {});
+    },
 
     testDudAlwaysTrue: function () {
       var query = 
       "FOR x IN " + replace.name() 
       + " FILTER x.value == x.value || x.value == 2 || x.value == 3 SORT x.value RETURN x.value";
      
-      ruleIsNotUsed(query, {});
+      isRuleUsed(query, {});
       assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {}));
 
     },
@@ -472,7 +479,7 @@ function NewAqlReplaceORWithINTestSuite () {
       var query = 
         "FOR x IN " + replace.name() + " FILTER x.val1 == 1 || 2 == x.val1 || 3 == x.val1 || 4 == x.val2 RETURN x";
 
-      ruleIsNotUsed(query, {});
+      isRuleUsed(query, {});
     },
 
     testDudDifferentAttributesWithBool1: function () {