Merge branch 'aql2' of https://github.com/triAGENS/ArangoDB into aql2

arangod/Aql/AqlValue.cpp

@@ -116,8 +116,17 @@ AqlValue AqlValue::clone () const {
     case DOCVEC: {
       auto c = new std::vector<AqlItemBlock*>;
       c->reserve(_vector->size());
-      for (auto it = _vector->begin(); it != _vector->end(); ++it) {
+      try {
-        c->push_back((*it)->slice(0, (*it)->size()));
+        for (auto it = _vector->begin(); it != _vector->end(); ++it) {
+          c->push_back((*it)->slice(0, (*it)->size()));
+        }
+      }
+      catch (...) {
+        for (auto x : *c) {
+          delete x;
+        }
+        delete c;
+        throw;
       }
       return AqlValue(c);
     }

arangod/Aql/Query.cpp

@@ -219,7 +219,7 @@ QueryResult Query::execute () {
       int res = trx.begin();
 
       if (res != TRI_ERROR_NO_ERROR) {
-        return QueryResult(res, TRI_errno_string(res));
+        return transactionError(res, trx);
       }
 
       plan = ExecutionPlan::instanciateFromAst(parser.ast());
@@ -243,7 +243,7 @@ QueryResult Query::execute () {
       int res = trx.begin();
 
       if (res != TRI_ERROR_NO_ERROR) {
-        return QueryResult(res, TRI_errno_string(res));
+        return transactionError(res, trx);
       }
     }
     
@@ -368,7 +368,7 @@ QueryResult Query::explain () {
     int res = trx.begin();
 
     if (res != TRI_ERROR_NO_ERROR) {
-      return QueryResult(res, TRI_errno_string(res));
+        return transactionError(res, trx);
     }
 
     plan = ExecutionPlan::instanciateFromAst(parser.ast());
@@ -482,6 +482,26 @@ char* Query::registerString (std::string const& p,
 // --SECTION--                                                   private methods
 // -----------------------------------------------------------------------------
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief neatly format transaction errors to the user.
+////////////////////////////////////////////////////////////////////////////////
+
+QueryResult Query::transactionError (int errorCode, AQL_TRANSACTION_V8 const& trx)
+{
+  std::string err;
+  err += std::string(TRI_errno_string(errorCode));
+
+  auto detail = trx.getErrorData();
+  if (detail.size() > 0) {
+    err += std::string(" (") + detail + std::string(")");
+  }
+
+  err += std::string("\nwhile executing:\n") + _queryString + std::string("\n");
+
+  return QueryResult(errorCode, err);
+}
+
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief read the "optimizer.rules" section from the options
 ////////////////////////////////////////////////////////////////////////////////

arangod/Aql/Query.h

@@ -35,6 +35,8 @@
 #include "Aql/BindParameters.h"
 #include "Aql/Collections.h"
 #include "Aql/QueryResult.h"
+#include "Utils/AqlTransaction.h"
+#include "Utils/V8TransactionContext.h"
 
 struct TRI_json_s;
 struct TRI_vocbase_s;
@@ -218,6 +220,12 @@ namespace triagens {
 
         std::vector<std::string> getRulesFromOptions () const;
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief neatly format transaction errors to the user.
+////////////////////////////////////////////////////////////////////////////////
+
+        QueryResult transactionError (int errorCode, AQL_TRANSACTION_V8 const& trx);
+
 // -----------------------------------------------------------------------------
 // --SECTION--                                                 private variables
 // -----------------------------------------------------------------------------

arangod/Utils/AqlTransaction.h

@@ -70,7 +70,8 @@ namespace triagens {
           this->addHint(TRI_TRANSACTION_HINT_LOCK_ENTIRELY, false);
 
           for (auto it = collections->begin(); it != collections->end(); ++it) {
-            processCollection((*it).second);
+            if (processCollection((*it).second) != TRI_ERROR_NO_ERROR)
+              break;
           }
         }
 
@@ -91,12 +92,12 @@ namespace triagens {
 /// @brief add a collection to the transaction
 ////////////////////////////////////////////////////////////////////////////////
 
-        void processCollection (triagens::aql::Collection* collection) {
+        int processCollection (triagens::aql::Collection* collection) {
           if (ServerState::instance()->isCoordinator()) {
-            processCollectionCoordinator(collection);
+            return processCollectionCoordinator(collection);
           }
           else {
-            processCollectionNormal(collection);
+            return processCollectionNormal(collection);
           }
         }
 
@@ -104,17 +105,17 @@ namespace triagens {
 /// @brief add a coordinator collection to the transaction
 ////////////////////////////////////////////////////////////////////////////////
 
-        void processCollectionCoordinator (triagens::aql::Collection* collection) {
+        int processCollectionCoordinator (triagens::aql::Collection* collection) {
           TRI_voc_cid_t cid = this->resolver()->getCollectionIdCluster(collection->name);
 
-          this->addCollection(cid, collection->name.c_str(), collection->accessType);
+          return this->addCollection(cid, collection->name.c_str(), collection->accessType);
         }
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief add a regular collection to the transaction
 ////////////////////////////////////////////////////////////////////////////////
 
-        void processCollectionNormal (triagens::aql::Collection* collection) {
+        int processCollectionNormal (triagens::aql::Collection* collection) {
           TRI_vocbase_col_t const* col = this->resolver()->getCollectionStruct(collection->name);
           TRI_voc_cid_t cid = 0;
 
@@ -128,6 +129,8 @@ namespace triagens {
               col != nullptr) {
             collection->collection = const_cast<TRI_vocbase_col_t*>(col);
           }
+
+          return res;
         }
 
 ////////////////////////////////////////////////////////////////////////////////

js/server/tests/aql-optimizer-rule-remove-unnecessary-filters.js

@@ -0,0 +1,159 @@
+/*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 */
+////////////////////////////////////////////////////////////////////////////////
+/// @brief tests for optimizer rules
+///
+/// @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 Jan Steemann
+/// @author Copyright 2012, triAGENS GmbH, Cologne, Germany
+////////////////////////////////////////////////////////////////////////////////
+
+var jsunity = require("jsunity");
+var errors = require("internal").errors;
+var helper = require("org/arangodb/aql-helper");
+var getQueryResults = helper.getQueryResults2;
+var assertQueryError = helper.assertQueryError2;
+var isEqual = helper.isEqual;
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test suite
+////////////////////////////////////////////////////////////////////////////////
+
+function optimizerRuleTestSuite () {
+  var ruleName = "remove-unnecessary-calculations";
+  // various choices to control the optimizer: 
+  var paramNone   = { optimizer: { rules: [ "-all" ] } };
+  var paramEnabled    = { optimizer: { rules: [ "-all", "+" + ruleName ] } };
+  var paramDisabled  = { optimizer: { rules: [ "+all", "-" + ruleName ] } };
+  return {
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief set up
+////////////////////////////////////////////////////////////////////////////////
+
+    setUp : function () {
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief tear down
+////////////////////////////////////////////////////////////////////////////////
+
+    tearDown : function () {
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test that rule has no effect when explicitly disabled
+////////////////////////////////////////////////////////////////////////////////
+
+    testRuleDisabled : function () {
+      var queries = [ 
+          "LET a = SLEEP(2) RETURN 1"
+      ];
+
+      queries.forEach(function(query) {
+        var result = AQL_EXPLAIN(query, { }, paramNone);
+        assertEqual([ ], result.plan.rules);
+      });
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test that rule has no effect
+////////////////////////////////////////////////////////////////////////////////
+
+    testRuleNoEffect : function () {
+      var queries = [ 
+          "FOR a IN 1 RETURN a + 1"
+      ];
+
+      queries.forEach(function(query) {
+        var result = AQL_EXPLAIN(query, { }, paramEnabled);
+        assertEqual([ ], result.plan.rules, query);
+      });
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test that rule has an effect
+////////////////////////////////////////////////////////////////////////////////
+
+    testRuleHasEffect : function () {
+      var queries = [ 
+          "FOR i IN 1..10 LET a = 1 FILTER i == a RETURN i"
+//          "FOR i IN 1..10 LET a = i + 1 FILTER i != a RETURN i"
+      ];
+
+      queries.forEach(function(query) {
+        var result = AQL_EXPLAIN(query, { }, paramEnabled);
+          //require("internal").print(result);
+        assertEqual([ ruleName ], result.plan.rules);
+      });
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test generated plans
+////////////////////////////////////////////////////////////////////////////////
+
+
+    testPlans : function () {
+      var plans = [ 
+          ["FOR i IN 1..10 LET a = 1 FILTER i == a RETURN i", ["SingletonNode", "CalculationNode", "EnumerateListNode", "CalculationNode", "FilterNode", "ReturnNode" ]]
+          ];
+
+      plans.forEach(function(plan) {
+          var result = AQL_EXPLAIN(plan[0], { }, paramEnabled);
+          assertEqual([ ruleName ], result.plan.rules, plan[0]);
+          //require("internal").print(helper.getCompactPlan(result).map(function(node) { return node.type; }));
+          assertEqual(plan[1], helper.getCompactPlan(result).map(function(node) { return node.type; }), plan[0]);
+      });
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test results
+////////////////////////////////////////////////////////////////////////////////
+
+    testResults : function () {
+      var queries = [ 
+          ["FOR i IN 1..10 LET a = 1 FILTER i == a RETURN i", ["SingletonNode", "CalculationNode", "EnumerateListNode", "CalculationNode", "FilterNode", "ReturnNode" ]]
+      ];
+
+      queries.forEach(function(query) {
+          var resultDisabled = AQL_EXECUTE(query[0], { }, paramDisabled).json;
+          var resultEnabled  = AQL_EXECUTE(query[0], { }, paramEnabled).json;
+
+          assertTrue(isEqual(resultDisabled, resultEnabled), query[0]);
+      });
+    }
+
+  };
+}
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief executes the test suite
+////////////////////////////////////////////////////////////////////////////////
+
+jsunity.run(optimizerRuleTestSuite);
+
+return jsunity.done();
+
+// Local Variables:
+// mode: outline-minor
+// outline-regexp: "^\\(/// @brief\\|/// @addtogroup\\|// --SECTION--\\|/// @page\\|/// @}\\)"
+// End: