From f97044952ed1b3863793a2268c5a070c08a1e7f2 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Thu, 11 Sep 2014 17:59:32 +0200 Subject: [PATCH 1/3] Add de/stringifyier for transaction type enum --- arangod/VocBase/transaction.cpp | 13 +++++++++++++ arangod/VocBase/transaction.h | 11 +++++++++++ 2 files changed, 24 insertions(+) diff --git a/arangod/VocBase/transaction.cpp b/arangod/VocBase/transaction.cpp index 6aa8700bb2..7d8115c94e 100644 --- a/arangod/VocBase/transaction.cpp +++ b/arangod/VocBase/transaction.cpp @@ -64,6 +64,19 @@ /// @brief whether or not a transaction consists of a single operation //////////////////////////////////////////////////////////////////////////////// + +TRI_transaction_type_e TRI_GetTransactionTypeFromStr(const char* s) { + if (!strcmp(s, "read")) { + return TRI_TRANSACTION_READ; + } + else if (!strcmp(s, "write")) { + return TRI_TRANSACTION_WRITE; + } + else { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "invalid transaction type"); + } +} + static triagens::wal::LogfileManager* GetLogfileManager () { return triagens::wal::LogfileManager::instance(); } diff --git a/arangod/VocBase/transaction.h b/arangod/VocBase/transaction.h index b936b35164..6f1fb89513 100644 --- a/arangod/VocBase/transaction.h +++ b/arangod/VocBase/transaction.h @@ -177,6 +177,17 @@ TRI_transaction_collection_t; // ----------------------------------------------------------------------------- // --SECTION-- constructors / destructors // ----------------------------------------------------------------------------- +inline const char* TRI_TransactionTypeGetStr(TRI_transaction_type_e t) { + switch (t) { + case TRI_TRANSACTION_READ: + return "read"; + case TRI_TRANSACTION_WRITE: + return "write"; + } + return ""; +} + +TRI_transaction_type_e TRI_GetTransactionTypeFromStr(const char* s); //////////////////////////////////////////////////////////////////////////////// /// @brief create a new transaction From f25f12c098f617de9ce8c9d96daf58010769e685 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Thu, 11 Sep 2014 18:00:00 +0200 Subject: [PATCH 2/3] Move parsing of calculations out of the whole json parser, so we can properly instanciate the transaction first. --- arangod/Aql/ExecutionNode.cpp | 8 ++++---- arangod/Aql/ExecutionPlan.cpp | 37 ++++++++++++++++++++++++++++++++++- arangod/Aql/ExecutionPlan.h | 6 +++++- arangod/Aql/Query.cpp | 20 +++++++++++-------- 4 files changed, 57 insertions(+), 14 deletions(-) diff --git a/arangod/Aql/ExecutionNode.cpp b/arangod/Aql/ExecutionNode.cpp index cdc98f7113..e8e0fbf6e9 100644 --- a/arangod/Aql/ExecutionNode.cpp +++ b/arangod/Aql/ExecutionNode.cpp @@ -442,7 +442,7 @@ void SingletonNode::toJsonHelper (triagens::basics::Json& nodes, EnumerateCollectionNode::EnumerateCollectionNode (Ast* ast, basics::Json const& base) : ExecutionNode(base), _vocbase(ast->query()->vocbase()), - _collection(ast->query()->collections()->add(JsonHelper::checkAndGetStringValue(base.json(), "collection"), TRI_TRANSACTION_READ)), + _collection(ast->query()->collections()->get(JsonHelper::checkAndGetStringValue(base.json(), "collection"))), _outVariable(varFromJson(ast, base, "outVariable")) { } @@ -601,8 +601,8 @@ void IndexRangeNode::toJsonHelper (triagens::basics::Json& nodes, IndexRangeNode::IndexRangeNode (Ast* ast, basics::Json const& json) : ExecutionNode(json), _vocbase(ast->query()->vocbase()), - _collection(ast->query()->collections()->add(JsonHelper::checkAndGetStringValue(json.json(), - "collection"), TRI_TRANSACTION_READ)), + _collection(ast->query()->collections()->get(JsonHelper::checkAndGetStringValue(json.json(), + "collection"))), _outVariable(varFromJson(ast, json, "outVariable")), _ranges() { Json ranges(TRI_UNKNOWN_MEM_ZONE, JsonHelper::checkAndGetListValue(json.json(), "ranges")); @@ -1137,7 +1137,7 @@ ModificationNode::ModificationNode (Ast* ast, basics::Json const& base) : ExecutionNode(base), _vocbase(ast->query()->vocbase()), - _collection(ast->query()->collections()->add(JsonHelper::checkAndGetStringValue(base.json(), "collection"), TRI_TRANSACTION_WRITE)), + _collection(ast->query()->collections()->get(JsonHelper::checkAndGetStringValue(base.json(), "collection"))), _options(base) { TRI_ASSERT(_vocbase != nullptr); TRI_ASSERT(_collection != nullptr); diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index 98660e1e9a..7ee711ec4f 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -101,6 +101,28 @@ ExecutionPlan* ExecutionPlan::instanciateFromAst (Ast* ast) { //////////////////////////////////////////////////////////////////////////////// /// @brief create an execution plan from JSON //////////////////////////////////////////////////////////////////////////////// +void ExecutionPlan::getCollectionsFromJson(Ast *ast, + triagens::basics::Json const& json) +{ + Json jsonCollectionList = json.get("collections"); + + auto const size = jsonCollectionList.size(); + + if (! jsonCollectionList.isList()) { + THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "json collections is not list"); + } + + for (size_t i = 0; i < size; i++) { + Json oneJsonCollection = jsonCollectionList.at(i); + auto typeStr = JsonHelper::checkAndGetStringValue(oneJsonCollection.json(), "type"); + + ast->query()->collections()->add( + JsonHelper::checkAndGetStringValue(oneJsonCollection.json(), "name"), + TRI_GetTransactionTypeFromStr(JsonHelper::checkAndGetStringValue(oneJsonCollection.json(), "type").c_str())); + } + +} + ExecutionPlan* ExecutionPlan::instanciateFromJson (Ast* ast, triagens::basics::Json const& json) { @@ -121,7 +143,8 @@ ExecutionPlan* ExecutionPlan::instanciateFromJson (Ast* ast, /// @brief export to JSON, returns an AUTOFREE Json object //////////////////////////////////////////////////////////////////////////////// -triagens::basics::Json ExecutionPlan::toJson (TRI_memory_zone_t* zone, +triagens::basics::Json ExecutionPlan::toJson (Ast* ast, + TRI_memory_zone_t* zone, bool verbose) const { triagens::basics::Json result = _root->toJson(zone, verbose); @@ -133,6 +156,17 @@ triagens::basics::Json ExecutionPlan::toJson (TRI_memory_zone_t* zone, } result.set("rules", rules); + triagens::basics::Json jsonCollectionList(Json::List); + auto usedCollections = *ast->query()->collections()->collections(); + + for (auto c : usedCollections) { + Json json(Json::Array); + + jsonCollectionList(json("name", Json(c.first)) + ("type", Json(TRI_TransactionTypeGetStr(c.second->accessType)))); + + } + result.set("collections", jsonCollectionList); return result; } @@ -1180,6 +1214,7 @@ ExecutionNode* ExecutionPlan::fromJson (Ast* ast, Json const& json) { ExecutionNode* ret = nullptr; Json nodes = json.get("nodes"); + std::cout << nodes.toString() << "\n"; if (! nodes.isList()) { THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "nodes is not a list"); diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h index e5586e8a78..a2ebff5cb5 100644 --- a/arangod/Aql/ExecutionPlan.h +++ b/arangod/Aql/ExecutionPlan.h @@ -85,6 +85,9 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// /// @brief create an execution plan from JSON //////////////////////////////////////////////////////////////////////////////// + static void getCollectionsFromJson(Ast *ast, + triagens::basics::Json const& Json); + static ExecutionPlan* instanciateFromJson (Ast* ast, triagens::basics::Json const& Json); @@ -93,7 +96,8 @@ namespace triagens { /// @brief export to JSON, returns an AUTOFREE Json object //////////////////////////////////////////////////////////////////////////////// - triagens::basics::Json toJson (TRI_memory_zone_t* zone, + triagens::basics::Json toJson (Ast* ast, + TRI_memory_zone_t* zone, bool verbose) const; //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Aql/Query.cpp b/arangod/Aql/Query.cpp index 3eb14bfe38..fd0b166821 100644 --- a/arangod/Aql/Query.cpp +++ b/arangod/Aql/Query.cpp @@ -200,6 +200,7 @@ void Query::registerError (int code, } } + //////////////////////////////////////////////////////////////////////////////// /// @brief execute an AQL query //////////////////////////////////////////////////////////////////////////////// @@ -236,12 +237,7 @@ QueryResult Query::execute () { } } else { - // we have an execution plan in JSON format - plan = ExecutionPlan::instanciateFromJson(parser.ast(), _queryJson); - if (plan == nullptr) { - // oops - return QueryResult(TRI_ERROR_INTERNAL); - } + ExecutionPlan::getCollectionsFromJson(parser.ast(), _queryJson); // creating the plan may have produced some collections // we need to add them to the transaction now (otherwise the query will fail) @@ -254,6 +250,14 @@ QueryResult Query::execute () { if (res != TRI_ERROR_NO_ERROR) { return transactionError(res, trx); } + + // we have an execution plan in JSON format + plan = ExecutionPlan::instanciateFromJson(parser.ast(), _queryJson); + if (plan == nullptr) { + // oops + return QueryResult(TRI_ERROR_INTERNAL); + } + } // get enabled/disabled rules @@ -405,7 +409,7 @@ QueryResult Query::explain () { for (auto it : plans) { TRI_ASSERT(it != nullptr); - out.add(it->toJson(TRI_UNKNOWN_MEM_ZONE, verbosePlans())); + out.add(it->toJson(parser.ast(), TRI_UNKNOWN_MEM_ZONE, verbosePlans())); } result.json = out.steal(); @@ -415,7 +419,7 @@ QueryResult Query::explain () { plan = opt.stealBest(); // Now we own the best one again TRI_ASSERT(plan != nullptr); - result.json = plan->toJson(TRI_UNKNOWN_MEM_ZONE, verbosePlans()).steal(); + result.json = plan->toJson(parser.ast(), TRI_UNKNOWN_MEM_ZONE, verbosePlans()).steal(); delete plan; } From 2a6a6fd1320ed01cbab6b104f8b1db6c98616651 Mon Sep 17 00:00:00 2001 From: Willi Goesgens Date: Thu, 11 Sep 2014 18:00:47 +0200 Subject: [PATCH 3/3] work on test which inspects all optimizer passes by executing them via the AQL_EXECUTEJSON interface --- .../aql-optimizer-rule-use-index-for-sort.js | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-use-index-for-sort.js b/js/server/tests/aql-optimizer-rule-use-index-for-sort.js index 147001c9c6..3677206f56 100644 --- a/js/server/tests/aql-optimizer-rule-use-index-for-sort.js +++ b/js/server/tests/aql-optimizer-rule-use-index-for-sort.js @@ -1,5 +1,5 @@ /*jslint indent: 2, nomen: true, maxlen: 200, sloppy: true, vars: true, white: true, plusplus: true */ -/*global require, exports, assertTrue, assertEqual, AQL_EXECUTE, AQL_EXPLAIN, fail, loopmax */ +/*global require, exports, assertTrue, assertEqual, AQL_EXECUTE, AQL_EXECUTEJSON, AQL_EXPLAIN, fail, loopmax */ //////////////////////////////////////////////////////////////////////////////// /// @brief tests for optimizer rules @@ -37,6 +37,35 @@ var assertQueryError = helper.assertQueryError2; var isEqual = helper.isEqual; var findExecutionNodes = helper.findExecutionNodes; var findReferencedNodes = helper.findReferencedNodes; +//var getQueryMultiplePlansAndExecutions = helper.getQueryMultiplePlansAndExecutions; + +function getQueryMultiplePlansAndExecutions (query, bindVars) { + var plan; + var i; + var plans = []; + var allPlans = []; + var results = []; + var paramNone = { optimizer: { rules: [ "-all" ]}, verbosePlans: true}; + var paramAllPlans = { allPlans : true, verbosePlans: true}; + PY(query); + // first fetch the unmodified version + plans [0] = AQL_EXPLAIN(query, bindVars, paramNone); + // then all of the ones permuted by by the optimizer. + allPlans = AQL_EXPLAIN(query, bindVars, paramAllPlans); + + PY(allPlans.plans[0]); + + for (i=0; i < allPlans.plans.length; i++) { + plans[i+1] = {'plan':allPlans.plans[i]}; + } + // Now execute each of these variations. + for (i=0; i < plans.length; i++) { + PY(plans[i]); + results += AQL_EXECUTEJSON(plans[i].plan, {}); + } + + return {'plans': plans, 'results': results}; +} //////////////////////////////////////////////////////////////////////////////// /// @brief test suite @@ -223,7 +252,7 @@ function optimizerRuleTestSuite() { /// @brief test that rule has an effect //////////////////////////////////////////////////////////////////////////////// testRuleHasEffect : function () { - + var allresults; var queries = [ "FOR v IN " + colName + " SORT v.d DESC RETURN [v.d]",// currently only ASC supported, but we use the index range anyways. todo: this may change. @@ -242,11 +271,14 @@ function optimizerRuleTestSuite() { QResults[1] = AQL_EXECUTE(query, { }, paramIndexFromSort ).json; assertTrue(isEqual(QResults[0], QResults[1]), "Result " + i + " is Equal?"); + + allresults = getQueryMultiplePlansAndExecutions(query, {}); + PY(allresults); i++; }); }, - +/* //////////////////////////////////////////////////////////////////////////////// /// @brief test that rule has an effect, but the sort is kept in place since @@ -768,7 +800,7 @@ function optimizerRuleTestSuite() { // assertTrue(isEqual(QResults[0], QResults[1]), "Results are Equal?"); } - +*/ }; }