From d12b5baf12da6c10921ee2be5a59e4acf42dd184 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 1 Nov 2014 14:30:18 +0000 Subject: [PATCH 01/26] first try at replace-OR-with-IN optimizer rule. --- arangod/Aql/Optimizer.cpp | 6 ++ arangod/Aql/Optimizer.h | 3 + arangod/Aql/OptimizerRules.cpp | 129 +++++++++++++++++++++++++++++++++ arangod/Aql/OptimizerRules.h | 11 +++ 4 files changed, 149 insertions(+) diff --git a/arangod/Aql/Optimizer.cpp b/arangod/Aql/Optimizer.cpp index 696f9a9e68..075155b848 100644 --- a/arangod/Aql/Optimizer.cpp +++ b/arangod/Aql/Optimizer.cpp @@ -463,6 +463,12 @@ void Optimizer::setupRules () { useIndexForSort_pass6, true); + // try to replace simple OR conditions with IN + registerRule("replace-OR-with-IN", + replaceORwithIN, + replaceORwithIN_pass6, + true); + if (ExecutionEngine::isCoordinator()) { // distribute operations in cluster registerRule("scatter-in-cluster", diff --git a/arangod/Aql/Optimizer.h b/arangod/Aql/Optimizer.h index 0c070a9f4e..1090eed34b 100644 --- a/arangod/Aql/Optimizer.h +++ b/arangod/Aql/Optimizer.h @@ -130,6 +130,9 @@ namespace triagens { // try to find sort blocks which are superseeded by indexes useIndexForSort_pass6 = 820, + + // replace simple OR conditions with IN + replaceORwithIN_pass6 = 830, ////////////////////////////////////////////////////////////////////////////// /// "Pass 10": final transformations for the cluster diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 69e81c4e21..1ec656747e 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2257,6 +2257,135 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, return TRI_ERROR_NO_ERROR; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief +//////////////////////////////////////////////////////////////////////////////// + +// recursively check the expression in the calculation node, to make sure it is +// an OR of equality comparisons over the same variable and attribute. + +bool buildExpression (AstNode const* node, + AstNode*& expr, + ExecutionPlan* plan) { + + if (node->type == NODE_TYPE_REFERENCE) { + if (expr->numMembers() == 0) { + return true; + } + auto thisVar = static_cast(node->getData()); + auto existingVar = + static_cast(expr->getMember(0)->getMember(0)->getData()); + if (thisVar->id == existingVar->id) { + return true; + } + return false; + } + + if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if(buildExpression(node->getMember(0), expr, plan)) { + if (expr->numMembers() == 0) { + expr->addMember(node->clone(plan->getAst())); + return true; + } + if (node->getStringValue() + == expr->getMember(0)->getStringValue()) { + return true; + } + } + return false; + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { + auto lhs = node->getMember(0); + auto rhs = node->getMember(1); + + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if(buildExpression(rhs, expr, plan)){ + TRI_ASSERT(expr->numMembers() != 0); + if (lhs->type == NODE_TYPE_VALUE) { + if (expr->numMembers() == 1) { + expr->addMember(new AstNode(NODE_TYPE_LIST)); + } + // keep the value in the lhs + expr->getMember(1)->addMember(lhs->clone(plan->getAst())); + return true; + } + } + return false; + } + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if(buildExpression(lhs, expr, plan)){ + TRI_ASSERT(expr->numMembers() != 0); + if (rhs->type == NODE_TYPE_VALUE) { + if (expr->numMembers() == 1) { + expr->addMember(new AstNode(NODE_TYPE_LIST)); + } + // keep the value in the rhs + expr->getMember(1)->addMember(rhs->clone(plan->getAst())); + return true; + } + } + return false; + } + return false; + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { + if (buildExpression(node->getMember(0), expr, plan)) { + if (buildExpression(node->getMember(1), expr, plan)) { + return true; + } + } + } + + return false; +} + +int triagens::aql::replaceORwithIN (Optimizer* opt, + ExecutionPlan* plan, + Optimizer::Rule const* rule) { + std::vector nodes + = plan->findNodesOfType(triagens::aql::ExecutionNode::FILTER, true); + + bool modified = false; + for (auto n : nodes) { + auto deps = n->getDependencies(); + TRI_ASSERT(deps.size() == 1); + TRI_ASSERT(deps[0]->getType() == triagens::aql::ExecutionNode::CALCULATION); + auto fn = static_cast(n); + auto cn = static_cast(deps[0]); + + auto inVar = fn->getVariablesUsedHere(); + auto outVar = cn->getVariablesSetHere(); + + if (outVar.size() != 1 || outVar[0]->id != inVar[0]->id) { + continue; + } + + AstNode* expr = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); + + if (buildExpression(cn->expression()->node(), expr, plan)) { + deps = cn->getDependencies(); + TRI_ASSERT(deps.size() == 1); + + plan->unlinkNode(cn); + + cn = new CalculationNode(plan, plan->nextId(), + new Expression(plan->getAst(), expr), outVar[0]->clone()); + plan->registerNode(cn); + cn->addDependency(deps[0]); + fn->addDependency(cn); + modified = false; + plan->findVarUsage(); //FIXME appropriate place for this? + } + } + + opt->addPlan(plan, rule->level, modified); + + return TRI_ERROR_NO_ERROR; +} + // Local Variables: // mode: outline-minor // outline-regexp: "^\\(/// @brief\\|/// {@inheritDoc}\\|/// @addtogroup\\|// --SECTION--\\|/// @\\}\\)" diff --git a/arangod/Aql/OptimizerRules.h b/arangod/Aql/OptimizerRules.h index 9119b6869e..83f2b68d69 100644 --- a/arangod/Aql/OptimizerRules.h +++ b/arangod/Aql/OptimizerRules.h @@ -164,6 +164,17 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// int undistributeRemoveAfterEnumColl (Optimizer*, ExecutionPlan*, Optimizer::Rule const*); + +//////////////////////////////////////////////////////////////////////////////// +/// @brief this rule replaces expressions of the type: +/// x.val == 1 || x.val == 2 || x.val == 3 +// with +// x.val IN [1,2,3] +// when the OR conditions are present in the same FILTER node, and refer to the +// same (single) attribute. +//////////////////////////////////////////////////////////////////////////////// + + int replaceORwithIN (Optimizer*, ExecutionPlan*, Optimizer::Rule const*); } // namespace aql } // namespace triagens From cd710ed55db042b4c553b73dc7619aebc79d1231 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 1 Nov 2014 16:06:53 +0000 Subject: [PATCH 02/26] snapshot --- arangod/Aql/OptimizerRules.cpp | 92 ++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 43 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 1ec656747e..c97df09c0c 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -35,6 +35,14 @@ using namespace triagens::aql; using Json = triagens::basics::Json; using EN = triagens::aql::ExecutionNode; +//#if 0 +#define ENTER_BLOCK try { (void) 0; +#define LEAVE_BLOCK } catch (...) { std::cout << "caught an exception in " << __FUNCTION__ << ", " << __FILE__ << ":" << __LINE__ << "!\n"; throw; } +//#else +//#define ENTER_BLOCK +//#define LEAVE_BLOCK +//#endif + // ----------------------------------------------------------------------------- // --SECTION-- rules for the optimizer // ----------------------------------------------------------------------------- @@ -48,7 +56,7 @@ using EN = triagens::aql::ExecutionNode; int triagens::aql::removeRedundantSorts (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { - std::vector nodes = plan->findNodesOfType(triagens::aql::ExecutionNode::SORT, true); + std::vector nodes = plan->findNodesOfType(EN::SORT, true); std::unordered_set toUnlink; triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); @@ -77,7 +85,7 @@ int triagens::aql::removeRedundantSorts (Optimizer* opt, auto current = stack.back(); stack.pop_back(); - if (current->getType() == triagens::aql::ExecutionNode::SORT) { + if (current->getType() == EN::SORT) { // we found another sort. now check if they are compatible! auto other = static_cast(current)->getSortInformation(plan, &buffer); @@ -126,17 +134,17 @@ int triagens::aql::removeRedundantSorts (Optimizer* opt, } } } - else if (current->getType() == triagens::aql::ExecutionNode::FILTER) { + else if (current->getType() == EN::FILTER) { // ok: a filter does not depend on sort order } - else if (current->getType() == triagens::aql::ExecutionNode::CALCULATION) { + else if (current->getType() == EN::CALCULATION) { // ok: a filter does not depend on sort order only if it does not throw if (current->canThrow()) { ++nodesRelyingOnSort; } } - else if (current->getType() == triagens::aql::ExecutionNode::ENUMERATE_LIST || - current->getType() == triagens::aql::ExecutionNode::ENUMERATE_COLLECTION) { + else if (current->getType() == EN::ENUMERATE_LIST || + current->getType() == EN::ENUMERATE_COLLECTION) { // ok, but we cannot remove two different sorts if one of these node types is between them // example: in the following query, the one sort will be optimized away: // FOR i IN [ { a: 1 }, { a: 2 } , { a: 3 } ] SORT i.a ASC SORT i.a DESC RETURN i @@ -188,7 +196,7 @@ int triagens::aql::removeUnnecessaryFiltersRule (Optimizer* opt, bool modified = false; std::unordered_set toUnlink; // should we enter subqueries?? - std::vector nodes = plan->findNodesOfType(triagens::aql::ExecutionNode::FILTER, true); + std::vector nodes = plan->findNodesOfType(EN::FILTER, true); for (auto n : nodes) { // filter nodes always have one input variable @@ -200,7 +208,7 @@ int triagens::aql::removeUnnecessaryFiltersRule (Optimizer* opt, auto setter = plan->getVarSetBy(variable->id); if (setter == nullptr || - setter->getType() != triagens::aql::ExecutionNode::CALCULATION) { + setter->getType() != EN::CALCULATION) { // filter variable was not introduced by a calculation. continue; } @@ -254,7 +262,7 @@ int triagens::aql::removeUnnecessaryFiltersRule (Optimizer* opt, int triagens::aql::moveCalculationsUpRule (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { - std::vector nodes = plan->findNodesOfType(triagens::aql::ExecutionNode::CALCULATION, true); + std::vector nodes = plan->findNodesOfType(EN::CALCULATION, true); bool modified = false; for (auto n : nodes) { @@ -333,7 +341,7 @@ int triagens::aql::moveCalculationsUpRule (Optimizer* opt, int triagens::aql::moveFiltersUpRule (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { - std::vector nodes = plan->findNodesOfType(triagens::aql::ExecutionNode::FILTER, true); + std::vector nodes = plan->findNodesOfType(EN::FILTER, true); bool modified = false; for (auto n : nodes) { @@ -349,7 +357,7 @@ int triagens::aql::moveFiltersUpRule (Optimizer* opt, auto current = stack.back(); stack.pop_back(); - if (current->getType() == triagens::aql::ExecutionNode::LIMIT) { + if (current->getType() == EN::LIMIT) { // cannot push a filter beyond a LIMIT node break; } @@ -500,7 +508,7 @@ int triagens::aql::removeRedundantCalculationsRule (Optimizer* opt, std::unordered_map replacements; std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::CALCULATION, true); + = plan->findNodesOfType(EN::CALCULATION, true); for (auto n : nodes) { auto nn = static_cast(n); @@ -535,7 +543,7 @@ int triagens::aql::removeRedundantCalculationsRule (Optimizer* opt, auto current = stack.back(); stack.pop_back(); - if (current->getType() == triagens::aql::ExecutionNode::CALCULATION) { + if (current->getType() == EN::CALCULATION) { try { static_cast(current)->expression()->stringify(&buffer); } @@ -584,7 +592,7 @@ int triagens::aql::removeRedundantCalculationsRule (Optimizer* opt, } } - if (current->getType() == triagens::aql::ExecutionNode::AGGREGATE) { + if (current->getType() == EN::AGGREGATE) { if (static_cast(current)->hasOutVariable()) { // COLLECT ... INTO is evil (tm): it needs to keep all already defined variables // we need to abort optimization here @@ -632,14 +640,14 @@ int triagens::aql::removeUnnecessaryCalculationsRule (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { std::vector const types = { - triagens::aql::ExecutionNode::CALCULATION, - triagens::aql::ExecutionNode::SUBQUERY + EN::CALCULATION, + EN::SUBQUERY }; std::vector nodes = plan->findNodesOfType(types, true); std::unordered_set toUnlink; for (auto n : nodes) { - if (n->getType() == triagens::aql::ExecutionNode::CALCULATION) { + if (n->getType() == EN::CALCULATION) { auto nn = static_cast(n); if (nn->canThrow() || @@ -983,7 +991,7 @@ class FilterToEnumCollFinder : public WalkerWorker { auto x = static_cast(node->getData()); auto setter = _plan->getVarSetBy(x->id); if (setter != nullptr && - setter->getType() == triagens::aql::ExecutionNode::ENUMERATE_COLLECTION) { + setter->getType() == EN::ENUMERATE_COLLECTION) { enumCollVar = x; } return; @@ -1132,7 +1140,7 @@ int triagens::aql::useIndexRange (Optimizer* opt, Optimizer::Rule const* rule) { std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::FILTER, true); + = plan->findNodesOfType(EN::FILTER, true); for (auto n : nodes) { auto nn = static_cast(n); @@ -1441,7 +1449,7 @@ int triagens::aql::useIndexForSort (Optimizer* opt, Optimizer::Rule const* rule) { bool planModified = false; std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::SORT, true); + = plan->findNodesOfType(EN::SORT, true); for (auto n : nodes) { auto thisSortNode = static_cast(n); SortAnalysis node(thisSortNode); @@ -1503,7 +1511,7 @@ int triagens::aql::interchangeAdjacentEnumerations (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::ENUMERATE_COLLECTION, + = plan->findNodesOfType(EN::ENUMERATE_COLLECTION, true); std::unordered_set nodesSet; for (auto n : nodes) { @@ -1532,7 +1540,7 @@ int triagens::aql::interchangeAdjacentEnumerations (Optimizer* opt, break; } if (deps[0]->getType() != - triagens::aql::ExecutionNode::ENUMERATE_COLLECTION) { + EN::ENUMERATE_COLLECTION) { break; } nwalker = deps[0]; @@ -1818,7 +1826,7 @@ int triagens::aql::distributeFilternCalcToCluster (Optimizer* opt, bool modified = false; std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::GATHER, true); + = plan->findNodesOfType(EN::GATHER, true); for (auto n : nodes) { @@ -1911,7 +1919,7 @@ int triagens::aql::distributeSortToCluster (Optimizer* opt, bool modified = false; std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::GATHER, true); + = plan->findNodesOfType(EN::GATHER, true); for (auto n : nodes) { auto remoteNodeList = n->getDependencies(); @@ -1993,7 +2001,7 @@ int triagens::aql::removeUnnecessaryRemoteScatter (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::REMOTE, true); + = plan->findNodesOfType(EN::REMOTE, true); std::unordered_set toUnlink; for (auto n : nodes) { @@ -2237,7 +2245,7 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, ExecutionPlan* plan, Optimizer::Rule const* rule) { std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::REMOVE, true); + = plan->findNodesOfType(EN::REMOVE, true); std::unordered_set toUnlink; for (auto n : nodes) { @@ -2287,8 +2295,8 @@ bool buildExpression (AstNode const* node, expr->addMember(node->clone(plan->getAst())); return true; } - if (node->getStringValue() - == expr->getMember(0)->getStringValue()) { + if (strcmp(node->getStringValue(), + expr->getMember(0)->getStringValue()) == 0) { return true; } } @@ -2337,22 +2345,24 @@ bool buildExpression (AstNode const* node, return true; } } + std::cout << "OR\n"; } return false; } int triagens::aql::replaceORwithIN (Optimizer* opt, - ExecutionPlan* plan, - Optimizer::Rule const* rule) { + ExecutionPlan* plan, + Optimizer::Rule const* rule) { + ENTER_BLOCK; std::vector nodes - = plan->findNodesOfType(triagens::aql::ExecutionNode::FILTER, true); + = plan->findNodesOfType(EN::FILTER, true); bool modified = false; for (auto n : nodes) { auto deps = n->getDependencies(); TRI_ASSERT(deps.size() == 1); - TRI_ASSERT(deps[0]->getType() == triagens::aql::ExecutionNode::CALCULATION); + TRI_ASSERT(deps[0]->getType() == EN::CALCULATION); auto fn = static_cast(n); auto cn = static_cast(deps[0]); @@ -2366,17 +2376,12 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, AstNode* expr = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); if (buildExpression(cn->expression()->node(), expr, plan)) { - deps = cn->getDependencies(); - TRI_ASSERT(deps.size() == 1); - - plan->unlinkNode(cn); - - cn = new CalculationNode(plan, plan->nextId(), - new Expression(plan->getAst(), expr), outVar[0]->clone()); - plan->registerNode(cn); - cn->addDependency(deps[0]); - fn->addDependency(cn); - modified = false; + auto newNode = new CalculationNode(plan, plan->nextId(), + new Expression(plan->getAst(), expr), + outVar[0]->clone()); + plan->registerNode(newNode); + plan->replaceNode(cn, newNode); + modified = true; plan->findVarUsage(); //FIXME appropriate place for this? } } @@ -2384,6 +2389,7 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, opt->addPlan(plan, rule->level, modified); return TRI_ERROR_NO_ERROR; + LEAVE_BLOCK; } // Local Variables: From 2c10e943b8d134427d8419af2f22ab0be13713e7 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 1 Nov 2014 17:12:44 +0000 Subject: [PATCH 03/26] still doesn't work --- arangod/Aql/OptimizerRules.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index c97df09c0c..8fb1044aa8 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2345,7 +2345,6 @@ bool buildExpression (AstNode const* node, return true; } } - std::cout << "OR\n"; } return false; @@ -2373,19 +2372,22 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, continue; } - AstNode* expr = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); + AstNode* ast = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); - if (buildExpression(cn->expression()->node(), expr, plan)) { - auto newNode = new CalculationNode(plan, plan->nextId(), - new Expression(plan->getAst(), expr), - outVar[0]->clone()); + if (buildExpression(cn->expression()->node(), ast, plan)) { + auto expr = new Expression(plan->getAst(), const_cast(ast)); + auto newNode = new CalculationNode(plan, plan->nextId(), expr, outVar[0]); + + //TODO clone outVar[0]? plan->registerNode(newNode); plan->replaceNode(cn, newNode); modified = true; - plan->findVarUsage(); //FIXME appropriate place for this? } } - + + if (modified) { + plan->findVarUsage(); + } opt->addPlan(plan, rule->level, modified); return TRI_ERROR_NO_ERROR; From 5f6964796e367bee39014e009b51002541affac0 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 1 Nov 2014 19:06:59 +0000 Subject: [PATCH 04/26] still not working --- arangod/Aql/OptimizerRules.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 8fb1044aa8..f159eefa90 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2371,9 +2371,11 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, if (outVar.size() != 1 || outVar[0]->id != inVar[0]->id) { continue; } + if (cn->expression()->node()->type != NODE_TYPE_OPERATOR_BINARY_OR) { + continue; + } AstNode* ast = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); - if (buildExpression(cn->expression()->node(), ast, plan)) { auto expr = new Expression(plan->getAst(), const_cast(ast)); auto newNode = new CalculationNode(plan, plan->nextId(), expr, outVar[0]); From 97206f8c30368665536a589c75d02741246398f4 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Sat, 1 Nov 2014 21:52:17 +0100 Subject: [PATCH 05/26] fixed cloning of AST nodes --- arangod/Aql/Ast.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arangod/Aql/Ast.cpp b/arangod/Aql/Ast.cpp index ad8f33214f..e28bccae3e 100644 --- a/arangod/Aql/Ast.cpp +++ b/arangod/Aql/Ast.cpp @@ -1018,20 +1018,25 @@ AstNode* Ast::clone (AstNode const* node) { } else if (type == NODE_TYPE_VALUE) { switch (node->value.type) { + case VALUE_TYPE_NULL: + copy->value.type = VALUE_TYPE_NULL; + break; case VALUE_TYPE_BOOL: + copy->value.type = VALUE_TYPE_BOOL; copy->setBoolValue(node->getBoolValue()); break; case VALUE_TYPE_INT: + copy->value.type = VALUE_TYPE_INT; copy->setIntValue(node->getIntValue()); break; case VALUE_TYPE_DOUBLE: + copy->value.type = VALUE_TYPE_DOUBLE; copy->setDoubleValue(node->getDoubleValue()); break; case VALUE_TYPE_STRING: + copy->value.type = VALUE_TYPE_STRING; copy->setStringValue(node->getStringValue()); break; - default: { - } } } From b301102655599173ebc24a615f509586691b599c Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 09:32:12 +0000 Subject: [PATCH 06/26] Jan's patch --- arangod/Aql/OptimizerRules.cpp | 159 +++++++++++++++++---------------- 1 file changed, 81 insertions(+), 78 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index f159eefa90..63fef36520 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2269,86 +2269,73 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, /// @brief //////////////////////////////////////////////////////////////////////////////// -// recursively check the expression in the calculation node, to make sure it is -// an OR of equality comparisons over the same variable and attribute. +struct OrToInConverter { + AstNode const* variableNode; + std::vector valueNodes; + std::string variableName; -bool buildExpression (AstNode const* node, - AstNode*& expr, - ExecutionPlan* plan) { - - if (node->type == NODE_TYPE_REFERENCE) { - if (expr->numMembers() == 0) { - return true; - } - auto thisVar = static_cast(node->getData()); - auto existingVar = - static_cast(expr->getMember(0)->getMember(0)->getData()); - if (thisVar->id == existingVar->id) { - return true; + AstNode* buildInExpression (Ast* ast) { + // the list of comparison values + auto list = ast->createNodeList(); + for (auto x : valueNodes) { + list->addMember(x); } - return false; + + // return a new IN operator node + return ast->createNodeBinaryOperator(NODE_TYPE_OPERATOR_BINARY_IN, + variableNode->clone(ast), + list); } - - if (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if(buildExpression(node->getMember(0), expr, plan)) { - if (expr->numMembers() == 0) { - expr->addMember(node->clone(plan->getAst())); - return true; - } - if (strcmp(node->getStringValue(), - expr->getMember(0)->getStringValue()) == 0) { + + bool canConvertExpression (AstNode const* node) { + if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { + return (canConvertExpression(node->getMember(0)) && + canConvertExpression(node->getMember(1))); + } + + if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { + auto lhs = node->getMember(0); + auto rhs = node->getMember(1); + + if (lhs->type == NODE_TYPE_VALUE && canConvertExpression(rhs)) { + // value == attr + valueNodes.push_back(lhs); return true; } - } - return false; - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { - auto lhs = node->getMember(0); - auto rhs = node->getMember(1); - - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if(buildExpression(rhs, expr, plan)){ - TRI_ASSERT(expr->numMembers() != 0); - if (lhs->type == NODE_TYPE_VALUE) { - if (expr->numMembers() == 1) { - expr->addMember(new AstNode(NODE_TYPE_LIST)); - } - // keep the value in the lhs - expr->getMember(1)->addMember(lhs->clone(plan->getAst())); - return true; - } - } - return false; - } - - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if(buildExpression(lhs, expr, plan)){ - TRI_ASSERT(expr->numMembers() != 0); - if (rhs->type == NODE_TYPE_VALUE) { - if (expr->numMembers() == 1) { - expr->addMember(new AstNode(NODE_TYPE_LIST)); - } - // keep the value in the rhs - expr->getMember(1)->addMember(rhs->clone(plan->getAst())); - return true; - } - } - return false; - } - return false; - } - - if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { - if (buildExpression(node->getMember(0), expr, plan)) { - if (buildExpression(node->getMember(1), expr, plan)) { + if (rhs->type == NODE_TYPE_VALUE && canConvertExpression(lhs)) { + // attr == value + valueNodes.push_back(rhs); return true; } + + // fall-through intentional } + + else if (node->type == NODE_TYPE_REFERENCE || + node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + // get a string representation of the node for comparisons + triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); + node->append(&buffer, false); + + if (variableName.empty()) { + TRI_ASSERT(valueNodes.empty()); + // we don't have anything to compare with + // now store the node and its stringified version for future comparisons + variableNode = node; + variableName = std::string(buffer.c_str(), buffer.length()); + return true; + } + else if (std::string(buffer.c_str(), buffer.length()) == variableName) { + // already have collected a variable. the other variable is identical + return true; + } + + // fall-through intentional + } + + return false; } - - return false; -} +}; int triagens::aql::replaceORwithIN (Optimizer* opt, ExecutionPlan* plan, @@ -2371,16 +2358,32 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, if (outVar.size() != 1 || outVar[0]->id != inVar[0]->id) { continue; } - if (cn->expression()->node()->type != NODE_TYPE_OPERATOR_BINARY_OR) { + if (cn->expression()->node()->type != NODE_TYPE_OPERATOR_BINARY_OR) { continue; } - - AstNode* ast = new AstNode(NODE_TYPE_OPERATOR_BINARY_IN); - if (buildExpression(cn->expression()->node(), ast, plan)) { - auto expr = new Expression(plan->getAst(), const_cast(ast)); - auto newNode = new CalculationNode(plan, plan->nextId(), expr, outVar[0]); - //TODO clone outVar[0]? + OrToInConverter converter; + if (converter.canConvertExpression(cn->expression()->node())) { + Expression* expr = nullptr; + ExecutionNode* newNode = nullptr; + auto inNode = converter.buildInExpression(plan->getAst()); + + try { + expr = new Expression(plan->getAst(), inNode); + } + catch (...) { + delete inNode; + throw; + } + + try { + newNode = new CalculationNode(plan, plan->nextId(), expr, outVar[0]); + } + catch (...) { + delete expr; + throw; + } + plan->registerNode(newNode); plan->replaceNode(cn, newNode); modified = true; From fa7668e1ceae48edb5a3ef7dd2eb614dc3ec6c44 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 10:38:05 +0000 Subject: [PATCH 07/26] snapshot --- arangod/Aql/OptimizerRules.cpp | 43 +++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 63fef36520..ee4f47465d 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2274,6 +2274,33 @@ struct OrToInConverter { std::vector valueNodes; std::string variableName; + // node should be an attribute access + AstNode* getValueFromArray (AstNode const* node) { + TRI_ASSERT(node->type == NODE_TYPE_ATTRIBUTE_ACCESS); + AstNode* array = node->getMember(0); + + if (array->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + array = getValueFromArray(array); + } + TRI_ASSERT(array->type == NODE_TYPE_ARRAY); + + char const* key = node->getStringValue(); + + for (auto i = 0; i < array->numMembers(); i++) { + AstNode* arrayElement = array->getMember(i); + TRI_ASSERT(arrayElement->type == NODE_TYPE_ARRAY_ELEMENT); + if (strcmp(key, arrayElement->getStringValue()) == 0){ + TRI_ASSERT(arrayElement->getMember(0)->type == NODE_TYPE_VALUE); + return arrayElement->getMember(0); + } + } + return nullptr; + } + + /*AstNode* getValueFromList (AstNode const* node) { + return node; //TODO implement + }*/ + AstNode* buildInExpression (Ast* ast) { // the list of comparison values auto list = ast->createNodeList(); @@ -2307,7 +2334,21 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } - + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(rhs)) { + // value == attr + valueNodes.push_back(lhs); + return true; + } + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(lhs)) { + // attr == value + valueNodes.push_back(rhs); + return true; + } + /*if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { + // value == attr + if (lhs->getMember(0)->type == NODE_TYPE_LIST) { + } + }*/ // fall-through intentional } From 1ff7e8d25295c5b20242142660f3eac50ea2f570 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 10:42:16 +0000 Subject: [PATCH 08/26] cleaning up --- arangod/Aql/Optimizer.cpp | 14 ++++++------- arangod/Aql/Optimizer.h | 14 +++++++------ arangod/Aql/OptimizerRules.cpp | 38 ++++------------------------------ 3 files changed, 19 insertions(+), 47 deletions(-) diff --git a/arangod/Aql/Optimizer.cpp b/arangod/Aql/Optimizer.cpp index 075155b848..7ee136e35a 100644 --- a/arangod/Aql/Optimizer.cpp +++ b/arangod/Aql/Optimizer.cpp @@ -420,7 +420,7 @@ void Optimizer::setupRules () { moveFiltersUpRule, moveFiltersUpRule_pass4, true); - + ////////////////////////////////////////////////////////////////////////////// /// "Pass 5": try to remove redundant or unnecessary nodes (second try) /// use levels between 601 and 699 for this @@ -450,6 +450,12 @@ void Optimizer::setupRules () { /// "Pass 6": use indexes if possible for FILTER and/or SORT nodes /// use levels between 701 and 799 for this ////////////////////////////////////////////////////////////////////////////// + + // try to replace simple OR conditions with IN + registerRule("replace-OR-with-IN", + replaceORwithIN, + replaceORwithIN_pass6, + true); // try to find a filter after an enumerate collection and find an index . . . registerRule("use-index-range", @@ -463,12 +469,6 @@ void Optimizer::setupRules () { useIndexForSort_pass6, true); - // try to replace simple OR conditions with IN - registerRule("replace-OR-with-IN", - replaceORwithIN, - replaceORwithIN_pass6, - true); - if (ExecutionEngine::isCoordinator()) { // distribute operations in cluster registerRule("scatter-in-cluster", diff --git a/arangod/Aql/Optimizer.h b/arangod/Aql/Optimizer.h index 1090eed34b..051fa18fd0 100644 --- a/arangod/Aql/Optimizer.h +++ b/arangod/Aql/Optimizer.h @@ -103,6 +103,7 @@ namespace triagens { // move filters up the dependency chain (to make result sets as small // as possible as early as possible) moveFiltersUpRule_pass4 = 620, + ////////////////////////////////////////////////////////////////////////////// /// "Pass 5": try to remove redundant or unnecessary nodes (second try) @@ -125,14 +126,15 @@ namespace triagens { ////////////////////////////////////////////////////////////////////////////// pass6 = 800, - // try to find a filter after an enumerate collection and find an index . . . - useIndexRange_pass6 = 810, - - // try to find sort blocks which are superseeded by indexes - useIndexForSort_pass6 = 820, // replace simple OR conditions with IN - replaceORwithIN_pass6 = 830, + replaceORwithIN_pass6 = 810, + + // try to find a filter after an enumerate collection and find an index . . . + useIndexRange_pass6 = 820, + + // try to find sort blocks which are superseeded by indexes + useIndexForSort_pass6 = 830, ////////////////////////////////////////////////////////////////////////////// /// "Pass 10": final transformations for the cluster diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index ee4f47465d..4e83b902d8 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2274,33 +2274,6 @@ struct OrToInConverter { std::vector valueNodes; std::string variableName; - // node should be an attribute access - AstNode* getValueFromArray (AstNode const* node) { - TRI_ASSERT(node->type == NODE_TYPE_ATTRIBUTE_ACCESS); - AstNode* array = node->getMember(0); - - if (array->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - array = getValueFromArray(array); - } - TRI_ASSERT(array->type == NODE_TYPE_ARRAY); - - char const* key = node->getStringValue(); - - for (auto i = 0; i < array->numMembers(); i++) { - AstNode* arrayElement = array->getMember(i); - TRI_ASSERT(arrayElement->type == NODE_TYPE_ARRAY_ELEMENT); - if (strcmp(key, arrayElement->getStringValue()) == 0){ - TRI_ASSERT(arrayElement->getMember(0)->type == NODE_TYPE_VALUE); - return arrayElement->getMember(0); - } - } - return nullptr; - } - - /*AstNode* getValueFromList (AstNode const* node) { - return node; //TODO implement - }*/ - AstNode* buildInExpression (Ast* ast) { // the list of comparison values auto list = ast->createNodeList(); @@ -2334,21 +2307,18 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(rhs)) { + if ((lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || lhs->type == NODE_TYPE_INDEXED_ACCESS) + && canConvertExpression(rhs)) { // value == attr valueNodes.push_back(lhs); return true; } - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(lhs)) { + if ((rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_INDEXED_ACCESS) + && canConvertExpression(lhs)) { // attr == value valueNodes.push_back(rhs); return true; } - /*if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { - // value == attr - if (lhs->getMember(0)->type == NODE_TYPE_LIST) { - } - }*/ // fall-through intentional } From 62cb8d3a935da8a3c2e38d44a1b6b55af789fee8 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 11:10:18 +0000 Subject: [PATCH 09/26] bugfix --- arangod/Aql/OptimizerRules.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 4e83b902d8..509c2bab55 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2307,14 +2307,27 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } - if ((lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || lhs->type == NODE_TYPE_INDEXED_ACCESS) - && canConvertExpression(rhs)) { - // value == attr + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if (lhs->getMember(0)->isArray() && canConvertExpression(rhs)) { + // value == attr + valueNodes.push_back(lhs); + return true; + } + } + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if (rhs->getMember(0)->isArray() && canConvertExpression(lhs)) { + // value == attr + valueNodes.push_back(rhs); + return true; + } + } + + if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { + // attr == value valueNodes.push_back(lhs); return true; } - if ((rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || rhs->type == NODE_TYPE_INDEXED_ACCESS) - && canConvertExpression(lhs)) { + if (rhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(lhs)) { // attr == value valueNodes.push_back(rhs); return true; From 836fa08831fe7f00a29cb53d903ed1b73281fe90 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 11:45:43 +0000 Subject: [PATCH 10/26] adding tests --- .../aql-optimizer-rule-replace-OR-with-IN.js | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js new file mode 100644 index 0000000000..a9cde19788 --- /dev/null +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -0,0 +1,164 @@ +/*jshint strict: false, maxlen: 500 */ +/*global require, assertEqual, assertTrue, AQL_EXPLAIN */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tests for Ahuacatl, skiplist index queries +/// +/// @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 +/// @author Copyright 2014, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +var internal = require("internal"); +var jsunity = require("jsunity"); +var helper = require("org/arangodb/aql-helper"); +var getQueryResults = helper.getQueryResults; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test suite +//////////////////////////////////////////////////////////////////////////////// + +function NewAqlReplaceORWithINTestSuite () { + var replace; + var ruleName = "replace-OR-with-IN"; + + var isRuleUsed = function (query, params) { + var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all", ruleName ] } }); + assertTrue(result.plan.rules.indexOf(ruleName) === -1, query); + }; + + return { + +//////////////////////////////////////////////////////////////////////////////// +/// @brief set up +//////////////////////////////////////////////////////////////////////////////// + + setUp : function () { + internal.db._drop("UnitTestsNewAqlReplaceORWithINTestSuite"); + replace = internal.db._create("UnitTestsNewAqlReplaceORWithINTestSuite"); + + for (var i = 1; i <= 10; ++i) { + replace.save({ "value" : i}); + replace.save({"a" : {"b" : i}}); + } + + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tear down +//////////////////////////////////////////////////////////////////////////////// + + tearDown : function () { + internal.db._drop("UnitTestsNewAqlReplaceORWithINTestSuite"); + replace = null; + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test the rule fires for actual values +//////////////////////////////////////////////////////////////////////////////// + + testFires2Values1 : function () { + var query = "FOR x IN " + replace.name() + + " FILTER x.value == 1 || x.value == 2 SORT x.value RETURN x.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query); + assertEqual(expected, actual); + }, + + testFires2Values2 : function () { + var query = "FOR x IN " + replace.name() + + " FILTER x.a.b == 1 || x.a.b == 2 SORT x.a.b RETURN x.a.b"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query); + assertEqual(expected, actual); + }, + + testFires4Values1 : function () { + var query = "FOR x IN " + replace.name() + " FILTER x.value == 1 " + + "|| x.value == 2 || x.value == 3 || x.value == 4 SORT x.value RETURN x.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2, 3, 4]; + var actual = getQueryResults(query); + assertEqual(expected, actual); + }, + + testFiresNoAttributeAccess : function () { + var query = + "FOR x IN " + replace.name() + " FILTER x == 1 || x == 2 RETURN x"; + + isRuleUsed(query, {}); + + var expected = [ ]; + var actual = getQueryResults(query); + assertEqual(expected, actual); + }, + + testFiresNoCollection : function () { + var query = + "FOR x in 1..10 FILTER x == 1 || x == 2 SORT x RETURN x"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query); + assertEqual(expected, actual); + + }, + + testFiresBind : function () { + var query = + "FOR v IN numbers FILTER v.value == @a || v.value == @b SORT v.value RETURN v.value"; + var params = {"a": 1, "b": 2}; + + isRuleUsed(query, params); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, params); + assertEqual(expected, actual); + + }, + + testFiresVariables : function () { + var query = + "LET x = 1 LET y = 2 FOR v IN numbers FILTER v.value == x || v.value == y SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + + }, + }; +} + +jsunity.run(NewAqlReplaceORWithINTestSuite); + +return jsunity.done(); From 0ceb51cbd7f959dc17a1977cba7a8427bf446fd2 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 20:24:59 +0000 Subject: [PATCH 11/26] compiler warning. --- arangod/Aql/ExecutionBlock.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arangod/Aql/ExecutionBlock.cpp b/arangod/Aql/ExecutionBlock.cpp index 7c79a8ff9d..7b205b703c 100644 --- a/arangod/Aql/ExecutionBlock.cpp +++ b/arangod/Aql/ExecutionBlock.cpp @@ -3748,8 +3748,9 @@ size_t GatherBlock::skipSome (size_t atLeast, size_t atMost) { // the non-simple case . . . size_t available = 0; // nr of available rows - size_t index; // an index of a non-empty buffer - + size_t index = 0; // an index of a non-empty buffer + TRI_ASSERT(_dependencies.size() != 0); + // pull more blocks from dependencies . . . for (size_t i = 0; i < _dependencies.size(); i++) { if (_gatherBlockBuffer.at(i).empty()) { From d992b100fa4bda17f015067b5ee70ad749bd30bf Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 20:25:16 +0000 Subject: [PATCH 12/26] tests complete. --- .../aql-optimizer-rule-replace-OR-with-IN.js | 129 +++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js index a9cde19788..9fa62f9b19 100644 --- a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -42,7 +42,14 @@ function NewAqlReplaceORWithINTestSuite () { var ruleName = "replace-OR-with-IN"; var isRuleUsed = function (query, params) { - var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all", ruleName ] } }); + var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all", "+" + ruleName ] } }); + assertTrue(result.plan.rules.indexOf(ruleName) !== -1, query); + var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all" ] } }); + assertTrue(result.plan.rules.indexOf(ruleName) === -1, query); + }; + + var ruleIsNotUsed = function (query, params) { + var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all", "+" + ruleName ] } }); assertTrue(result.plan.rules.indexOf(ruleName) === -1, query); }; @@ -134,7 +141,8 @@ function NewAqlReplaceORWithINTestSuite () { testFiresBind : function () { var query = - "FOR v IN numbers FILTER v.value == @a || v.value == @b SORT v.value RETURN v.value"; + "FOR v IN " + replace.name() + + " FILTER v.value == @a || v.value == @b SORT v.value RETURN v.value"; var params = {"a": 1, "b": 2}; isRuleUsed(query, params); @@ -147,14 +155,128 @@ function NewAqlReplaceORWithINTestSuite () { testFiresVariables : function () { var query = - "LET x = 1 LET y = 2 FOR v IN numbers FILTER v.value == x || v.value == y SORT v.value RETURN v.value"; + "LET x = 1 LET y = 2 FOR v IN " + replace.name() + + " FILTER v.value == x || v.value == y SORT v.value RETURN v.value"; isRuleUsed(query, {}); var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + }, + + testFires2AttributeAccesses1 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER v.value == x.a || v.value == x.b SORT v.value RETURN v.value" + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFires2AttributeAccesses2 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER x.a == v.value || v.value == x.b SORT v.value RETURN v.value" + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFires2AttributeAccesses3 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER x.a == v.value || x.b == v.value SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + + testFiresMixed1 : function () { + var query = + "LET x = [1,2] FOR v IN " + replace.name() + + " FILTER x[0] == v.value || x[1] == v.value SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFiresMixed2 : function () { + var query = + "LET x = [1,2] LET y = {b:2} FOR v IN " + replace.name() + + " FILTER x[0] == v.value || y.b == v.value SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 2 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testDudDifferentAttributes1 : function () { + var query = + "FOR x IN " + replace.name() + " FILTER x.val1 == 1 || x.val2 == 2 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudDifferentVariables : function () { + var query = + "FOR y IN " + replace.name() + " FOR x IN " + replace.name() + + " FILTER x.val1 == 1 || y.val1 == 2 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudDifferentAttributes2: function () { + var query = + "FOR x IN " + replace.name() + " FILTER x.val1 == 1 || x == 2 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudNoOR1: function () { + var query = + "FOR x IN " + replace.name() + " FILTER x.val1 == 1 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudNoOR2: function () { + var query = + "FOR x IN " + replace.name() + + " FILTER x.val1 == 1 && x.val2 == 2 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudNonEquality1: function () { + var query = + "FOR x IN " + replace.name() + + " FILTER x.val1 > 1 || x.val1 == 2 RETURN x"; + + ruleIsNotUsed(query, {}); + }, + + testDudNonEquality2: function () { + var query = + "FOR x IN " + replace.name() + + " FILTER x.val1 == 1 || 2 < x.val1 RETURN x"; + + ruleIsNotUsed(query, {}); }, }; } @@ -162,3 +284,4 @@ function NewAqlReplaceORWithINTestSuite () { jsunity.run(NewAqlReplaceORWithINTestSuite); return jsunity.done(); + From a5fffd696e1f992eeede477da0bff12028f3e8ea Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 20:46:18 +0000 Subject: [PATCH 13/26] more tests --- .../aql-optimizer-rule-replace-OR-with-IN.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js index 9fa62f9b19..b55cad3df9 100644 --- a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -226,6 +226,30 @@ function NewAqlReplaceORWithINTestSuite () { assertEqual(expected, actual); }, + testSelfReference1 : function () { + var query = + "FOR v IN " + replace.name() + + " FILTER v.value == v.a.b || v.value == 10 || v.value == 7 SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 7, 10 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testSelfReference2 : function () { + var query = + "FOR v IN " + replace.name() + + " FILTER v.a.b == v.value || v.value == 10 || 7 == v.value SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 7, 10 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + testDudDifferentAttributes1 : function () { var query = "FOR x IN " + replace.name() + " FILTER x.val1 == 1 || x.val2 == 2 RETURN x"; From f6e8dcd6ca923e473f7e68ca3af51820c8406801 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 3 Nov 2014 22:23:41 +0000 Subject: [PATCH 14/26] more general case working. Code needs cleaning up --- arangod/Aql/OptimizerRules.cpp | 70 ++++++++++++++++++++++++++++------ 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 509c2bab55..688f9869ab 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2272,6 +2272,8 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, struct OrToInConverter { AstNode const* variableNode; std::vector valueNodes; + std::vector possibleNames; + std::vector possibleNodes; std::string variableName; AstNode* buildInExpression (Ast* ast) { @@ -2307,19 +2309,63 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if (lhs->getMember(0)->isArray() && canConvertExpression(rhs)) { - // value == attr - valueNodes.push_back(lhs); - return true; - } + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS + && lhs->getMember(0)->isArray() && canConvertExpression(rhs)) { + // value == attr + valueNodes.push_back(lhs); + return true; } - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if (rhs->getMember(0)->isArray() && canConvertExpression(lhs)) { - // value == attr - valueNodes.push_back(rhs); - return true; - } + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS + && rhs->getMember(0)->isArray() && canConvertExpression(lhs)) { + // value == attr + valueNodes.push_back(rhs); + return true; + } + // for things like "FOR a IN docs a.x == a.y || a.z == 1 RETURN a" + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS + && lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + if (variableName.empty()) { + triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); + lhs->append(&buffer, false); + auto lhsName = std::string(buffer.c_str(), buffer.length()); + + triagens::basics::StringBuffer buffer1(TRI_UNKNOWN_MEM_ZONE); + rhs->append(&buffer1, false); + auto rhsName = std::string(buffer1.c_str(), buffer1.length()); + + if (! possibleNames.empty()) { + if (lhsName == possibleNames[0] || rhsName == possibleNames[0]) { + variableName = possibleNames[0]; + variableNode = possibleNodes[0]; + } + else if (lhsName == possibleNames[1] || rhsName == possibleNames[1]) { + variableName = possibleNames[1]; + variableNode = possibleNodes[1]; + } + else { + return false; + } + possibleNames.clear(); + possibleNodes.clear(); + } + else { + possibleNames.push_back(lhsName); + possibleNames.push_back(rhsName); + possibleNodes.push_back(lhs); + possibleNodes.push_back(rhs); + return true; + } + } + if (! variableName.empty()) { + if (canConvertExpression(rhs)) { + valueNodes.push_back(lhs); + return true; + } + if (canConvertExpression(lhs)) { + valueNodes.push_back(rhs); + return true; + } + } } if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { From 04770d356325e4029feed9efd319ed3405d2c29a Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 13:01:51 +0000 Subject: [PATCH 15/26] nearly working better version of previous --- arangod/Aql/OptimizerRules.cpp | 203 +++++++++++++++++++++------------ 1 file changed, 128 insertions(+), 75 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 688f9869ab..4fe4fddafb 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2270,11 +2270,34 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, //////////////////////////////////////////////////////////////////////////////// struct OrToInConverter { - AstNode const* variableNode; + AstNode const* variableNode = nullptr; + //const char* variableName; std::vector valueNodes; - std::vector possibleNames; std::vector possibleNodes; - std::string variableName; + std::vector orConditions; + + std::string getString (AstNode const* node) { + triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); + node->append(&buffer, false); + return std::string(buffer.c_str(), buffer.length()); + } + + bool compareNodes (AstNode const* lhs, AstNode const* rhs) { + if (lhs->numMembers() == rhs->numMembers()) { + if (lhs->numMembers() == 1) { + return (getString(lhs) == getString(rhs)); + } + else { + for (size_t i = 0; i < lhs->numMembers(); i++) { + if (! compareNodes(lhs->getMember(i), rhs->getMember(i))) { + return false; + } + } + return true; + } + } + return false; + } AstNode* buildInExpression (Ast* ast) { // the list of comparison values @@ -2289,6 +2312,100 @@ struct OrToInConverter { list); } + bool flattenOr (AstNode const* node) { + if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { + return (flattenOr(node->getMember(0)) && flattenOr(node->getMember(1))); + } + if (node->type == NODE_TYPE_OPERATOR_BINARY_EQ) { + orConditions.push_back(node); + return true; + } + return false; + } + + bool findNameAndNode (AstNode const* node) { + + if (! flattenOr(node)) { + return false; + } + TRI_ASSERT(orConditions.size() > 1); + + for (AstNode const* n: orConditions) { + + auto lhs = n->getMember(0); + auto rhs = n->getMember(1); + + if (variableNode != nullptr) { + return (compareNodes(lhs, variableNode) + || compareNodes(rhs, variableNode)); + } + + if (lhs->type == NODE_TYPE_VALUE + && (rhs->type == NODE_TYPE_REFERENCE + || rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS + || rhs->type == NODE_TYPE_INDEXED_ACCESS)) { + variableNode = rhs; + //variableName = getString(rhs); + return true; + } + + if (rhs->type == NODE_TYPE_VALUE + && (lhs->type == NODE_TYPE_REFERENCE + || lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS + || lhs->type == NODE_TYPE_INDEXED_ACCESS)) { + variableNode = lhs; + //variableName = getString(lhs); + return true; + } + + if (lhs->type == NODE_TYPE_REFERENCE) { + variableNode = lhs; + //variableName = getString(lhs); + return true; + } + if (rhs->type == NODE_TYPE_REFERENCE) { + variableNode = rhs; + //variableName = getString(rhs); + return true; + } + + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || + lhs->type == NODE_TYPE_INDEXED_ACCESS) { + if (possibleNodes.size() == 2) { + for (size_t i = 0; i < 2; i++) { + if (compareNodes(lhs, possibleNodes[i])) { + variableNode = possibleNodes[i]; + //variableName = getString(possibleNodes[i]); + return true; + } + } + return false; + } + else { + possibleNodes.push_back(lhs); + } + } + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || + rhs->type == NODE_TYPE_INDEXED_ACCESS) { + if (possibleNodes.size() == 2) { + for (size_t i = 0; i < 2; i++) { + if (compareNodes(rhs, possibleNodes[i])) { + variableNode = possibleNodes[i]; + //variableName = getString(possibleNodes[i]); + return true; + } + } + return false; + } + else { + possibleNodes.push_back(rhs); + //continue; + } + } + } + return false; + } + bool canConvertExpression (AstNode const* node) { if (node->type == NODE_TYPE_OPERATOR_BINARY_OR) { return (canConvertExpression(node->getMember(0)) && @@ -2309,65 +2426,16 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS - && lhs->getMember(0)->isArray() && canConvertExpression(rhs)) { + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(rhs)) { // value == attr valueNodes.push_back(lhs); return true; } - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS - && rhs->getMember(0)->isArray() && canConvertExpression(lhs)) { + if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(lhs)) { // value == attr valueNodes.push_back(rhs); return true; } - // for things like "FOR a IN docs a.x == a.y || a.z == 1 RETURN a" - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS - && lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - if (variableName.empty()) { - triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); - lhs->append(&buffer, false); - auto lhsName = std::string(buffer.c_str(), buffer.length()); - - triagens::basics::StringBuffer buffer1(TRI_UNKNOWN_MEM_ZONE); - rhs->append(&buffer1, false); - auto rhsName = std::string(buffer1.c_str(), buffer1.length()); - - if (! possibleNames.empty()) { - if (lhsName == possibleNames[0] || rhsName == possibleNames[0]) { - variableName = possibleNames[0]; - variableNode = possibleNodes[0]; - } - else if (lhsName == possibleNames[1] || rhsName == possibleNames[1]) { - variableName = possibleNames[1]; - variableNode = possibleNodes[1]; - } - else { - return false; - } - possibleNames.clear(); - possibleNodes.clear(); - } - else { - possibleNames.push_back(lhsName); - possibleNames.push_back(rhsName); - possibleNodes.push_back(lhs); - possibleNodes.push_back(rhs); - return true; - } - } - if (! variableName.empty()) { - if (canConvertExpression(rhs)) { - valueNodes.push_back(lhs); - return true; - } - if (canConvertExpression(lhs)) { - valueNodes.push_back(rhs); - return true; - } - } - } - if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { // attr == value valueNodes.push_back(lhs); @@ -2380,29 +2448,13 @@ struct OrToInConverter { } // fall-through intentional } - else if (node->type == NODE_TYPE_REFERENCE || - node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { + node->type == NODE_TYPE_ATTRIBUTE_ACCESS || + node->type == NODE_TYPE_INDEXED_ACCESS) { // get a string representation of the node for comparisons - triagens::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE); - node->append(&buffer, false); - - if (variableName.empty()) { - TRI_ASSERT(valueNodes.empty()); - // we don't have anything to compare with - // now store the node and its stringified version for future comparisons - variableNode = node; - variableName = std::string(buffer.c_str(), buffer.length()); - return true; - } - else if (std::string(buffer.c_str(), buffer.length()) == variableName) { - // already have collected a variable. the other variable is identical - return true; - } - - // fall-through intentional + std::string nodeString = getString(node); + return nodeString == getString(variableNode); } - return false; } }; @@ -2433,7 +2485,8 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, } OrToInConverter converter; - if (converter.canConvertExpression(cn->expression()->node())) { + if (converter.findNameAndNode(cn->expression()->node()) + && converter.canConvertExpression(cn->expression()->node())) { Expression* expr = nullptr; ExecutionNode* newNode = nullptr; auto inNode = converter.buildInExpression(plan->getAst()); From 2e48bbb82bbce89f2aa33020e880a19fb7919fd6 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 13:16:39 +0000 Subject: [PATCH 16/26] tests are green again --- arangod/Aql/OptimizerRules.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 4fe4fddafb..90cf526b4f 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2284,7 +2284,7 @@ struct OrToInConverter { bool compareNodes (AstNode const* lhs, AstNode const* rhs) { if (lhs->numMembers() == rhs->numMembers()) { - if (lhs->numMembers() == 1) { + if (lhs->numMembers() < 2) { return (getString(lhs) == getString(rhs)); } else { @@ -2379,7 +2379,6 @@ struct OrToInConverter { return true; } } - return false; } else { possibleNodes.push_back(lhs); From 206df8fc128603021db805691d0fa70f86fdbef4 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 20:28:30 +0000 Subject: [PATCH 17/26] more tests. --- .../aql-optimizer-rule-replace-OR-with-IN.js | 87 ++++++++++++++++++- 1 file changed, 84 insertions(+), 3 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js index b55cad3df9..c878365b79 100644 --- a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -53,6 +53,14 @@ function NewAqlReplaceORWithINTestSuite () { assertTrue(result.plan.rules.indexOf(ruleName) === -1, query); }; + var executeWithRule = function (query, params) { + return AQL_EXECUTE(query, params, { optimizer: { rules: [ "-all", "+" + ruleName ] } }).json; + } + + var executeWithoutRule = function (query, params) { + return AQL_EXECUTE(query, params, { optimizer: { rules: [ "-all" ] } }).json; + } + return { //////////////////////////////////////////////////////////////////////////////// @@ -64,8 +72,10 @@ function NewAqlReplaceORWithINTestSuite () { replace = internal.db._create("UnitTestsNewAqlReplaceORWithINTestSuite"); for (var i = 1; i <= 10; ++i) { - replace.save({ "value" : i}); + replace.save({ "value" : i, x: [i]}); replace.save({"a" : {"b" : i}}); + replace.save({"value": i + 10, "bb": i, "cc": 10 - i }); + } }, @@ -226,7 +236,7 @@ function NewAqlReplaceORWithINTestSuite () { assertEqual(expected, actual); }, - testSelfReference1 : function () { + testFiresSelfReference1 : function () { var query = "FOR v IN " + replace.name() + " FILTER v.value == v.a.b || v.value == 10 || v.value == 7 SORT v.value RETURN v.value"; @@ -238,7 +248,7 @@ function NewAqlReplaceORWithINTestSuite () { assertEqual(expected, actual); }, - testSelfReference2 : function () { + testFiresSelfReference2 : function () { var query = "FOR v IN " + replace.name() + " FILTER v.a.b == v.value || v.value == 10 || 7 == v.value SORT v.value RETURN v.value"; @@ -249,7 +259,71 @@ function NewAqlReplaceORWithINTestSuite () { var actual = getQueryResults(query, {}); assertEqual(expected, actual); }, + + testFiresAttributeIsList1 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER v.x[0] == x.a || v.x[0] == 3 SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + testFiresAttributeIsList2 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER x.a == v.x[0] || v.x[0] == 3 SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFiresAttributeIsList3 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER x.a == v.x[0] || 3 == v.x[0] SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFiresAttributeIsList4 : function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER v.x[0] == x.a || 3 == v.x[0] SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + }, + + testFires2Loops: function () { + var query = + "FOR x IN " + replace.name() + + " FOR y IN " + replace.name() + + " FILTER x.value == y.bb || x.value == y.cc" + + " FILTER x.value != null SORT x.value RETURN x.value"; + + isRuleUsed(query, {}); + + var expected = [ 1, 1, 2, 2, 3, 3, 4, 4, 5, 6, 6, 7, 7, 8, 8, 9, 9, 10 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); + assertEqual(expected, executeWithoutRule(query, {})); + }, + testDudDifferentAttributes1 : function () { var query = "FOR x IN " + replace.name() + " FILTER x.val1 == 1 || x.val2 == 2 RETURN x"; @@ -302,6 +376,13 @@ function NewAqlReplaceORWithINTestSuite () { ruleIsNotUsed(query, {}); }, + + testDudAttributeIsList: function () { + var query = + "LET x = {a:1,b:2} FOR v IN " + replace.name() + + " FILTER v.x[0] == x.a || v.x[1] == 3 SORT v.value RETURN v.value"; + ruleIsNotUsed(query, {}); + } }; } From 9c71ee48c7dc9e4c49006bfcd33218f13064a5c0 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 20:38:56 +0000 Subject: [PATCH 18/26] and more --- .../aql-optimizer-rule-replace-OR-with-IN.js | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js index c878365b79..7fdd028134 100644 --- a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -323,6 +323,47 @@ function NewAqlReplaceORWithINTestSuite () { assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); assertEqual(expected, executeWithoutRule(query, {})); }, + + testFiresNonsense1: function () { + var query = + "FOR v in " + replace.name() + + " FILTER 1 == 2 || v.value == 2 || v.value == 3 SORT v.value RETURN v.value" ; + + isRuleUsed(query, {}); + + var expected = [ 2, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); + assertEqual(expected, executeWithoutRule(query, {})); + }, + + testFiresNonsense2: function () { + var query = "FOR v in " + replace.name() + + " FILTER 1 == 2 || 2 == v.value || v.value == 3 SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 2, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); + assertEqual(expected, executeWithoutRule(query, {})); + }, + + testFiresNonsense3: function () { + var query = + "FOR v in " + replace.name() + + " FILTER v.value == 2 || 3 == v.value || 1 == 2 SORT v.value RETURN v.value"; + + isRuleUsed(query, {}); + + var expected = [ 2, 3 ]; + var actual = getQueryResults(query, {}); + assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); + assertEqual(expected, executeWithoutRule(query, {})); + }, testDudDifferentAttributes1 : function () { var query = From 61afce9d10c5f071d6913db502c0b28790ca4b90 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 21:04:56 +0000 Subject: [PATCH 19/26] cleaning up --- arangod/Aql/OptimizerRules.cpp | 36 ++++++++++------------------------ 1 file changed, 10 insertions(+), 26 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 90cf526b4f..b721d57720 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2320,10 +2320,13 @@ struct OrToInConverter { orConditions.push_back(node); return true; } + if (node->type == NODE_TYPE_VALUE) { + return true; + } return false; } - bool findNameAndNode (AstNode const* node) { + bool findCommonNode (AstNode const* node) { if (! flattenOr(node)) { return false; @@ -2340,32 +2343,13 @@ struct OrToInConverter { || compareNodes(rhs, variableNode)); } - if (lhs->type == NODE_TYPE_VALUE - && (rhs->type == NODE_TYPE_REFERENCE - || rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS - || rhs->type == NODE_TYPE_INDEXED_ACCESS)) { + if (lhs->isConstant()) { variableNode = rhs; - //variableName = getString(rhs); return true; } - if (rhs->type == NODE_TYPE_VALUE - && (lhs->type == NODE_TYPE_REFERENCE - || lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS - || lhs->type == NODE_TYPE_INDEXED_ACCESS)) { + if (rhs->isConstant()) { variableNode = lhs; - //variableName = getString(lhs); - return true; - } - - if (lhs->type == NODE_TYPE_REFERENCE) { - variableNode = lhs; - //variableName = getString(lhs); - return true; - } - if (rhs->type == NODE_TYPE_REFERENCE) { - variableNode = rhs; - //variableName = getString(rhs); return true; } @@ -2375,7 +2359,6 @@ struct OrToInConverter { for (size_t i = 0; i < 2; i++) { if (compareNodes(lhs, possibleNodes[i])) { variableNode = possibleNodes[i]; - //variableName = getString(possibleNodes[i]); return true; } } @@ -2390,7 +2373,6 @@ struct OrToInConverter { for (size_t i = 0; i < 2; i++) { if (compareNodes(rhs, possibleNodes[i])) { variableNode = possibleNodes[i]; - //variableName = getString(possibleNodes[i]); return true; } } @@ -2398,7 +2380,6 @@ struct OrToInConverter { } else { possibleNodes.push_back(rhs); - //continue; } } } @@ -2453,7 +2434,10 @@ struct OrToInConverter { // get a string representation of the node for comparisons std::string nodeString = getString(node); return nodeString == getString(variableNode); + } else if (node->isBoolValue()) { + return true; } + return false; } }; @@ -2484,7 +2468,7 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, } OrToInConverter converter; - if (converter.findNameAndNode(cn->expression()->node()) + if (converter.findCommonNode(cn->expression()->node()) && converter.canConvertExpression(cn->expression()->node())) { Expression* expr = nullptr; ExecutionNode* newNode = nullptr; From 3b49921a15c2e055d059946d43f7558e1de7c8a7 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 21:09:21 +0000 Subject: [PATCH 20/26] more cleaning up --- arangod/Aql/OptimizerRules.cpp | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index b721d57720..6e6c6f6080 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2396,36 +2396,15 @@ struct OrToInConverter { auto lhs = node->getMember(0); auto rhs = node->getMember(1); - if (lhs->type == NODE_TYPE_VALUE && canConvertExpression(rhs)) { - // value == attr + if (canConvertExpression(rhs) && ! canConvertExpression(lhs)) { valueNodes.push_back(lhs); return true; - } - if (rhs->type == NODE_TYPE_VALUE && canConvertExpression(lhs)) { - // attr == value + } + + if (canConvertExpression(lhs) && ! canConvertExpression(rhs)) { valueNodes.push_back(rhs); return true; - } - if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(rhs)) { - // value == attr - valueNodes.push_back(lhs); - return true; - } - if (rhs->type == NODE_TYPE_ATTRIBUTE_ACCESS && canConvertExpression(lhs)) { - // value == attr - valueNodes.push_back(rhs); - return true; - } - if (lhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(rhs)) { - // attr == value - valueNodes.push_back(lhs); - return true; - } - if (rhs->type == NODE_TYPE_INDEXED_ACCESS && canConvertExpression(lhs)) { - // attr == value - valueNodes.push_back(rhs); - return true; - } + } // fall-through intentional } else if (node->type == NODE_TYPE_REFERENCE || From 643bb4f43b5f651a9c426dcbda1a75d7a705686d Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 21:22:19 +0000 Subject: [PATCH 21/26] more tests --- .../aql-optimizer-rule-replace-OR-with-IN.js | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js index 7fdd028134..21579d81dd 100644 --- a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js +++ b/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js @@ -102,6 +102,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires2Values2 : function () { @@ -113,6 +114,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires4Values1 : function () { @@ -124,6 +126,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2, 3, 4]; var actual = getQueryResults(query); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresNoAttributeAccess : function () { @@ -135,6 +138,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ ]; var actual = getQueryResults(query); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresNoCollection : function () { @@ -146,7 +150,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query); assertEqual(expected, actual); - + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresBind : function () { @@ -160,7 +164,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, params); assertEqual(expected, actual); - + assertEqual(executeWithRule(query, params), executeWithoutRule(query, params)); }, testFiresVariables : function () { @@ -173,6 +177,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires2AttributeAccesses1 : function () { @@ -185,6 +190,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires2AttributeAccesses2 : function () { @@ -197,6 +203,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires2AttributeAccesses3 : function () { @@ -209,6 +216,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, @@ -222,6 +230,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresMixed2 : function () { @@ -234,6 +243,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 2 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresSelfReference1 : function () { @@ -246,6 +256,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 7, 10 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresSelfReference2 : function () { @@ -258,6 +269,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 7, 10 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresAttributeIsList1 : function () { @@ -270,6 +282,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 3 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresAttributeIsList2 : function () { @@ -282,6 +295,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 3 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresAttributeIsList3 : function () { @@ -294,6 +308,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 3 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFiresAttributeIsList4 : function () { @@ -306,6 +321,7 @@ function NewAqlReplaceORWithINTestSuite () { var expected = [ 1, 3 ]; var actual = getQueryResults(query, {}); assertEqual(expected, actual); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); }, testFires2Loops: function () { @@ -364,6 +380,16 @@ function NewAqlReplaceORWithINTestSuite () { assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); assertEqual(expected, executeWithoutRule(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, {}); + assertEqual(executeWithRule(query, {}), executeWithoutRule(query, {})); + + }, testDudDifferentAttributes1 : function () { var query = From 37ab716edcf4e29084b662a1e1ff1ac06950d0d5 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 21:42:34 +0000 Subject: [PATCH 22/26] cleaning up --- arangod/Aql/OptimizerRules.cpp | 34 ++++++++-------------------------- 1 file changed, 8 insertions(+), 26 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 6e6c6f6080..efd4c83fb6 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2270,8 +2270,8 @@ int triagens::aql::undistributeRemoveAfterEnumColl (Optimizer* opt, //////////////////////////////////////////////////////////////////////////////// struct OrToInConverter { - AstNode const* variableNode = nullptr; - //const char* variableName; + AstNode const* variableNode; + std::string variableName; std::vector valueNodes; std::vector possibleNodes; std::vector orConditions; @@ -2282,23 +2282,6 @@ struct OrToInConverter { return std::string(buffer.c_str(), buffer.length()); } - bool compareNodes (AstNode const* lhs, AstNode const* rhs) { - if (lhs->numMembers() == rhs->numMembers()) { - if (lhs->numMembers() < 2) { - return (getString(lhs) == getString(rhs)); - } - else { - for (size_t i = 0; i < lhs->numMembers(); i++) { - if (! compareNodes(lhs->getMember(i), rhs->getMember(i))) { - return false; - } - } - return true; - } - } - return false; - } - AstNode* buildInExpression (Ast* ast) { // the list of comparison values auto list = ast->createNodeList(); @@ -2338,11 +2321,6 @@ struct OrToInConverter { auto lhs = n->getMember(0); auto rhs = n->getMember(1); - if (variableNode != nullptr) { - return (compareNodes(lhs, variableNode) - || compareNodes(rhs, variableNode)); - } - if (lhs->isConstant()) { variableNode = rhs; return true; @@ -2357,8 +2335,9 @@ struct OrToInConverter { lhs->type == NODE_TYPE_INDEXED_ACCESS) { if (possibleNodes.size() == 2) { for (size_t i = 0; i < 2; i++) { - if (compareNodes(lhs, possibleNodes[i])) { + if (getString(lhs) == getString(possibleNodes[i])) { variableNode = possibleNodes[i]; + variableName = getString(variableNode); return true; } } @@ -2371,8 +2350,9 @@ struct OrToInConverter { rhs->type == NODE_TYPE_INDEXED_ACCESS) { if (possibleNodes.size() == 2) { for (size_t i = 0; i < 2; i++) { - if (compareNodes(rhs, possibleNodes[i])) { + if (getString(rhs) == getString(possibleNodes[i])) { variableNode = possibleNodes[i]; + variableName = getString(variableNode); return true; } } @@ -2405,6 +2385,8 @@ struct OrToInConverter { valueNodes.push_back(rhs); return true; } + // if canConvertExpression(lhs) and canConvertExpression(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_REFERENCE || From 095cc8a9e3238142e9e353f36d43c0586f108111 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 21:49:37 +0000 Subject: [PATCH 23/26] removed incorrect assertion --- arangod/Aql/OptimizerRules.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index efd4c83fb6..0c67d1841b 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2414,7 +2414,10 @@ int triagens::aql::replaceORwithIN (Optimizer* opt, for (auto n : nodes) { auto deps = n->getDependencies(); TRI_ASSERT(deps.size() == 1); - TRI_ASSERT(deps[0]->getType() == EN::CALCULATION); + if (deps[0]->getType() != EN::CALCULATION) { + continue; + } + auto fn = static_cast(n); auto cn = static_cast(deps[0]); From 10220c320787802b1e29744ed701a848e596f454 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 22:01:36 +0000 Subject: [PATCH 24/26] adding functions. --- arangod/Aql/OptimizerRules.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 0c67d1841b..baf7e804ce 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2331,6 +2331,16 @@ struct OrToInConverter { return true; } + if (rhs->type == NODE_TYPE_FCALL || rhs->type == NODE_TYPE_FCALL_USER) { + variableNode = lhs; + return true; + } + + if (lhs->type == NODE_TYPE_FCALL || lhs->type == NODE_TYPE_FCALL_USER) { + variableNode = rhs; + return true; + } + if (lhs->type == NODE_TYPE_ATTRIBUTE_ACCESS || lhs->type == NODE_TYPE_INDEXED_ACCESS) { if (possibleNodes.size() == 2) { From 74ef8a71a28bf48882b35957a78f6d54db4a1307 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 11:20:04 +0100 Subject: [PATCH 25/26] added to Makefile --- UnitTests/Makefile.unittests | 1 + ...ce-OR-with-IN.js => aql-optimizer-rule-replace-or-with-in.js} | 0 2 files changed, 1 insertion(+) rename js/server/tests/{aql-optimizer-rule-replace-OR-with-IN.js => aql-optimizer-rule-replace-or-with-in.js} (100%) diff --git a/UnitTests/Makefile.unittests b/UnitTests/Makefile.unittests index 2cad3ffcf0..024b102e04 100755 --- a/UnitTests/Makefile.unittests +++ b/UnitTests/Makefile.unittests @@ -554,6 +554,7 @@ SHELL_SERVER_AQL = @top_srcdir@/js/server/tests/aql-arithmetic.js \ @top_srcdir@/js/server/tests/aql-optimizer-rule-remove-unnecessary-calculations.js \ @top_srcdir@/js/server/tests/aql-optimizer-rule-remove-unnecessary-filters.js \ @top_srcdir@/js/server/tests/aql-optimizer-rule-use-index-for-sort.js \ + @top_srcdir@/js/server/tests/aql-optimizer-rule-replace-or-with-in.js \ @top_srcdir@/js/server/tests/aql-parse.js \ @top_srcdir@/js/server/tests/aql-primary-index-noncluster.js \ @top_srcdir@/js/server/tests/aql-queries-collection.js \ diff --git a/js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js b/js/server/tests/aql-optimizer-rule-replace-or-with-in.js similarity index 100% rename from js/server/tests/aql-optimizer-rule-replace-OR-with-IN.js rename to js/server/tests/aql-optimizer-rule-replace-or-with-in.js From cbb96b31c2ee219afd8e6e590c1af71868ffc4d2 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 11:20:20 +0100 Subject: [PATCH 26/26] fixed jslint warnings --- .../tests/aql-optimizer-rule-replace-or-with-in.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-replace-or-with-in.js b/js/server/tests/aql-optimizer-rule-replace-or-with-in.js index 21579d81dd..4fda57c29a 100644 --- a/js/server/tests/aql-optimizer-rule-replace-or-with-in.js +++ b/js/server/tests/aql-optimizer-rule-replace-or-with-in.js @@ -1,5 +1,5 @@ /*jshint strict: false, maxlen: 500 */ -/*global require, assertEqual, assertTrue, AQL_EXPLAIN */ +/*global require, assertEqual, assertTrue, AQL_EXPLAIN, AQL_EXECUTE */ //////////////////////////////////////////////////////////////////////////////// /// @brief tests for Ahuacatl, skiplist index queries @@ -44,7 +44,7 @@ function NewAqlReplaceORWithINTestSuite () { var isRuleUsed = function (query, params) { var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all", "+" + ruleName ] } }); assertTrue(result.plan.rules.indexOf(ruleName) !== -1, query); - var result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all" ] } }); + result = AQL_EXPLAIN(query, params, { optimizer: { rules: [ "-all" ] } }); assertTrue(result.plan.rules.indexOf(ruleName) === -1, query); }; @@ -55,11 +55,11 @@ function NewAqlReplaceORWithINTestSuite () { var executeWithRule = function (query, params) { return AQL_EXECUTE(query, params, { optimizer: { rules: [ "-all", "+" + ruleName ] } }).json; - } + }; var executeWithoutRule = function (query, params) { return AQL_EXECUTE(query, params, { optimizer: { rules: [ "-all" ] } }).json; - } + }; return { @@ -183,7 +183,7 @@ function NewAqlReplaceORWithINTestSuite () { testFires2AttributeAccesses1 : function () { var query = "LET x = {a:1,b:2} FOR v IN " + replace.name() - + " FILTER v.value == x.a || v.value == x.b SORT v.value RETURN v.value" + + " FILTER v.value == x.a || v.value == x.b SORT v.value RETURN v.value"; isRuleUsed(query, {}); @@ -196,7 +196,7 @@ function NewAqlReplaceORWithINTestSuite () { testFires2AttributeAccesses2 : function () { var query = "LET x = {a:1,b:2} FOR v IN " + replace.name() - + " FILTER x.a == v.value || v.value == x.b SORT v.value RETURN v.value" + + " FILTER x.a == v.value || v.value == x.b SORT v.value RETURN v.value"; isRuleUsed(query, {});