From d12b5baf12da6c10921ee2be5a59e4acf42dd184 Mon Sep 17 00:00:00 2001 From: James Date: Sat, 1 Nov 2014 14:30:18 +0000 Subject: [PATCH 01/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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 b285ddbfaa97d4e9e19aa178775ec77eadb5ed88 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 4 Nov 2014 17:51:54 +0100 Subject: [PATCH 17/33] updated documentation --- Documentation/Books/Users/Aql/Basics.mdpp | 22 +-- Documentation/Books/Users/Aql/Operators.mdpp | 143 +++++++++++++++---- 2 files changed, 123 insertions(+), 42 deletions(-) diff --git a/Documentation/Books/Users/Aql/Basics.mdpp b/Documentation/Books/Users/Aql/Basics.mdpp index 98ffaf08e9..ba50dce447 100644 --- a/Documentation/Books/Users/Aql/Basics.mdpp +++ b/Documentation/Books/Users/Aql/Basics.mdpp @@ -46,17 +46,17 @@ AQL supports two types of comments: !SUBSECTION Keywords On the top level, AQL offers the following operations: -- FOR: list iteration -- RETURN: results projection -- FILTER: results filtering -- SORT: result sorting -- LIMIT: result slicing -- LET: variable assignment -- COLLECT: result grouping -- INSERT: insertion of new documents -- UPDATE: (partial) update of existing documents -- REPLACE: replacement of existing documents -- REMOVE: removal of existing documents +- `FOR`: list iteration +- `RETURN`: results projection +- `FILTER`: results filtering +- `SORT`: result sorting +- `LIMIT`: result slicing +- `LET`: variable assignment +- `COLLECT`: result grouping +- `INSERT`: insertion of new documents +- `UPDATE`: (partial) update of existing documents +- `REPLACE`: replacement of existing documents +- `REMOVE`: removal of existing documents Each of the above operations can be initiated in a query by using a keyword of the same name. An AQL query can (and typically does) consist of multiple of the diff --git a/Documentation/Books/Users/Aql/Operators.mdpp b/Documentation/Books/Users/Aql/Operators.mdpp index 02b7f03f7a..bf507c248f 100644 --- a/Documentation/Books/Users/Aql/Operators.mdpp +++ b/Documentation/Books/Users/Aql/Operators.mdpp @@ -19,8 +19,7 @@ The following comparison operators are supported: - *IN* test if a value is contained in a list - *NOT IN* test if a value is not contained in a list -The *IN* and *NOT IN* operators expect the second operand to be of type list. -All other operators accept any data types for the first and second operands. +These operators accept any data types for the first and second operands. Each of the comparison operators returns a boolean value if the comparison can be evaluated and returns *true* if the comparison evaluates to true, and *false* @@ -29,49 +28,131 @@ otherwise. Some examples for comparison operations in AQL: ``` -1 > 0 -true != null -45 <= "yikes!" -65 != "65" -65 == 65 -1.23 < 1.32 -1.5 IN [ 2, 3, 1.5 ] -42 NOT IN [ 17, 40, 50 ] +1 > 0 // true +true != null // true +45 <= "yikes!" // true +65 != "65" // true +65 == 65 // true +1.23 > 1.32 // false +1.5 IN [ 2, 3, 1.5 ] // true +"foo" IN null // false +42 NOT IN [ 17, 40, 50 ] // true ``` !SUBSUBSECTION Logical operators -Logical operators combine two boolean operands in a logical operation and return -a boolean result value. - -The following logical operators are supported: +The following logical operators are supported in AQL: - *&&* logical and operator - *||* logical or operator - *!* logical not/negation operator -Some examples for logical operations in AQL: - - u.age > 15 && u.address.city != "" - true || false - !u.isInvalid - -The *&&*, *||*, and *!* operators expect their input operands to be boolean -values each. If a non-boolean operand is used, the operation will fail with an -error. In case all operands are valid, the result of each logical operator is a -boolean value. - -Both the *&&* and *||* operators use short-circuit evaluation and only evaluate -the second operand if the result of the operation cannot be determined by -checking the first operand alone. - -ArangoDB also supports the following alternative forms for the logical operators: +AQL also supports the following alternative forms for the logical operators: - *AND* logical and operator - *OR* logical or operator - *NOT* logical not/negation operator -The alternative forms are functionally equivalent to the regular operators. +The alternative forms are aliases and functionally equivalent to the regular +operators. + +The two-operand logical operators in AQL will be executed with short-circuit +evaluation. The result of the logical operators in AQL is defined as follows: + +- `lhs && rhs` will return `lhs` if it is `false` or would be `false` when converted + into a boolean. If `lhs` is `true` or would be `true` when converted to a boolean, + `rhs` will be returned. +- `lhs || rhs` will return `lhs` if it is `true` or would be `true` when converted + into a boolean. If `lhs` is `false` or would be `false` when converted to a boolean, + `rhs` will be returned. +- `! value` will return the negated value of `value` converted into a boolean + +Some examples for logical operations in AQL: + + u.age > 15 && u.address.city != "" + true || false + ! u.isInvalid + 1 || ! 0 + +Older versions of ArangoDB required the operands of all logical operators to +be boolean values and failed when non-boolean values were passed into the +operators. Additionally, the result of any logical operation always was a +boolean value. + +This behavior has changed in ArangoDB 2.3. Passing non-boolean values to a +logical operator is now allowed. Any-non boolean operands will be casted +to boolean implicity by the operator, without making the query abort. The +result of logical and and logical or operations can now have any data type and +it not necessarily a boolean value. + +For example, the following logical operations will return a boolean values: + + 25 > 1 && 42 != 7 // true + 22 IN [ 23, 42 ] || 23 NOT IN [ 22, 7 ] // true + 25 != 25 // false + +whereas the following logical operations will not return boolean values: + + 1 || 7 // 1 + null || "foo" // "foo" + null && true // null + true && 23 // 23 + + +!SUBSUBSECTION Type conversion + +In some cases, an operator needs to perform an implicit type conversion of +its operand. For example, the logical negation operator (`!`) will cast its +operand to a boolean if it is not already a boolean. + +The arithmetic operators will also cast their operands to numbers before +performing the arithmetic operation. + +The *conversion to a boolean value* works as follows: +- `null` will be converted to `false` +- boolean values remain unchanged +- all numbers unequal to zero are `true`, zero is `false` +- the empty string is `false`, all other strings are `true` +- lists (`[ ]`) and documents (`{ }`) are `true`, regardless of their contents + +The *conversion to a numeric value* works as follows: +- `null` will be converted to `0` +- `false` will be converted to `0`, true will be converted to `1` +- a numeric value remains unchanged - NaN and Infinity are converted to `null` +- string values are converted to a number if they contain a valid string representation + of a number. Any whitespace at the start or the end of the string is ignored. Strings + with any other contents are converted to `null` +- an empty list is converted to `0`, a list with one member is converted to the numeric + representation of this one list member, and lists with more members are converted + to `null` +- documents are converted to `null` + +The *conversion to a string value* works as follows: +- `null` will be converted to the string `"null"` +- `false` and `true` will be converted to the strings `"false"` and `"true"` resp. +- numbers will be converted into strings. Scientific notation may be used +- a string value remains unchanged +- an empty list will be converted into the empty string, a list with a single member + will be converted into the string representation of this one list member. A list + with more than one member will be converted into a comma-separated list of the + string representations of the list's members +- documents will be converted to the string literal `"[object Object]"` + +The *conversion to a list* works as follows: +- `null` will be converted to the empty list (`[ ]`) +- a boolean value will be converted to a single-member list with the boolean value +- a numeric value will be converted to a single-member list with the number value +- a string value will be converted to a single-member list with the string value +- a list value remains unchanged +- a numeric value will be conve +- `false` and `true` will be converted to the strings `"false"` and `"true"` resp. +- numbers will be converted into strings. Scientific notation may be used. +- strings will keep their value +- an empty list will be converted into the empty string, a list with a single member + will be converted into the string representation of this one list member. A list + with more than one member will be converted into a comma-separated list of the + string representations of the list's members. +- documents will be converted to the string literal `"[object Object]"` !SUBSUBSECTION Arithmetic operators From 9dc8ce407adcab3c2c9b67cb577ab6e598221052 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 4 Nov 2014 17:52:31 +0100 Subject: [PATCH 18/33] updated CHANGELOG --- CHANGELOG | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 03ac89d57a..9b3d06aab9 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -21,6 +21,8 @@ v2.3.0 (XXXX-XX-XX) JSON directly. If authentication is turned on, the link requires authentication, too. +* documentation updates + v2.3.0-beta1 (2014-11-01) ------------------------- From 206df8fc128603021db805691d0fa70f86fdbef4 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 4 Nov 2014 20:28:30 +0000 Subject: [PATCH 19/33] 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 20/33] 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 21/33] 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 22/33] 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 23/33] 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 24/33] 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 25/33] 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 26/33] 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 bab1762ee9ae32c038285d3fc534819047fe5622 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Wed, 5 Nov 2014 10:56:10 +0100 Subject: [PATCH 27/33] Directly route results to v8-objects instead of going via a json object when calling AQL from a V8 context --- arangod/Aql/Query.cpp | 81 +++++++++++++++++++++++++++++-- arangod/Aql/Query.h | 10 +++- arangod/Aql/QueryResult.h | 2 +- arangod/Aql/QueryResultV8.h | 84 +++++++++++++++++++++++++++++++++ arangod/V8Server/v8-vocbase.cpp | 10 ++-- 5 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 arangod/Aql/QueryResultV8.h diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index d79b0f133e..f6cbedb00d 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -583,7 +583,7 @@ QueryResult Query::execute (QueryRegistry* registry) { return res; } - triagens::basics::Json json(triagens::basics::Json::List, 16); + triagens::basics::Json jsonResult(triagens::basics::Json::List, 16); triagens::basics::Json stats; AqlItemBlock* value; @@ -592,13 +592,13 @@ QueryResult Query::execute (QueryRegistry* registry) { auto doc = value->getDocumentCollection(0); size_t const n = value->size(); // reserve space for n additional results at once - json.reserve(n); + jsonResult.reserve(n); for (size_t i = 0; i < n; ++i) { AqlValue val = value->getValue(i, 0); if (! val.isEmpty()) { - json.add(val.toJson(_trx, doc)); + jsonResult.add(val.toJson(_trx, doc)); } } delete value; @@ -614,7 +614,7 @@ QueryResult Query::execute (QueryRegistry* registry) { QueryResult result(TRI_ERROR_NO_ERROR); result.warnings = warningsToJson(); - result.json = json.steal(); + result.json = jsonResult.steal(); result.stats = stats.steal(); if (_profile != nullptr) { @@ -641,6 +641,79 @@ QueryResult Query::execute (QueryRegistry* registry) { } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief execute an AQL query +/// may only be called with an active V8 handle scope +//////////////////////////////////////////////////////////////////////////////// + +QueryResultV8 Query::executeV8 (QueryRegistry* registry) { + + // Now start the execution: + try { + QueryResultV8 res = prepare(registry); + if (res.code != TRI_ERROR_NO_ERROR) { + return res; + } + + uint32_t j = 0; + QueryResultV8 result(TRI_ERROR_NO_ERROR); + result.result = v8::Array::New(); + triagens::basics::Json stats; + + AqlItemBlock* value; + + while (nullptr != (value = _engine->getSome(1, ExecutionBlock::DefaultBatchSize))) { + auto doc = value->getDocumentCollection(0); + size_t const n = value->size(); + // reserve space for n additional results at once + /// json.reserve(n); + + for (size_t i = 0; i < n; ++i) { + AqlValue val = value->getValue(i, 0); + + if (! val.isEmpty()) { + result.result->Set(j++, val.toV8(_trx, doc)); + } + } + delete value; + } + + stats = _engine->_stats.toJson(); + + _trx->commit(); + + cleanupPlanAndEngine(TRI_ERROR_NO_ERROR); + + enterState(FINALIZATION); + + result.warnings = warningsToJson(); + result.stats = stats.steal(); + + if (_profile != nullptr) { + result.profile = _profile->toJson(TRI_UNKNOWN_MEM_ZONE); + } + + return result; + } + catch (triagens::arango::Exception const& ex) { + cleanupPlanAndEngine(ex.code()); + return QueryResultV8(ex.code(), getStateString() + ex.message()); + } + catch (std::bad_alloc const&) { + cleanupPlanAndEngine(TRI_ERROR_OUT_OF_MEMORY); + return QueryResultV8(TRI_ERROR_OUT_OF_MEMORY, getStateString() + TRI_errno_string(TRI_ERROR_OUT_OF_MEMORY)); + } + catch (std::exception const& ex) { + cleanupPlanAndEngine(TRI_ERROR_INTERNAL); + return QueryResultV8(TRI_ERROR_INTERNAL, getStateString() + ex.what()); + } + catch (...) { + cleanupPlanAndEngine(TRI_ERROR_INTERNAL); + return QueryResult(TRI_ERROR_INTERNAL, getStateString() + TRI_errno_string(TRI_ERROR_INTERNAL)); + } +} + + //////////////////////////////////////////////////////////////////////////////// /// @brief parse an AQL query //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/Query.h b/arangod/Aql/Query.h index e75b4d7553..15a4ba5f89 100644 --- a/arangod/Aql/Query.h +++ b/arangod/Aql/Query.h @@ -34,7 +34,7 @@ #include "Basics/JsonHelper.h" #include "Aql/BindParameters.h" #include "Aql/Collections.h" -#include "Aql/QueryResult.h" +#include "Aql/QueryResultV8.h" #include "Aql/types.h" #include "Utils/AqlTransaction.h" #include "Utils/V8TransactionContext.h" @@ -299,6 +299,14 @@ namespace triagens { QueryResult execute (QueryRegistry*); +//////////////////////////////////////////////////////////////////////////////// +/// @brief execute an AQL query +/// may only be called with an active V8 handle scope +//////////////////////////////////////////////////////////////////////////////// + + QueryResultV8 executeV8 (QueryRegistry*); + + //////////////////////////////////////////////////////////////////////////////// /// @brief parse an AQL query //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/QueryResult.h b/arangod/Aql/QueryResult.h index 8fde08cc7b..efb371f451 100644 --- a/arangod/Aql/QueryResult.h +++ b/arangod/Aql/QueryResult.h @@ -80,7 +80,7 @@ namespace triagens { : QueryResult(TRI_ERROR_NO_ERROR) { } - ~QueryResult () { + virtual ~QueryResult () { if (warnings != nullptr) { TRI_FreeJson(zone, warnings); } diff --git a/arangod/Aql/QueryResultV8.h b/arangod/Aql/QueryResultV8.h new file mode 100644 index 0000000000..79a1d9f815 --- /dev/null +++ b/arangod/Aql/QueryResultV8.h @@ -0,0 +1,84 @@ +//////////////////////////////////////////////////////////////////////////////// +/// @brief Aql, query results +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2014 ArangoDB GmbH, Cologne, Germany +/// Copyright 2004-2014 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 ArangoDB GmbH, Cologne, Germany +/// +/// @author Jan Steemann +/// @author Copyright 2014, ArangoDB GmbH, Cologne, Germany +/// @author Copyright 2012-2013, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +#ifndef ARANGODB_AQL_QUERY_RESULTV8_H +#define ARANGODB_AQL_QUERY_RESULTV8_H 1 + +#include "Basics/Common.h" +#include "Basics/json.h" +#include "Aql/QueryResult.h" +#include + +namespace triagens { + namespace aql { + +// ----------------------------------------------------------------------------- +// --SECTION-- struct QueryResult +// ----------------------------------------------------------------------------- + + struct QueryResultV8 : public QueryResult { + QueryResultV8& operator= (QueryResultV8 const& other) = delete; + + QueryResultV8 (QueryResultV8&& other) + : QueryResult ( (QueryResult&&) other), + result(other.result) { + } + + QueryResultV8 (QueryResult&& other) + : QueryResult((QueryResult&&)other), + result(nullptr) { + } + + QueryResultV8 (int code, + std::string const& details) + : QueryResult(code, details), + result(nullptr) { + } + + explicit QueryResultV8 (int code) + : QueryResult(code, ""), + result(nullptr) { + } + + v8::Handle result; + }; + + } +} + +#endif + +// ----------------------------------------------------------------------------- +// --SECTION-- END-OF-FILE +// ----------------------------------------------------------------------------- + +// Local Variables: +// mode: outline-minor +// outline-regexp: "/// @brief\\|/// {@inheritDoc}\\|/// @page\\|// --SECTION--\\|/// @\\}" +// End: diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index ce132ee9ff..3ef763d063 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -1200,7 +1200,7 @@ static v8::Handle JS_ExecuteAql (v8::Arguments const& argv) { TRI_v8_global_t* v8g = static_cast(v8::Isolate::GetCurrent()->GetData()); triagens::aql::Query query(v8g->_applicationV8, true, vocbase, queryString.c_str(), queryString.size(), parameters, options, triagens::aql::PART_MAIN); - auto queryResult = query.execute(static_cast(v8g->_queryRegistry)); + auto queryResult = query.executeV8(static_cast(v8g->_queryRegistry)); if (queryResult.code != TRI_ERROR_NO_ERROR) { if (queryResult.code == TRI_ERROR_REQUEST_CANCELED) { @@ -1212,12 +1212,12 @@ static v8::Handle JS_ExecuteAql (v8::Arguments const& argv) { TRI_V8_EXCEPTION_FULL(scope, queryResult.code, queryResult.details); } - if (TRI_LengthListJson(queryResult.json) <= batchSize) { + if (queryResult.result->Length() <= batchSize) { // return the array value as it is. this is a performance optimisation v8::Handle result = v8::Object::New(); - if (queryResult.json != nullptr) { - result->Set(TRI_V8_STRING("json"), TRI_ObjectJson(queryResult.json)); - } + + result->Set(TRI_V8_STRING("json"), queryResult.result); + if (queryResult.stats != nullptr) { result->Set(TRI_V8_STRING("stats"), TRI_ObjectJson(queryResult.stats)); } From 74ef8a71a28bf48882b35957a78f6d54db4a1307 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 11:20:04 +0100 Subject: [PATCH 28/33] 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 29/33] 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, {}); From 987bfa1285307fd6049a77e607081917b6a0854b Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 12:10:10 +0100 Subject: [PATCH 30/33] extended optimizer rule to detect more OR to IN transformation opportunities --- arangod/Aql/OptimizerRules.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index baf7e804ce..3cd1146879 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -2331,12 +2331,16 @@ struct OrToInConverter { return true; } - if (rhs->type == NODE_TYPE_FCALL || rhs->type == NODE_TYPE_FCALL_USER) { + if (rhs->type == NODE_TYPE_FCALL || + rhs->type == NODE_TYPE_FCALL_USER || + rhs->type == NODE_TYPE_REFERENCE) { variableNode = lhs; return true; } - if (lhs->type == NODE_TYPE_FCALL || lhs->type == NODE_TYPE_FCALL_USER) { + if (lhs->type == NODE_TYPE_FCALL || + lhs->type == NODE_TYPE_FCALL_USER || + lhs->type == NODE_TYPE_REFERENCE) { variableNode = rhs; return true; } From e5abbe92e9d7e4b0dbdb174de2e8d9a8b8325ed5 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 12:19:36 +0100 Subject: [PATCH 31/33] fixed broken cursors if cursor size > batchSize --- arangod/V8Server/v8-vocbase.cpp | 4 +--- arangod/VocBase/general-cursor.cpp | 17 ++++++++++++++++- arangod/VocBase/general-cursor.h | 8 ++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/arangod/V8Server/v8-vocbase.cpp b/arangod/V8Server/v8-vocbase.cpp index 3ef763d063..a8396aac69 100644 --- a/arangod/V8Server/v8-vocbase.cpp +++ b/arangod/V8Server/v8-vocbase.cpp @@ -1251,7 +1251,7 @@ static v8::Handle JS_ExecuteAql (v8::Arguments const& argv) { queryResult.profile = nullptr; } - TRI_general_cursor_result_t* cursorResult = TRI_CreateResultGeneralCursor(queryResult.json); + TRI_general_cursor_result_t* cursorResult = TRI_CreateResultGeneralCursor(queryResult.result); if (cursorResult == nullptr) { if (extra != nullptr) { @@ -1260,8 +1260,6 @@ static v8::Handle JS_ExecuteAql (v8::Arguments const& argv) { TRI_V8_EXCEPTION_MEMORY(scope); } - queryResult.json = nullptr; - TRI_general_cursor_t* cursor = TRI_CreateGeneralCursor(vocbase, cursorResult, doCount, static_cast(batchSize), ttl, extra); diff --git a/arangod/VocBase/general-cursor.cpp b/arangod/VocBase/general-cursor.cpp index 3f3caa20eb..35c5d0b6b9 100644 --- a/arangod/VocBase/general-cursor.cpp +++ b/arangod/VocBase/general-cursor.cpp @@ -32,6 +32,7 @@ #include "Basics/json.h" #include "Basics/logging.h" #include "Basics/vector.h" +#include "V8/v8-conv.h" // ----------------------------------------------------------------------------- // --SECTION-- cursor result @@ -48,7 +49,7 @@ static void FreeData (TRI_general_cursor_result_t* result) { TRI_json_t* json = (TRI_json_t*) result->_data; - TRI_ASSERT(json); + TRI_ASSERT(json != nullptr); TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); } @@ -94,6 +95,20 @@ TRI_general_cursor_result_t* TRI_CreateResultGeneralCursor (TRI_json_t* data) { return TRI_CreateCursorResult((void*) data, &FreeData, &GetAt, &GetLength); } +//////////////////////////////////////////////////////////////////////////////// +/// @brief create a result set +//////////////////////////////////////////////////////////////////////////////// + +TRI_general_cursor_result_t* TRI_CreateResultGeneralCursor (v8::Handle const data) { + TRI_json_t* json = TRI_ObjectToJson(data); + + if (! TRI_IsListJson(json)) { + return nullptr; + } + + return TRI_CreateCursorResult((void*) json, &FreeData, &GetAt, &GetLength); +} + // ----------------------------------------------------------------------------- // --SECTION-- cursor store // ----------------------------------------------------------------------------- diff --git a/arangod/VocBase/general-cursor.h b/arangod/VocBase/general-cursor.h index 9c989b16cc..66718b1f44 100644 --- a/arangod/VocBase/general-cursor.h +++ b/arangod/VocBase/general-cursor.h @@ -36,6 +36,8 @@ #include "VocBase/server.h" #include "VocBase/vocbase.h" +#include + // ----------------------------------------------------------------------------- // --SECTION-- forward declarations // ----------------------------------------------------------------------------- @@ -90,6 +92,12 @@ TRI_general_cursor_result_t* TRI_CreateCursorResult (void*, TRI_general_cursor_result_t* TRI_CreateResultGeneralCursor (TRI_json_t*); +//////////////////////////////////////////////////////////////////////////////// +/// @brief create a result set +//////////////////////////////////////////////////////////////////////////////// + +TRI_general_cursor_result_t* TRI_CreateResultGeneralCursor (v8::Handle const); + // ----------------------------------------------------------------------------- // --SECTION-- cursor store // ----------------------------------------------------------------------------- From 19dc758c1a6f22c92b489d682ed5a8287ed28d80 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 13:04:57 +0100 Subject: [PATCH 32/33] added test --- js/server/tests/aql-skiplist-noncluster.js | 32 ++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/js/server/tests/aql-skiplist-noncluster.js b/js/server/tests/aql-skiplist-noncluster.js index 9b91dc8f1f..54d5fa3f02 100644 --- a/js/server/tests/aql-skiplist-noncluster.js +++ b/js/server/tests/aql-skiplist-noncluster.js @@ -1147,6 +1147,38 @@ function ahuacatlSkiplistTestSuite () { assertEqual(expected, actual); assertEqual([ "SingletonNode", "IndexRangeNode", "CalculationNode", "FilterNode", "CalculationNode", "CalculationNode", "SortNode", "CalculationNode", "ReturnNode" ], explain(query)); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test reverse iteration +//////////////////////////////////////////////////////////////////////////////// + + testReverseIteration : function () { + var expectedOne = [ ], expectedTwo = [ ]; + for (var i = 5; i >= 1; --i) { + for (var j = 5; j >= 1; --j) { + expectedOne.push(i); + expectedTwo.push([ i, j ]); + } + } + + var query = "FOR a IN " + skiplist.name() + " SORT a.a DESC RETURN a.a"; + var actual = getQueryResults(query); + assertEqual(expectedOne, actual); + + // produces an empty range + query = "FOR a IN " + skiplist.name() + " FILTER a.a >= 100 SORT a.a DESC RETURN a.a"; + actual = getQueryResults(query); + assertEqual([ ], actual); + + query = "FOR a IN " + skiplist.name() + " SORT a.a DESC, a.b DESC RETURN [ a.a, a.b ]"; + actual = getQueryResults(query); + assertEqual(expectedTwo, actual); + + // produces an empty range + query = "FOR a IN " + skiplist.name() + " FILTER a.a >= 100 SORT a.a DESC, a.b DESC RETURN [ a.a, a.b ]"; + actual = getQueryResults(query); + assertEqual([ ], actual); } }; From bf309279133d1909c5b9da5f67d67e3f001879cc Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 5 Nov 2014 14:40:00 +0100 Subject: [PATCH 33/33] removed canThrow tag for various built-in functions --- arangod/Aql/AstNode.cpp | 28 +++- arangod/Aql/AstNode.h | 11 +- arangod/Aql/ExecutionNode.cpp | 4 + arangod/Aql/ExecutionNode.h | 7 +- arangod/Aql/Executor.cpp | 126 +++++++++--------- arangod/Aql/OptimizerRules.cpp | 22 ++- ...aql-optimizer-rule-move-calculations-up.js | 5 +- .../aql-optimizer-rule-move-filters-up.js | 5 +- 8 files changed, 123 insertions(+), 85 deletions(-) diff --git a/arangod/Aql/AstNode.cpp b/arangod/Aql/AstNode.cpp index b6e762448d..a657e99839 100644 --- a/arangod/Aql/AstNode.cpp +++ b/arangod/Aql/AstNode.cpp @@ -1073,12 +1073,18 @@ bool AstNode::isComparisonOperator () const { //////////////////////////////////////////////////////////////////////////////// bool AstNode::canThrow () const { + if (hasFlag(FLAG_THROWS)) { + // fast track exit + return true; + } + // check sub-nodes first size_t const n = numMembers(); for (size_t i = 0; i < n; ++i) { auto member = getMember(i); if (member->canThrow()) { // if any sub-node may throw, the whole branch may throw + setFlag(FLAG_THROWS); return true; } } @@ -1093,13 +1099,16 @@ bool AstNode::canThrow () const { // potentially throwing. This is not correct on the one hand, but on // the other hand we must not optimize or move non-deterministic functions // during optimization - // TODO: move the check for isDeterministic into a function of its - // own and check it from the optimizer rules - return func->canThrow || ! func->isDeterministic; + if (func->canThrow) { + setFlag(FLAG_THROWS); + return true; + } + return false; } if (type == NODE_TYPE_FCALL_USER) { // user functions can always throw + setFlag(FLAG_THROWS); return true; } @@ -1144,12 +1153,18 @@ bool AstNode::canRunOnDBServer () const { //////////////////////////////////////////////////////////////////////////////// bool AstNode::isDeterministic () const { + if (hasFlag(FLAG_NONDETERMINISTIC)) { + // fast track exit + return false; + } + // check sub-nodes first size_t const n = numMembers(); for (size_t i = 0; i < n; ++i) { auto member = getMember(i); if (! member->isDeterministic()) { // if any sub-node is non-deterministic, we are neither + setFlag(FLAG_NONDETERMINISTIC); return false; } } @@ -1157,11 +1172,16 @@ bool AstNode::isDeterministic () const { if (type == NODE_TYPE_FCALL) { // built-in functions may or may not be deterministic auto func = static_cast(getData()); - return func->isDeterministic; + if (! func->isDeterministic) { + setFlag(FLAG_NONDETERMINISTIC); + return false; + } + return true; } if (type == NODE_TYPE_FCALL_USER) { // user functions are always non-deterministic + setFlag(FLAG_NONDETERMINISTIC); return false; } diff --git a/arangod/Aql/AstNode.h b/arangod/Aql/AstNode.h index 81812d307c..8e3e1f1973 100644 --- a/arangod/Aql/AstNode.h +++ b/arangod/Aql/AstNode.h @@ -59,11 +59,12 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// enum AstNodeFlagType : uint8_t { - FLAG_SORTED = 1, // node is a list and its members are sorted asc. - FLAG_CONSTANT = 2, // node value is constant (i.e. not dynamic) - FLAG_DYNAMIC = 4, // node value is dynamic (i.e. not constant) - FLAG_SIMPLE = 8 // node value is simple (i.e. for use in a simple expression) - + FLAG_SORTED = 1, // node is a list and its members are sorted asc. + FLAG_CONSTANT = 2, // node value is constant (i.e. not dynamic) + FLAG_DYNAMIC = 4, // node value is dynamic (i.e. not constant) + FLAG_SIMPLE = 8, // node value is simple (i.e. for use in a simple expression) + FLAG_THROWS = 16, // node can throws an exception + FLAG_NONDETERMINISTIC = 32 // node produces non-deterministic result (e.g. function call nodes) }; //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index a119bdfd1b..fd6acb4576 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/arangod/Aql/ExecutionNode.cpp @@ -1824,6 +1824,10 @@ SortInformation SortNode::getSortInformation (ExecutionPlan* plan, // variable introduced by a calculation auto expression = static_cast(setter)->expression(); + if (! expression->isDeterministic()) { + result.isDeterministic = false; + } + if (! expression->isAttributeAccess() && ! expression->isReference()) { result.isComplex = true; diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h index 7341a402ba..caae52f4aa 100644 --- a/arangod/Aql/ExecutionNode.h +++ b/arangod/Aql/ExecutionNode.h @@ -1718,9 +1718,10 @@ namespace triagens { }; std::vector> criteria; - bool isValid = true; - bool isComplex = false; - bool canThrow = false; + bool isValid = true; + bool isDeterministic = true; + bool isComplex = false; + bool canThrow = false; Match isCoveredBy (SortInformation const& other) { if (! isValid || ! other.isValid) { diff --git a/arangod/Aql/Executor.cpp b/arangod/Aql/Executor.cpp index d2bc2ac237..91d17846fe 100644 --- a/arangod/Aql/Executor.cpp +++ b/arangod/Aql/Executor.cpp @@ -104,63 +104,63 @@ std::unordered_map const Executor::FunctionNames{ { "TO_LIST", Function("TO_LIST", "AQL_TO_LIST", ".", true, false, true) }, // string functions - { "CONCAT", Function("CONCAT", "AQL_CONCAT", "sz,sz|+", true, true, true) }, - { "CONCAT_SEPARATOR", Function("CONCAT_SEPARATOR", "AQL_CONCAT_SEPARATOR", "s,sz,sz|+", true, true, true) }, - { "CHAR_LENGTH", Function("CHAR_LENGTH", "AQL_CHAR_LENGTH", "s", true, true, true) }, - { "LOWER", Function("LOWER", "AQL_LOWER", "s", true, true, true) }, - { "UPPER", Function("UPPER", "AQL_UPPER", "s", true, true, true) }, - { "SUBSTRING", Function("SUBSTRING", "AQL_SUBSTRING", "s,n|n", true, true, true) }, - { "CONTAINS", Function("CONTAINS", "AQL_CONTAINS", "s,s|b", true, true, true) }, - { "LIKE", Function("LIKE", "AQL_LIKE", "s,r|b", true, true, true) }, - { "LEFT", Function("LEFT", "AQL_LEFT", "s,n", true, true, true) }, - { "RIGHT", Function("RIGHT", "AQL_RIGHT", "s,n", true, true, true) }, - { "TRIM", Function("TRIM", "AQL_TRIM", "s|n", true, true, true) }, - { "FIND_FIRST", Function("FIND_FIRST", "AQL_FIND_FIRST", "s,s|zn,zn", true, true, true) }, - { "FIND_LAST", Function("FIND_LAST", "AQL_FIND_LAST", "s,s|zn,zn", true, true, true) }, + { "CONCAT", Function("CONCAT", "AQL_CONCAT", "sz,sz|+", true, false, true) }, + { "CONCAT_SEPARATOR", Function("CONCAT_SEPARATOR", "AQL_CONCAT_SEPARATOR", "s,sz,sz|+", true, false, true) }, + { "CHAR_LENGTH", Function("CHAR_LENGTH", "AQL_CHAR_LENGTH", "s", true, false, true) }, + { "LOWER", Function("LOWER", "AQL_LOWER", "s", true, false, true) }, + { "UPPER", Function("UPPER", "AQL_UPPER", "s", true, false, true) }, + { "SUBSTRING", Function("SUBSTRING", "AQL_SUBSTRING", "s,n|n", true, false, true) }, + { "CONTAINS", Function("CONTAINS", "AQL_CONTAINS", "s,s|b", true, false, true) }, + { "LIKE", Function("LIKE", "AQL_LIKE", "s,r|b", true, false, true) }, + { "LEFT", Function("LEFT", "AQL_LEFT", "s,n", true, false, true) }, + { "RIGHT", Function("RIGHT", "AQL_RIGHT", "s,n", true, false, true) }, + { "TRIM", Function("TRIM", "AQL_TRIM", "s|n", true, false, true) }, + { "FIND_FIRST", Function("FIND_FIRST", "AQL_FIND_FIRST", "s,s|zn,zn", true, false, true) }, + { "FIND_LAST", Function("FIND_LAST", "AQL_FIND_LAST", "s,s|zn,zn", true, false, true) }, // numeric functions - { "FLOOR", Function("FLOOR", "AQL_FLOOR", "n", true, true, true) }, - { "CEIL", Function("CEIL", "AQL_CEIL", "n", true, true, true) }, - { "ROUND", Function("ROUND", "AQL_ROUND", "n", true, true, true) }, - { "ABS", Function("ABS", "AQL_ABS", "n", true, true, true) }, + { "FLOOR", Function("FLOOR", "AQL_FLOOR", "n", true, false, true) }, + { "CEIL", Function("CEIL", "AQL_CEIL", "n", true, false, true) }, + { "ROUND", Function("ROUND", "AQL_ROUND", "n", true, false, true) }, + { "ABS", Function("ABS", "AQL_ABS", "n", true, false, true) }, { "RAND", Function("RAND", "AQL_RAND", "", false, false, true) }, - { "SQRT", Function("SQRT", "AQL_SQRT", "n", true, true, true) }, + { "SQRT", Function("SQRT", "AQL_SQRT", "n", true, false, true) }, // list functions - { "RANGE", Function("RANGE", "AQL_RANGE", "n,n|n", true, true, true) }, - { "UNION", Function("UNION", "AQL_UNION", "l,l|+",true, true, true) }, - { "UNION_DISTINCT", Function("UNION_DISTINCT", "AQL_UNION_DISTINCT", "l,l|+", true, true, true) }, - { "MINUS", Function("MINUS", "AQL_MINUS", "l,l|+", true, true, true) }, - { "INTERSECTION", Function("INTERSECTION", "AQL_INTERSECTION", "l,l|+", true, true, true) }, - { "FLATTEN", Function("FLATTEN", "AQL_FLATTEN", "l|n", true, true, true) }, - { "LENGTH", Function("LENGTH", "AQL_LENGTH", "las", true, true, true) }, - { "MIN", Function("MIN", "AQL_MIN", "l", true, true, true) }, - { "MAX", Function("MAX", "AQL_MAX", "l", true, true, true) }, - { "SUM", Function("SUM", "AQL_SUM", "l", true, true, true) }, - { "MEDIAN", Function("MEDIAN", "AQL_MEDIAN", "l", true, true, true) }, - { "AVERAGE", Function("AVERAGE", "AQL_AVERAGE", "l", true, true, true) }, - { "VARIANCE_SAMPLE", Function("VARIANCE_SAMPLE", "AQL_VARIANCE_SAMPLE", "l", true, true, true) }, - { "VARIANCE_POPULATION", Function("VARIANCE_POPULATION", "AQL_VARIANCE_POPULATION", "l", true, true, true) }, - { "STDDEV_SAMPLE", Function("STDDEV_SAMPLE", "AQL_STDDEV_SAMPLE", "l", true, true, true) }, - { "STDDEV_POPULATION", Function("STDDEV_POPULATION", "AQL_STDDEV_POPULATION", "l", true, true, true) }, - { "UNIQUE", Function("UNIQUE", "AQL_UNIQUE", "l", true, true, true) }, - { "SLICE", Function("SLICE", "AQL_SLICE", "l,n|n", true, true, true) }, - { "REVERSE", Function("REVERSE", "AQL_REVERSE", "ls", true, true, true) }, // note: REVERSE() can be applied on strings, too - { "FIRST", Function("FIRST", "AQL_FIRST", "l", true, true, true) }, - { "LAST", Function("LAST", "AQL_LAST", "l", true, true, true) }, - { "NTH", Function("NTH", "AQL_NTH", "l,n", true, true, true) }, - { "POSITION", Function("POSITION", "AQL_POSITION", "l,.|b", true, true, true) }, + { "RANGE", Function("RANGE", "AQL_RANGE", "n,n|n", true, false, true) }, + { "UNION", Function("UNION", "AQL_UNION", "l,l|+",true, false, true) }, + { "UNION_DISTINCT", Function("UNION_DISTINCT", "AQL_UNION_DISTINCT", "l,l|+", true, false, true) }, + { "MINUS", Function("MINUS", "AQL_MINUS", "l,l|+", true, false, true) }, + { "INTERSECTION", Function("INTERSECTION", "AQL_INTERSECTION", "l,l|+", true, false, true) }, + { "FLATTEN", Function("FLATTEN", "AQL_FLATTEN", "l|n", true, false, true) }, + { "LENGTH", Function("LENGTH", "AQL_LENGTH", "las", true, false, true) }, + { "MIN", Function("MIN", "AQL_MIN", "l", true, false, true) }, + { "MAX", Function("MAX", "AQL_MAX", "l", true, false, true) }, + { "SUM", Function("SUM", "AQL_SUM", "l", true, false, true) }, + { "MEDIAN", Function("MEDIAN", "AQL_MEDIAN", "l", true, false, true) }, + { "AVERAGE", Function("AVERAGE", "AQL_AVERAGE", "l", true, false, true) }, + { "VARIANCE_SAMPLE", Function("VARIANCE_SAMPLE", "AQL_VARIANCE_SAMPLE", "l", true, false, true) }, + { "VARIANCE_POPULATION", Function("VARIANCE_POPULATION", "AQL_VARIANCE_POPULATION", "l", true, false, true) }, + { "STDDEV_SAMPLE", Function("STDDEV_SAMPLE", "AQL_STDDEV_SAMPLE", "l", true, false, true) }, + { "STDDEV_POPULATION", Function("STDDEV_POPULATION", "AQL_STDDEV_POPULATION", "l", true, false, true) }, + { "UNIQUE", Function("UNIQUE", "AQL_UNIQUE", "l", true, false, true) }, + { "SLICE", Function("SLICE", "AQL_SLICE", "l,n|n", true, false, true) }, + { "REVERSE", Function("REVERSE", "AQL_REVERSE", "ls", true, false, true) }, // note: REVERSE() can be applied on strings, too + { "FIRST", Function("FIRST", "AQL_FIRST", "l", true, false, true) }, + { "LAST", Function("LAST", "AQL_LAST", "l", true, false, true) }, + { "NTH", Function("NTH", "AQL_NTH", "l,n", true, false, true) }, + { "POSITION", Function("POSITION", "AQL_POSITION", "l,.|b", true, false, true) }, // document functions - { "HAS", Function("HAS", "AQL_HAS", "az,s", true, true, true) }, - { "ATTRIBUTES", Function("ATTRIBUTES", "AQL_ATTRIBUTES", "a|b,b", true, true, true) }, - { "MERGE", Function("MERGE", "AQL_MERGE", "a,a|+", true, true, true) }, - { "MERGE_RECURSIVE", Function("MERGE_RECURSIVE", "AQL_MERGE_RECURSIVE", "a,a|+", true, true, true) }, + { "HAS", Function("HAS", "AQL_HAS", "az,s", true, false, true) }, + { "ATTRIBUTES", Function("ATTRIBUTES", "AQL_ATTRIBUTES", "a|b,b", true, false, true) }, + { "MERGE", Function("MERGE", "AQL_MERGE", "a,a|+", true, false, true) }, + { "MERGE_RECURSIVE", Function("MERGE_RECURSIVE", "AQL_MERGE_RECURSIVE", "a,a|+", true, false, true) }, { "DOCUMENT", Function("DOCUMENT", "AQL_DOCUMENT", "h.|.", false, true, false) }, - { "MATCHES", Function("MATCHES", "AQL_MATCHES", ".,l|b", true, true, true) }, - { "UNSET", Function("UNSET", "AQL_UNSET", "a,sl|+", true, true, true) }, - { "KEEP", Function("KEEP", "AQL_KEEP", "a,sl|+", true, true, true) }, - { "TRANSLATE", Function("TRANSLATE", "AQL_TRANSLATE", ".,a|.", true, true, true) }, + { "MATCHES", Function("MATCHES", "AQL_MATCHES", ".,l|b", true, false, true) }, + { "UNSET", Function("UNSET", "AQL_UNSET", "a,sl|+", true, false, true) }, + { "KEEP", Function("KEEP", "AQL_KEEP", "a,sl|+", true, false, true) }, + { "TRANSLATE", Function("TRANSLATE", "AQL_TRANSLATE", ".,a|.", true, false, true) }, // geo functions { "NEAR", Function("NEAR", "AQL_NEAR", "h,n,n|nz,s", false, true, false) }, @@ -199,26 +199,26 @@ std::unordered_map const Executor::FunctionNames{ // date functions { "DATE_NOW", Function("DATE_NOW", "AQL_DATE_NOW", "", false, false, true) }, - { "DATE_TIMESTAMP", Function("DATE_TIMESTAMP", "AQL_DATE_TIMESTAMP", "ns|ns,ns,ns,ns,ns,ns", true, true, true) }, - { "DATE_ISO8601", Function("DATE_ISO8601", "AQL_DATE_ISO8601", "ns|ns,ns,ns,ns,ns,ns", true, true, true) }, - { "DATE_DAYOFWEEK", Function("DATE_DAYOFWEEK", "AQL_DATE_DAYOFWEEK", "ns", true, true, true) }, - { "DATE_YEAR", Function("DATE_YEAR", "AQL_DATE_YEAR", "ns", true, true, true) }, - { "DATE_MONTH", Function("DATE_MONTH", "AQL_DATE_MONTH", "ns", true, true, true) }, - { "DATE_DAY", Function("DATE_DAY", "AQL_DATE_DAY", "ns", true, true, true) }, - { "DATE_HOUR", Function("DATE_HOUR", "AQL_DATE_HOUR", "ns", true, true, true) }, - { "DATE_MINUTE", Function("DATE_MINUTE", "AQL_DATE_MINUTE", "ns", true, true, true) }, - { "DATE_SECOND", Function("DATE_SECOND", "AQL_DATE_SECOND", "ns", true, true, true) }, - { "DATE_MILLISECOND", Function("DATE_MILLISECOND", "AQL_DATE_MILLISECOND", "ns", true, true, true) }, + { "DATE_TIMESTAMP", Function("DATE_TIMESTAMP", "AQL_DATE_TIMESTAMP", "ns|ns,ns,ns,ns,ns,ns", true, false, true) }, + { "DATE_ISO8601", Function("DATE_ISO8601", "AQL_DATE_ISO8601", "ns|ns,ns,ns,ns,ns,ns", true, false, true) }, + { "DATE_DAYOFWEEK", Function("DATE_DAYOFWEEK", "AQL_DATE_DAYOFWEEK", "ns", true, false, true) }, + { "DATE_YEAR", Function("DATE_YEAR", "AQL_DATE_YEAR", "ns", true, false, true) }, + { "DATE_MONTH", Function("DATE_MONTH", "AQL_DATE_MONTH", "ns", true, false, true) }, + { "DATE_DAY", Function("DATE_DAY", "AQL_DATE_DAY", "ns", true, false, true) }, + { "DATE_HOUR", Function("DATE_HOUR", "AQL_DATE_HOUR", "ns", true, false, true) }, + { "DATE_MINUTE", Function("DATE_MINUTE", "AQL_DATE_MINUTE", "ns", true, false, true) }, + { "DATE_SECOND", Function("DATE_SECOND", "AQL_DATE_SECOND", "ns", true, false, true) }, + { "DATE_MILLISECOND", Function("DATE_MILLISECOND", "AQL_DATE_MILLISECOND", "ns", true, false, true) }, // misc functions { "FAIL", Function("FAIL", "AQL_FAIL", "|s", false, true, true) }, - { "PASSTHRU", Function("PASSTHRU", "AQL_PASSTHRU", ".", false, true, true) }, - { "SLEEP", Function("SLEEP", "AQL_SLEEP", "n", false, false, true) }, + { "PASSTHRU", Function("PASSTHRU", "AQL_PASSTHRU", ".", false, false, true) }, + { "SLEEP", Function("SLEEP", "AQL_SLEEP", "n", false, true, true) }, { "COLLECTIONS", Function("COLLECTIONS", "AQL_COLLECTIONS", "", false, true, false) }, - { "NOT_NULL", Function("NOT_NULL", "AQL_NOT_NULL", ".|+", true, true, true) }, + { "NOT_NULL", Function("NOT_NULL", "AQL_NOT_NULL", ".|+", true, false, true) }, { "FIRST_LIST", Function("FIRST_LIST", "AQL_FIRST_LIST", ".|+", true, false, true) }, { "FIRST_DOCUMENT", Function("FIRST_DOCUMENT", "AQL_FIRST_DOCUMENT", ".|+", true, false, true) }, - { "PARSE_IDENTIFIER", Function("PARSE_IDENTIFIER", "AQL_PARSE_IDENTIFIER", ".", true, true, true) }, + { "PARSE_IDENTIFIER", Function("PARSE_IDENTIFIER", "AQL_PARSE_IDENTIFIER", ".", true, false, true) }, { "SKIPLIST", Function("SKIPLIST", "AQL_SKIPLIST", "h,a|n,n", false, true, false) }, { "CURRENT_USER", Function("CURRENT_USER", "AQL_CURRENT_USER", "", false, false, false) }, { "CURRENT_DATABASE", Function("CURRENT_DATABASE", "AQL_CURRENT_DATABASE", "", false, false, false) } diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 3cd1146879..bd1639b2f2 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -96,8 +96,8 @@ int triagens::aql::removeRedundantSorts (Optimizer* opt, if (nodesRelyingOnSort == 0) { // a sort directly followed by another sort: now remove one of them - if (other.canThrow) { - // if the sort can throw, we must not remove it + if (other.canThrow || ! other.isDeterministic) { + // if the sort can throw or is non-deterministic, we must not remove it break; } @@ -267,8 +267,9 @@ int triagens::aql::moveCalculationsUpRule (Optimizer* opt, for (auto n : nodes) { auto nn = static_cast(n); - if (nn->expression()->canThrow()) { - // we will only move expressions up that cannot throw + if (nn->expression()->canThrow() || + ! nn->expression()->isDeterministic()) { + // we will only move expressions up that cannot throw and that are deterministic continue; } @@ -367,6 +368,14 @@ int triagens::aql::moveFiltersUpRule (Optimizer* opt, break; } + if (current->getType() == EN::CALCULATION) { + // must not move a filter beyond a node with a non-deterministic result + auto calculation = static_cast(current); + if (! calculation->expression()->isDeterministic()) { + break; + } + } + bool found = false; auto&& varsSet = current->getVariablesSetHere(); @@ -650,9 +659,8 @@ int triagens::aql::removeUnnecessaryCalculationsRule (Optimizer* opt, if (n->getType() == EN::CALCULATION) { auto nn = static_cast(n); - if (nn->canThrow() || - ! nn->expression()->isDeterministic()) { - // If this node can throw or is non-deterministic, we must not optimize it away! + if (nn->canThrow()) { + // If this node can throw, we must not optimize it away! continue; } } diff --git a/js/server/tests/aql-optimizer-rule-move-calculations-up.js b/js/server/tests/aql-optimizer-rule-move-calculations-up.js index 40db9e7a0c..36c0f70efe 100644 --- a/js/server/tests/aql-optimizer-rule-move-calculations-up.js +++ b/js/server/tests/aql-optimizer-rule-move-calculations-up.js @@ -104,13 +104,16 @@ function optimizerRuleTestSuite () { "FOR i IN 1..10 LET a = 1 FILTER i == 1 RETURN a", "FOR i IN 1..10 FILTER i == 1 LET a = 1 RETURN i", "FOR i IN 1..10 LET a = 25 + 7 RETURN i", + "FOR i IN 1..10 LET a = MIN([ i, 1 ]) LET b = i + 1 RETURN [ a, b ]", + "FOR i IN 1..10 LET a = RAND() LET b = 25 + i RETURN i", + "FOR i IN 1..10 LET a = SLEEP(0.1) LET b = i + 1 RETURN b", "FOR i IN 1..10 FOR j IN 1..10 LET a = i + 2 RETURN i", "FOR i IN 1..10 FILTER i == 7 LET a = i COLLECT x = i RETURN x" ]; queries.forEach(function(query) { var result = AQL_EXPLAIN(query, { }, paramEnabled); - assertEqual([ ruleName ], result.plan.rules); + assertEqual([ ruleName ], result.plan.rules, query); }); }, diff --git a/js/server/tests/aql-optimizer-rule-move-filters-up.js b/js/server/tests/aql-optimizer-rule-move-filters-up.js index 1d38a9d840..ef23e0952e 100644 --- a/js/server/tests/aql-optimizer-rule-move-filters-up.js +++ b/js/server/tests/aql-optimizer-rule-move-filters-up.js @@ -88,7 +88,7 @@ function optimizerRuleTestSuite () { "FOR i IN 1..10 LET a = i LET b = i FILTER a == b RETURN i", "FOR i IN 1..10 FOR j IN 1..10 FILTER i > j RETURN i", "FOR i IN 1..10 LET a = 2 * i FILTER a == 1 RETURN i", - "FOR i IN 1..10 LIMIT 1 FILTER i == 1 RETURN i" + "FOR i IN 1..10 LET a = SLEEP(1) FILTER i == 1 RETURN i" // SLEEP is non-deterministic ]; queries.forEach(function(query) { @@ -106,7 +106,8 @@ function optimizerRuleTestSuite () { "FOR i IN 1..10 FOR j IN 1..10 FILTER i > 1 RETURN i", "FOR i IN 1..10 LET x = (FOR j IN [i] RETURN j) FILTER i > 1 RETURN i", "FOR i IN 1..10 FOR l IN 1..10 LET a = 2 * i FILTER i == 1 RETURN a", - "FOR i IN 1..10 FOR l IN 1..10 LET a = 2 * i FILTER i == 1 LIMIT 1 RETURN a" + "FOR i IN 1..10 FOR l IN 1..10 LET a = 2 * i FILTER i == 1 LIMIT 1 RETURN a", + "FOR i IN 1..10 FOR l IN 1..10 LET a = MIN([1, l]) FILTER i == 1 LIMIT 1 RETURN a", ]; queries.forEach(function(query) {