simplifications

arangod/Aql/Query.cpp

@@ -463,152 +463,119 @@ void Query::registerWarning(int code, char const* details) {
 /// execute calls it internally. The purpose of this separate method is
 /// to be able to only prepare a query from VelocyPack and then store it in the
 /// QueryRegistry.
-QueryResult Query::prepare(QueryRegistry* registry) {
+void Query::prepare(QueryRegistry* registry) {
   LOG_TOPIC(DEBUG, Logger::QUERIES) << TRI_microtime() - _startTime << " "
                                     << "Query::prepare"
                                     << " this: " << (uintptr_t) this;
   TRI_ASSERT(registry != nullptr);
 
-  try {
+  init();
-    init();
+  enterState(PARSING);
-    enterState(PARSING);
 
-    auto parser = std::make_unique<Parser>(this);
+  auto parser = std::make_unique<Parser>(this);
-    std::unique_ptr<ExecutionPlan> plan;
+  std::unique_ptr<ExecutionPlan> plan;
 
-    if (_queryString != nullptr) {
+  if (_queryString != nullptr) {
-      parser->parse(false);
+    parser->parse(false);
-      // put in bind parameters
+    // put in bind parameters
-      parser->ast()->injectBindParameters(_bindParameters);
+    parser->ast()->injectBindParameters(_bindParameters);
-    }
-
-    _isModificationQuery = parser->isModificationQuery();
-
-    // create the transaction object, but do not start it yet
-    AqlTransaction* trx = new AqlTransaction(
-        createTransactionContext(), _collections.collections(),
-        _part == PART_MAIN);
-    _trx = trx;
-
-    try {
-      bool planRegisters;
-      // As soon as we start du instantiate the plan we have to clean it
-      // up before killing the unique_ptr
-      if (_queryString != nullptr) {
-        // we have an AST
-        // optimize the ast
-        enterState(AST_OPTIMIZATION);
-
-        parser->ast()->validateAndOptimize();
-        
-        enterState(LOADING_COLLECTIONS);
-      
-        int res = trx->begin();
-
-        if (res != TRI_ERROR_NO_ERROR) {
-          return transactionError(res);
-        }
-
-        enterState(PLAN_INSTANTIATION);
-        plan.reset(ExecutionPlan::instantiateFromAst(parser->ast()));
-
-        if (plan.get() == nullptr) {
-          // oops
-          return QueryResult(TRI_ERROR_INTERNAL,
-                             "failed to create query execution engine");
-        }
-
-        // Run the query optimizer:
-        enterState(PLAN_OPTIMIZATION);
-        arangodb::aql::Optimizer opt(maxNumberOfPlans());
-        // get enabled/disabled rules
-        opt.createPlans(plan.release(), getRulesFromOptions(),
-                        inspectSimplePlans());
-        // Now plan and all derived plans belong to the optimizer
-        plan.reset(opt.stealBest());  // Now we own the best one again
-        planRegisters = true;
-      } else {  // no queryString, we are instantiating from _queryBuilder
-        enterState(PARSING);
-
-        VPackSlice const querySlice = _queryBuilder->slice();
-        ExecutionPlan::getCollectionsFromVelocyPack(parser->ast(), querySlice);
-
-        parser->ast()->variables()->fromVelocyPack(querySlice);
-        // creating the plan may have produced some collections
-        // we need to add them to the transaction now (otherwise the query will
-        // fail)
-        
-        enterState(LOADING_COLLECTIONS);
-        
-        int res = trx->addCollections(*_collections.collections());
-
-        if (res == TRI_ERROR_NO_ERROR) {
-          res = trx->begin();
-        }
-
-        if (res != TRI_ERROR_NO_ERROR) {
-          return transactionError(res);
-        }
-        
-        enterState(PLAN_INSTANTIATION);
-
-        // we have an execution plan in VelocyPack format
-        plan.reset(ExecutionPlan::instantiateFromVelocyPack(
-            parser->ast(), _queryBuilder->slice()));
-        if (plan.get() == nullptr) {
-          // oops
-          return QueryResult(TRI_ERROR_INTERNAL);
-        }
-
-        planRegisters = false;
-      }
-
-      TRI_ASSERT(plan.get() != nullptr);
-
-      // varsUsedLater and varsValid are unordered_sets and so their orders
-      // are not the same in the serialized and deserialized plans
-
-      // return the V8 context
-      exitContext();
-
-      enterState(EXECUTION);
-      ExecutionEngine* engine(ExecutionEngine::instantiateFromPlan(
-          registry, this, plan.get(), planRegisters));
-
-      // If all went well so far, then we keep _plan and _trx and
-      // return:
-      _plan = std::move(plan);
-      _engine = engine;
-      return QueryResult();
-    } catch (arangodb::basics::Exception const& ex) {
-      cleanupPlanAndEngine(ex.code());
-      return QueryResult(ex.code(), ex.message() + getStateString());
-    } catch (std::bad_alloc const&) {
-      cleanupPlanAndEngine(TRI_ERROR_OUT_OF_MEMORY);
-      return QueryResult(
-          TRI_ERROR_OUT_OF_MEMORY,
-          TRI_errno_string(TRI_ERROR_OUT_OF_MEMORY) + getStateString());
-    } catch (std::exception const& ex) {
-      cleanupPlanAndEngine(TRI_ERROR_INTERNAL);
-      return QueryResult(TRI_ERROR_INTERNAL, ex.what() + getStateString());
-    } catch (...) {
-      cleanupPlanAndEngine(TRI_ERROR_INTERNAL);
-      return QueryResult(TRI_ERROR_INTERNAL,
-                         TRI_errno_string(TRI_ERROR_INTERNAL) + getStateString());
-    }
-  } catch (arangodb::basics::Exception const& ex) {
-    return QueryResult(ex.code(), ex.message() + getStateString());
-  } catch (std::bad_alloc const&) {
-    return QueryResult(
-        TRI_ERROR_OUT_OF_MEMORY,
-        TRI_errno_string(TRI_ERROR_OUT_OF_MEMORY) + getStateString());
-  } catch (std::exception const& ex) {
-    return QueryResult(TRI_ERROR_INTERNAL, ex.what() + getStateString());
-  } catch (...) {
-    return QueryResult(TRI_ERROR_INTERNAL,
-                       TRI_errno_string(TRI_ERROR_INTERNAL) + getStateString());
   }
 
+  _isModificationQuery = parser->isModificationQuery();
+
+  // create the transaction object, but do not start it yet
+  AqlTransaction* trx = new AqlTransaction(
+      createTransactionContext(), _collections.collections(),
+      _part == PART_MAIN);
+  _trx = trx;
+
+  bool planRegisters;
+  // As soon as we start du instantiate the plan we have to clean it
+  // up before killing the unique_ptr
+  if (_queryString != nullptr) {
+    // we have an AST
+    // optimize the ast
+    enterState(AST_OPTIMIZATION);
+
+    parser->ast()->validateAndOptimize();
+    
+    enterState(LOADING_COLLECTIONS);
+  
+    int res = trx->begin();
+
+    if (res != TRI_ERROR_NO_ERROR) {
+      THROW_ARANGO_EXCEPTION_MESSAGE(res, buildErrorMessage(res));
+    }
+
+    enterState(PLAN_INSTANTIATION);
+    plan.reset(ExecutionPlan::instantiateFromAst(parser->ast()));
+
+    if (plan.get() == nullptr) {
+      // oops
+      THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "failed to create query execution engine");
+    }
+
+    // Run the query optimizer:
+    enterState(PLAN_OPTIMIZATION);
+    arangodb::aql::Optimizer opt(maxNumberOfPlans());
+    // get enabled/disabled rules
+    opt.createPlans(plan.release(), getRulesFromOptions(),
+                    inspectSimplePlans());
+    // Now plan and all derived plans belong to the optimizer
+    plan.reset(opt.stealBest());  // Now we own the best one again
+    planRegisters = true;
+  } else {  // no queryString, we are instantiating from _queryBuilder
+    enterState(PARSING);
+
+    VPackSlice const querySlice = _queryBuilder->slice();
+    ExecutionPlan::getCollectionsFromVelocyPack(parser->ast(), querySlice);
+
+    parser->ast()->variables()->fromVelocyPack(querySlice);
+    // creating the plan may have produced some collections
+    // we need to add them to the transaction now (otherwise the query will
+    // fail)
+    
+    enterState(LOADING_COLLECTIONS);
+    
+    int res = trx->addCollections(*_collections.collections());
+
+    if (res == TRI_ERROR_NO_ERROR) {
+      res = trx->begin();
+    }
+
+    if (res != TRI_ERROR_NO_ERROR) {
+      THROW_ARANGO_EXCEPTION_MESSAGE(res, buildErrorMessage(res));
+    }
+    
+    enterState(PLAN_INSTANTIATION);
+
+    // we have an execution plan in VelocyPack format
+    plan.reset(ExecutionPlan::instantiateFromVelocyPack(
+        parser->ast(), _queryBuilder->slice()));
+    if (plan.get() == nullptr) {
+      // oops
+      THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_INTERNAL, "could not create plan from vpack");
+    }
+
+    planRegisters = false;
+  }
+
+  TRI_ASSERT(plan.get() != nullptr);
+
+  // varsUsedLater and varsValid are unordered_sets and so their orders
+  // are not the same in the serialized and deserialized plans
+
+  // return the V8 context
+  exitContext();
+
+  enterState(EXECUTION);
+  ExecutionEngine* engine(ExecutionEngine::instantiateFromPlan(
+      registry, this, plan.get(), planRegisters));
+
+  // If all went well so far, then we keep _plan and _trx and
+  // return:
+  _plan = std::move(plan);
+  _engine = engine;
 }
 
 /// @brief execute an AQL query
@@ -635,7 +602,7 @@ QueryResult Query::execute(QueryRegistry* registry) {
 
       if (cacheEntry != nullptr) {
         // got a result from the query cache
-        QueryResult res(TRI_ERROR_NO_ERROR);
+        QueryResult res;
         // we don't have yet a transaction when we're here, so let's create
         // a mimimal context to build the result
         res.context = std::make_shared<StandaloneTransactionContext>(_vocbase);
@@ -648,11 +615,8 @@ QueryResult Query::execute(QueryRegistry* registry) {
       }
     }
 
-    QueryResult result = prepare(registry);
+    // will throw if it fails
-
+    prepare(registry);
-    if (result.code != TRI_ERROR_NO_ERROR) {
-      return result;
-    }
 
     if (_queryString == nullptr) {
       // we don't have query string... now pass query id to WorkMonitor
@@ -742,6 +706,7 @@ QueryResult Query::execute(QueryRegistry* registry) {
 
     _trx->commit();
    
+    QueryResult result; 
     result.context = _trx->transactionContext();
 
     _engine->_stats.setExecutionTime(TRI_microtime() - _startTime);
@@ -811,26 +776,23 @@ QueryResultV8 Query::executeV8(v8::Isolate* isolate, QueryRegistry* registry) {
 
       if (cacheEntry != nullptr) {
         // got a result from the query cache
-        QueryResultV8 res(TRI_ERROR_NO_ERROR);
+        QueryResultV8 result;
         // we don't have yet a transaction when we're here, so let's create
         // a mimimal context to build the result
-        res.context = std::make_shared<StandaloneTransactionContext>(_vocbase);
+        result.context = std::make_shared<StandaloneTransactionContext>(_vocbase);
 
         v8::Handle<v8::Value> values =
             TRI_VPackToV8(isolate, cacheEntry->_queryResult->slice(),
-                          res.context->getVPackOptions());
+                          result.context->getVPackOptions());
         TRI_ASSERT(values->IsArray());
-        res.result = v8::Handle<v8::Array>::Cast(values);
+        result.result = v8::Handle<v8::Array>::Cast(values);
-        res.cached = true;
+        result.cached = true;
-        return res;
+        return result;
       }
     }
 
-    QueryResultV8 result = prepare(registry);
+    // will throw if it fails
-
+    prepare(registry);
-    if (result.code != TRI_ERROR_NO_ERROR) {
-      return result;
-    }
 
     work.reset(new AqlWorkStack(_vocbase, _id, _queryString, _queryLength));
 
@@ -841,6 +803,7 @@ QueryResultV8 Query::executeV8(v8::Isolate* isolate, QueryRegistry* registry) {
       useQueryCache = false;
     }
 
+    QueryResultV8 result;
     result.result = v8::Array::New(isolate);
 
     TRI_ASSERT(_engine != nullptr);
@@ -1014,7 +977,7 @@ QueryResult Query::explain() {
     int res = _trx->begin();
 
     if (res != TRI_ERROR_NO_ERROR) {
-      return transactionError(res);
+      THROW_ARANGO_EXCEPTION_MESSAGE(res, buildErrorMessage(res));
     }
 
     enterState(PLAN_INSTANTIATION);
@@ -1299,16 +1262,17 @@ bool Query::canUseQueryCache() const {
   return false;
 }
 
-/// @brief neatly format transaction error to the user.
+/// @brief neatly format exception messages for the users
-QueryResult Query::transactionError(int errorCode) const {
+std::string Query::buildErrorMessage(int errorCode) const {
   std::string err(TRI_errno_string(errorCode));
 
   if (_queryString != nullptr && verboseErrors()) {
-    err +=
+    err += "\nwhile executing:\n";
-        std::string("\nwhile executing:\n") + _queryString + std::string("\n");
+    err.append(_queryString, _queryLength);
+    err += "\n";
   }
 
-  return QueryResult(errorCode, err);
+  return err;
 }
 
 /// @brief read the "optimizer.inspectSimplePlans" section from the options

arangod/Aql/Query.h

@@ -242,7 +242,7 @@ class Query {
   /// execute calls it internally. The purpose of this separate method is
   /// to be able to only prepare a query from VelocyPack and then store it in the
   /// QueryRegistry.
-  QueryResult prepare(QueryRegistry*);
+  void prepare(QueryRegistry*);
 
   /// @brief execute an AQL query
   QueryResult execute(QueryRegistry*);
@@ -370,8 +370,8 @@ class Query {
   /// @brief read the "optimizer.rules" section from the options
   std::vector<std::string> getRulesFromOptions() const;
 
-  /// @brief neatly format transaction errors to the user.
+  /// @brief neatly format exception messages for the users
-  QueryResult transactionError(int errorCode) const;
+  std::string buildErrorMessage(int errorCode) const;
 
   /// @brief enter a new state
   void enterState(ExecutionState);

arangod/Aql/QueryResultV8.h

@@ -44,6 +44,7 @@ struct QueryResultV8 : public QueryResult {
   QueryResultV8(int code, std::string const& details)
       : QueryResult(code, details), result() {}
 
+  QueryResultV8() : QueryResult(TRI_ERROR_NO_ERROR) {}
   explicit QueryResultV8(int code) : QueryResult(code, ""), result() {}
 
   v8::Handle<v8::Array> result;

arangod/Aql/RestAqlHandler.cpp

@@ -95,14 +95,18 @@ void RestAqlHandler::createQueryFromVelocyPack() {
       VelocyPackHelper::getStringValue(querySlice, "part", "");
 
   auto planBuilder = std::make_shared<VPackBuilder>(VPackBuilder::clone(plan));
-  auto query = new Query(false, _vocbase, planBuilder, options,
+  auto query = std::make_unique<Query>(false, _vocbase, planBuilder, options,
-                         (part == "main" ? PART_MAIN : PART_DEPENDENT));
+                                      (part == "main" ? PART_MAIN : PART_DEPENDENT));
-  QueryResult res = query->prepare(_queryRegistry);
+  
-  if (res.code != TRI_ERROR_NO_ERROR) {
+  try {
-    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query: " << res.details;
+    query->prepare(_queryRegistry);
-    generateError(rest::ResponseCode::BAD,
+  } catch (std::exception const& ex) {
-                  TRI_ERROR_QUERY_BAD_JSON_PLAN, res.details);
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query: " << ex.what();
-    delete query;
+    generateError(rest::ResponseCode::BAD, TRI_ERROR_QUERY_BAD_JSON_PLAN, ex.what());
+    return;
+  } catch (...) {
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query";
+    generateError(rest::ResponseCode::BAD, TRI_ERROR_QUERY_BAD_JSON_PLAN);
     return;
   }
 
@@ -116,14 +120,15 @@ void RestAqlHandler::createQueryFromVelocyPack() {
   }
 
   _qId = TRI_NewTickServer();
+  auto transactionContext = query->trx()->transactionContext().get();
 
   try {
-    _queryRegistry->insert(_qId, query, ttl);
+    _queryRegistry->insert(_qId, query.get(), ttl);
+    query.release();
   } catch (...) {
     LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "could not keep query in registry";
     generateError(rest::ResponseCode::BAD, TRI_ERROR_INTERNAL,
                   "could not keep query in registry");
-    delete query;
     return;
   }
 
@@ -139,8 +144,7 @@ void RestAqlHandler::createQueryFromVelocyPack() {
     return;
   }
 
-  sendResponse(rest::ResponseCode::ACCEPTED, answerBody.slice(),
+  sendResponse(rest::ResponseCode::ACCEPTED, answerBody.slice(), transactionContext);
-               query->trx()->transactionContext().get());
 }
 
 // POST method for /_api/aql/parse (internal)
@@ -306,15 +310,19 @@ void RestAqlHandler::createQueryFromString() {
   auto options = std::make_shared<VPackBuilder>(
       VPackBuilder::clone(querySlice.get("options")));
 
-  auto query = new Query(false, _vocbase, queryString.c_str(),
+  auto query = std::make_unique<Query>(false, _vocbase, queryString.c_str(),
                          queryString.size(), bindVars, options,
                          (part == "main" ? PART_MAIN : PART_DEPENDENT));
-  QueryResult res = query->prepare(_queryRegistry);
+  
-  if (res.code != TRI_ERROR_NO_ERROR) {
+  try {
-    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query: " << res.details;
+    query->prepare(_queryRegistry);
-    generateError(rest::ResponseCode::BAD,
+  } catch (std::exception const& ex) {
-                  TRI_ERROR_QUERY_BAD_JSON_PLAN, res.details);
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query: " << ex.what();
-    delete query;
+    generateError(rest::ResponseCode::BAD, TRI_ERROR_QUERY_BAD_JSON_PLAN, ex.what());
+    return;
+  } catch (...) {
+    LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "failed to instantiate the query";
+    generateError(rest::ResponseCode::BAD, TRI_ERROR_QUERY_BAD_JSON_PLAN);
     return;
   }
 
@@ -327,15 +335,16 @@ void RestAqlHandler::createQueryFromString() {
     ttl = arangodb::basics::StringUtils::doubleDecimal(ttlstring);
   }
 
+  auto transactionContext = query->trx()->transactionContext().get();
   _qId = TRI_NewTickServer();
 
   try {
-    _queryRegistry->insert(_qId, query, ttl);
+    _queryRegistry->insert(_qId, query.get(), ttl);
+    query.release();
   } catch (...) {
     LOG_TOPIC(ERR, arangodb::Logger::FIXME) << "could not keep query in registry";
     generateError(rest::ResponseCode::BAD, TRI_ERROR_INTERNAL,
                   "could not keep query in registry");
-    delete query;
     return;
   }
 
@@ -351,8 +360,7 @@ void RestAqlHandler::createQueryFromString() {
     return;
   }
 
-  sendResponse(rest::ResponseCode::ACCEPTED, answerBody.slice(),
+  sendResponse(rest::ResponseCode::ACCEPTED, answerBody.slice(), transactionContext);
-               query->trx()->transactionContext().get());
 }
 
 // PUT method for /_api/aql/<operation>/<queryId>, (internal)