From 6ddaf08ccbd1ab43b9e0e263133a8425738a3471 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 09:47:28 +0100 Subject: [PATCH 01/17] fixed potential memleak --- arangod/Aql/ExecutionBlock.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arangod/Aql/ExecutionBlock.cpp b/arangod/Aql/ExecutionBlock.cpp index b676e5cb78..895f5a4b67 100644 --- a/arangod/Aql/ExecutionBlock.cpp +++ b/arangod/Aql/ExecutionBlock.cpp @@ -876,6 +876,10 @@ IndexRangeBlock::~IndexRangeBlock () { if (_freeCondition && _condition != nullptr) { delete _condition; } + + if (_skiplistIterator != nullptr) { + TRI_FreeSkiplistIterator(_skiplistIterator); + } } int IndexRangeBlock::initialize () { @@ -1708,6 +1712,7 @@ void IndexRangeBlock::readSkiplistIndex (size_t atMost) { } catch (...) { TRI_FreeSkiplistIterator(_skiplistIterator); + _skiplistIterator = nullptr; throw; } LEAVE_BLOCK; From 368530dd5a02cfc0867ceb9aa4de2d82f26e273f Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 9 Dec 2014 09:59:07 +0100 Subject: [PATCH 02/17] Fix number of plan cap. --- arangod/Aql/OptimizerRules.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 3e4e90f516..4098c0deb1 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1328,7 +1328,7 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, while (i < changes.size()) { possibilities *= changes[i].second.size(); i++; - if (possibilities * nrPlans > 20) { + if (possibilities + nrPlans > 30) { break; } } From d0b041842a272ea1631c119566f26eb064d9a779 Mon Sep 17 00:00:00 2001 From: Tomas Bosak Date: Tue, 9 Dec 2014 10:50:06 +0100 Subject: [PATCH 03/17] Update document API description. Include information about _key attribute which is also returned along with _id and _rev for replace, update and delete document operations. --- arangod/RestHandler/RestDocumentHandler.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/arangod/RestHandler/RestDocumentHandler.cpp b/arangod/RestHandler/RestDocumentHandler.cpp index 29656de01d..1d2606605c 100644 --- a/arangod/RestHandler/RestDocumentHandler.cpp +++ b/arangod/RestHandler/RestDocumentHandler.cpp @@ -1014,8 +1014,9 @@ bool RestDocumentHandler::checkDocument () { /// of *true*. /// /// The body of the response contains a JSON object with the information about -/// the handle and the revision. The attribute *_id* contains the known -/// *document-handle* of the updated document, the attribute *_rev* +/// the handle and the revision. The attribute *_id* contains the known +/// *document-handle* of the updated document, *_key* contains the key which +/// uniquely identifies a document in a given collection, and the attribute *_rev* /// contains the new document revision. /// /// If the document does not exist, then a *HTTP 404* is returned and the @@ -1249,7 +1250,8 @@ bool RestDocumentHandler::replaceDocument () { /// /// The body of the response contains a JSON object with the information about /// the handle and the revision. The attribute *_id* contains the known -/// *document-handle* of the updated document, the attribute *_rev* +/// *document-handle* of the updated document, *_key* contains the key which +/// uniquely identifies a document in a given collection, and the attribute *_rev* /// contains the new document revision. /// /// If the document does not exist, then a *HTTP 404* is returned and the @@ -1679,9 +1681,10 @@ bool RestDocumentHandler::modifyDocumentCoordinator ( /// /// @RESTDESCRIPTION /// The body of the response contains a JSON object with the information about -/// the handle and the revision. The attribute *_id* contains the known -/// *document-handle* of the deleted document, the attribute *_rev* -/// contains the document revision. +/// the handle and the revision. The attribute *_id* contains the known +/// *document-handle* of the deleted document, *_key* contains the key which +/// uniquely identifies a document in a given collection, and the attribute *_rev* +/// contains the new document revision. /// /// If the *waitForSync* parameter is not specified or set to /// *false*, then the collection's default *waitForSync* behavior is From 797b6dbf75354f23fd76b181f96fda93cba4780c Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 11:01:58 +0100 Subject: [PATCH 04/17] added index tests --- js/server/tests/aql-optimizer-indexes.js | 233 +++++++++++++++++++++++ 1 file changed, 233 insertions(+) diff --git a/js/server/tests/aql-optimizer-indexes.js b/js/server/tests/aql-optimizer-indexes.js index 227e8fefd1..6c5231cd7b 100644 --- a/js/server/tests/aql-optimizer-indexes.js +++ b/js/server/tests/aql-optimizer-indexes.js @@ -250,6 +250,239 @@ function optimizerIndexesTestSuite () { assertEqual(0, results.stats.scannedFull); assertNotEqual(0, results.stats.scannedIndex); assertEqual([ [ [ 'test1' ], [ 'test2' ], [ 'test3' ], [ 'test4' ], [ 'test5' ], [ 'test6' ], [ 'test7' ], [ 'test8' ], [ 'test9' ], [ 'test10' ] ] ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testMultipleSubqueriesMultipleIndexes : function () { + c.ensureHashIndex("value"); // now we have a hash and a skiplist index + var query = "LET a = (FOR x IN " + c.name() + " FILTER x.value == 1 RETURN x._key) " + + "LET b = (FOR x IN " + c.name() + " FILTER x.value == 2 RETURN x._key) " + + "LET c = (FOR x IN " + c.name() + " FILTER x.value == 3 RETURN x._key) " + + "LET d = (FOR x IN " + c.name() + " FILTER x.value == 4 RETURN x._key) " + + "LET e = (FOR x IN " + c.name() + " FILTER x.value == 5 RETURN x._key) " + + "LET f = (FOR x IN " + c.name() + " FILTER x.value == 6 RETURN x._key) " + + "LET g = (FOR x IN " + c.name() + " FILTER x.value == 7 RETURN x._key) " + + "LET h = (FOR x IN " + c.name() + " FILTER x.value == 8 RETURN x._key) " + + "LET i = (FOR x IN " + c.name() + " FILTER x.value == 9 RETURN x._key) " + + "LET j = (FOR x IN " + c.name() + " FILTER x.value == 10 RETURN x._key) " + + "RETURN [ a, b, c, d, e, f, g, h, i, j ]"; + + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var walker = function (nodes, func) { + nodes.forEach(function(node) { + if (node.type === "SubqueryNode") { + walker(node.subquery.nodes, func); + } + func(node); + }); + }; + + var indexNodes = 0, collectionNodes = 0; + walker(plan.nodes, function (node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + if (indexNodes <= 5) { + assertEqual("hash", node.index.type); + } + else { + assertEqual("skiplist", node.index.type); + } + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(10, indexNodes); + assertEqual(32, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ [ [ 'test1' ], [ 'test2' ], [ 'test3' ], [ 'test4' ], [ 'test5' ], [ 'test6' ], [ 'test7' ], [ 'test8' ], [ 'test9' ], [ 'test10' ] ] ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testMultipleSubqueriesHashIndexes : function () { + c.dropIndex(c.getIndexes()[1]); // drop skiplist index + c.ensureHashIndex("value"); + var query = "LET a = (FOR x IN " + c.name() + " FILTER x.value == 1 RETURN x._key) " + + "LET b = (FOR x IN " + c.name() + " FILTER x.value == 2 RETURN x._key) " + + "LET c = (FOR x IN " + c.name() + " FILTER x.value == 3 RETURN x._key) " + + "LET d = (FOR x IN " + c.name() + " FILTER x.value == 4 RETURN x._key) " + + "LET e = (FOR x IN " + c.name() + " FILTER x.value == 5 RETURN x._key) " + + "LET f = (FOR x IN " + c.name() + " FILTER x.value == 6 RETURN x._key) " + + "LET g = (FOR x IN " + c.name() + " FILTER x.value == 7 RETURN x._key) " + + "LET h = (FOR x IN " + c.name() + " FILTER x.value == 8 RETURN x._key) " + + "LET i = (FOR x IN " + c.name() + " FILTER x.value == 9 RETURN x._key) " + + "LET j = (FOR x IN " + c.name() + " FILTER x.value == 10 RETURN x._key) " + + "RETURN [ a, b, c, d, e, f, g, h, i, j ]"; + + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var walker = function (nodes, func) { + nodes.forEach(function(node) { + if (node.type === "SubqueryNode") { + walker(node.subquery.nodes, func); + } + func(node); + }); + }; + + var indexNodes = 0, collectionNodes = 0; + walker(plan.nodes, function (node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + assertEqual("hash", node.index.type); + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(10, indexNodes); + assertEqual(1, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ [ [ 'test1' ], [ 'test2' ], [ 'test3' ], [ 'test4' ], [ 'test5' ], [ 'test6' ], [ 'test7' ], [ 'test8' ], [ 'test9' ], [ 'test10' ] ] ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testJoinMultipleIndexes : function () { + c.ensureHashIndex("value"); // now we have a hash and a skiplist index + var query = "FOR i IN " + c.name() + " FILTER i.value < 10 FOR j IN " + c.name() + " FILTER j.value == i.value RETURN j._key"; + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var collectionNodes = 0, indexNodes = 0; + plan.nodes.forEach(function(node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + if (indexNodes === 1) { + // skiplist must be used for the first FOR + assertEqual("skiplist", node.index.type); + assertEqual("i", node.outVariable.name); + } + else { + // second FOR should use a hash index + assertEqual("hash", node.index.type); + assertEqual("j", node.outVariable.name); + } + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(2, indexNodes); + assertEqual(4, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ 'test0', 'test1', 'test2', 'test3', 'test4', 'test5', 'test6', 'test7', 'test8', 'test9' ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testJoinRangesMultipleIndexes : function () { + c.ensureHashIndex("value"); // now we have a hash and a skiplist index + var query = "FOR i IN " + c.name() + " FILTER i.value < 5 FOR j IN " + c.name() + " FILTER j.value < i.value RETURN j._key"; + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var collectionNodes = 0, indexNodes = 0; + plan.nodes.forEach(function(node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + // skiplist must be used for both FORs + assertEqual("skiplist", node.index.type); + if (indexNodes === 1) { + assertEqual("i", node.outVariable.name); + } + else { + assertEqual("j", node.outVariable.name); + } + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(2, indexNodes); + assertEqual(2, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ 'test0', 'test0', 'test1', 'test0', 'test1', 'test2', 'test0', 'test1', 'test2', 'test3' ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testTripleJoin : function () { + c.ensureHashIndex("value"); // now we have a hash and a skiplist index + var query = "FOR i IN " + c.name() + " FILTER i.value == 4 FOR j IN " + c.name() + " FILTER j.value == i.value FOR k IN " + c.name() + " FILTER k.value < j.value RETURN k._key"; + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var collectionNodes = 0, indexNodes = 0; + plan.nodes.forEach(function(node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + if (indexNodes === 1) { + assertEqual("hash", node.index.type); + assertEqual("i", node.outVariable.name); + } + else if (indexNodes === 2) { + assertEqual("hash", node.index.type); + assertEqual("j", node.outVariable.name); + } + else { + assertEqual("skiplist", node.index.type); + assertEqual("k", node.outVariable.name); + } + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(3, indexNodes); + assertEqual(12, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ 'test0', 'test1', 'test2', 'test3' ], results.json); } }; From 07214a41b7b73a0c217889e21faa4054ea216bc6 Mon Sep 17 00:00:00 2001 From: Patrick Huber Date: Tue, 9 Dec 2014 14:12:36 +0100 Subject: [PATCH 05/17] reference ansible role on ansible galaxy --- Documentation/Books/Users/Installing/Linux.mdpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Documentation/Books/Users/Installing/Linux.mdpp b/Documentation/Books/Users/Installing/Linux.mdpp index ebbcc1b2ff..78b6ed8864 100644 --- a/Documentation/Books/Users/Installing/Linux.mdpp +++ b/Documentation/Books/Users/Installing/Linux.mdpp @@ -70,3 +70,10 @@ A Chef recipe is available from jbianquetti at: https://github.com/jbianquetti/chef-arangodb +!SECTION Using ansible + +An [Ansible](http://ansible.com) role is available trough [Ansible-Galaxy](https://galaxy.ansible.com) + +* Role on Ansible-Galaxy: https://galaxy.ansible.com/list#/roles/2344 +* Source on Github: https://github.com/stackmagic/ansible-arangodb + From a617dcffc59f2343ce9c158e0b4367ebd8d321e5 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 14:50:28 +0100 Subject: [PATCH 06/17] added tests --- js/server/tests/aql-optimizer-indexes.js | 57 +++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/js/server/tests/aql-optimizer-indexes.js b/js/server/tests/aql-optimizer-indexes.js index 6c5231cd7b..4d7e3ced05 100644 --- a/js/server/tests/aql-optimizer-indexes.js +++ b/js/server/tests/aql-optimizer-indexes.js @@ -328,7 +328,6 @@ function optimizerIndexesTestSuite () { "LET j = (FOR x IN " + c.name() + " FILTER x.value == 10 RETURN x._key) " + "RETURN [ a, b, c, d, e, f, g, h, i, j ]"; - var explain = AQL_EXPLAIN(query); var plan = explain.plan; @@ -483,6 +482,62 @@ function optimizerIndexesTestSuite () { assertEqual(0, results.stats.scannedFull); assertNotEqual(0, results.stats.scannedIndex); assertEqual([ 'test0', 'test1', 'test2', 'test3' ], results.json); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testSubqueryMadness : function () { + c.ensureHashIndex("value"); // now we have a hash and a skiplist index + var query = "LET a = (FOR x IN " + c.name() + " FILTER x.value == 1 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET b = (FOR x IN " + c.name() + " FILTER x.value == 2 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET c = (FOR x IN " + c.name() + " FILTER x.value == 3 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET d = (FOR x IN " + c.name() + " FILTER x.value == 4 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET e = (FOR x IN " + c.name() + " FILTER x.value == 5 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET f = (FOR x IN " + c.name() + " FILTER x.value == 6 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET g = (FOR x IN " + c.name() + " FILTER x.value == 7 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET h = (FOR x IN " + c.name() + " FILTER x.value == 8 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET i = (FOR x IN " + c.name() + " FILTER x.value == 9 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "LET j = (FOR x IN " + c.name() + " FILTER x.value == 10 FOR y IN " + c.name() + " FILTER y.value == x.value RETURN x._key) " + + "RETURN [ a, b, c, d, e, f, g, h, i, j ]"; + + var explain = AQL_EXPLAIN(query); + var plan = explain.plan; + + var walker = function (nodes, func) { + nodes.forEach(function(node) { + if (node.type === "SubqueryNode") { + walker(node.subquery.nodes, func); + } + func(node); + }); + }; + + var indexNodes = 0, collectionNodes = 0; + walker(plan.nodes, function (node) { + if (node.type === "IndexRangeNode") { + ++indexNodes; + if (indexNodes < 5) { + assertEqual("hash", node.index.type); + } + else { + assertEqual("skiplist", node.index.type); + } + } + else if (node.type === "EnumerateCollectionNode") { + ++collectionNodes; + } + }); + + assertEqual(0, collectionNodes); + assertEqual(20, indexNodes); + assertEqual(36, explain.stats.plansCreated); + + var results = AQL_EXECUTE(query); + assertEqual(0, results.stats.scannedFull); + assertNotEqual(0, results.stats.scannedIndex); + assertEqual([ [ [ 'test1' ], [ 'test2' ], [ 'test3' ], [ 'test4' ], [ 'test5' ], [ 'test6' ], [ 'test7' ], [ 'test8' ], [ 'test9' ], [ 'test10' ] ] ], results.json); } }; From d638c1c0c9aecf9a7a202ee037f1a49f8729a4a7 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 15:18:53 +0100 Subject: [PATCH 07/17] fixed choicung --- arangod/Aql/OptimizerRules.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 4098c0deb1..a51f9c68e1 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1359,8 +1359,10 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, plan->replaceNode(plan->getNodeById(id), v[choice]); modified = true; // Free the other nodes, if they are there: - for (size_t k = 1; k < v.size(); k++) { - delete v[k]; + for (size_t k = 0; k < v.size(); k++) { + if (k != choice) { + delete v[k]; + } } v.clear(); // take the new node away from changes such that // cleanupChanges does not touch it From be421d926fd3d62a16bb2e0e41ddf2e314e2afb5 Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 9 Dec 2014 15:40:22 +0100 Subject: [PATCH 08/17] Add new test for use-index-range rule and no rule explosion. --- UnitTests/Makefile.unittests | 1 + .../aql-optimizer-rule-use-index-range.js | 237 ++++++++++++++++++ 2 files changed, 238 insertions(+) create mode 100644 js/server/tests/aql-optimizer-rule-use-index-range.js diff --git a/UnitTests/Makefile.unittests b/UnitTests/Makefile.unittests index 578045a35d..fb21703958 100755 --- a/UnitTests/Makefile.unittests +++ b/UnitTests/Makefile.unittests @@ -569,6 +569,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-replace-or-with-in.js \ + @top_srcdir@/js/server/tests/aql-optimizer-rule-use-index-range.js \ @top_srcdir@/js/server/tests/aql-optimizer-rule-use-index-for-sort.js \ @top_srcdir@/js/server/tests/aql-optimizer-stats-noncluster.js \ @top_srcdir@/js/server/tests/aql-parse.js \ diff --git a/js/server/tests/aql-optimizer-rule-use-index-range.js b/js/server/tests/aql-optimizer-rule-use-index-range.js new file mode 100644 index 0000000000..177f0db66f --- /dev/null +++ b/js/server/tests/aql-optimizer-rule-use-index-range.js @@ -0,0 +1,237 @@ +/*jshint strict: false, maxlen: 500 */ +/*global require, assertEqual, assertTrue, AQL_EXPLAIN, AQL_EXECUTE */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tests for optimizer rule use index-range +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2014-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 triAGENS GmbH, Cologne, Germany +/// +/// @author Max Neunhoeffer +/// @author Copyright 2014, triAGENS GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +var internal = require("internal"); +var jsunity = require("jsunity"); +var helper = require("org/arangodb/aql-helper"); +var removeAlwaysOnClusterRules = helper.removeAlwaysOnClusterRules; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test suite +//////////////////////////////////////////////////////////////////////////////// + +function optimizerRuleUseIndexRangeTester () { + var suiteName = "TestUseIndexRangeRule"; + var ruleName = "use-index-range"; + var collBaseName = "UTUseIndexRange"; + var collNames = ["NoInd", "SkipInd", "HashInd", "BothInd"]; + var collNoInd; + var collSkipInd; + var collHashInd; + var collBothInd; + + // various choices to control the optimizer: + var paramNone = { optimizer: { rules: [ "-all" ] } }; + var paramEnabled = { optimizer: { rules: [ "-all", "+" + ruleName ] } }; + var paramDisabled = { optimizer: { rules: [ "+all", "-" + ruleName ] } }; + var paramAll = { optimizer: { rules: [ "+all" ] } }; + var paramEnabledAllPlans = { optimizer: { rules: [ "-all", "+" + ruleName ]}, + allPlans: true }; + var paramAllAllPlans = { optimizer: { rules: [ "+all" ]}, allPlans: true }; + + return { + +//////////////////////////////////////////////////////////////////////////////// +/// @brief set up +//////////////////////////////////////////////////////////////////////////////// + + setUp : function () { + var n = collNames.map(function(x) { return collBaseName + x; }); + var colls = []; + for (var i = 0; i < n.length; i++) { + var collName = n[i]; + internal.db._drop(collName); + coll = internal.db._create(collName); + for (var j = 0; j < 10; j++) { + coll.insert({"a":j, "s":"s"+j}); + } + colls.push(coll); + } + collNoInd = colls[0]; + collSkipInd = colls[1]; + collSkipInd.ensureSkiplist("a"); + collHashInd = colls[2]; + collHashInd.ensureHashIndex("a"); + collBothInd = colls[3]; + collBothInd.ensureHashIndex("a"); + collBothInd.ensureSkiplist("a"); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tear down +//////////////////////////////////////////////////////////////////////////////// + + tearDown : function () { + var n = collNames.map(function(x) { return collBaseName + x; }); + for (var i = 0; i < n.length; i++) { + internal.db._drop(n[i]); + } + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that rule has no effect when explicitly disabled +//////////////////////////////////////////////////////////////////////////////// + + testRuleDisabled : function () { + var queries = [ + "FOR i IN UTUseIndexRangeNoInd FILTER i.a >= 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a >= 2 RETURN i", + "FOR i IN UTUseIndexRangeHashInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a == 2 RETURN i", + ]; + + queries.forEach(function(query) { + var result = AQL_EXPLAIN(query, { }, paramNone); + assertEqual([ ], removeAlwaysOnClusterRules(result.plan.rules), query); + }); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that rule has no effect +//////////////////////////////////////////////////////////////////////////////// + + testRuleNoEffect : function () { + var queries = [ + "FOR i IN UTUseIndexRangeNoInd FILTER i.a >= 2 RETURN i", + "FOR i IN UTUseIndexRangeNoInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeHashInd FILTER i.a >= 2 RETURN i" + ]; + + queries.forEach(function(query) { + var result = AQL_EXPLAIN(query, { }, paramEnabled); + assertEqual([ ], removeAlwaysOnClusterRules(result.plan.rules), query); + }); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that rule has an effect +//////////////////////////////////////////////////////////////////////////////// + + testRuleHasEffect : function () { + var queries = [ + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a >= 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 && i.a > 0 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 && i.a > 3 RETURN i", + "FOR i IN UTUseIndexRangeHashInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a <= 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a > 2 RETURN i" + ]; + + queries.forEach(function(query) { + var result = AQL_EXPLAIN(query, { }, paramEnabled); + assertEqual([ ruleName ], removeAlwaysOnClusterRules(result.plan.rules), + query); + }); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that rule has an effect +//////////////////////////////////////////////////////////////////////////////// + + testRuleHasEffectWithAll : function () { + var queries = [ + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a >= 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 && i.a > 0 RETURN i", + "FOR i IN UTUseIndexRangeSkipInd FILTER i.a < 2 && i.a > 3 RETURN i", + "FOR i IN UTUseIndexRangeHashInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a == 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a <= 2 RETURN i", + "FOR i IN UTUseIndexRangeBothInd FILTER i.a > 2 RETURN i" + ]; + + queries.forEach(function(query) { + var result = AQL_EXPLAIN(query, { }, paramAll); + assertTrue(result.plan.rules.indexOf(ruleName) >= 0, query); + }); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that plan explosion does not happen +//////////////////////////////////////////////////////////////////////////////// + + testRuleHasNoPlanExplosion : function () { + var query = + "LET A = (FOR a IN UTUseIndexRangeBothInd FILTER a.a == 2 RETURN a) "+ + "LET B = (FOR b IN UTUseIndexRangeBothInd FILTER b.a == 2 RETURN b) "+ + "LET C = (FOR c IN UTUseIndexRangeBothInd FILTER c.a == 2 RETURN c) "+ + "LET D = (FOR d IN UTUseIndexRangeBothInd FILTER d.a == 2 RETURN d) "+ + "LET E = (FOR e IN UTUseIndexRangeBothInd FILTER e.a == 2 RETURN e) "+ + "LET F = (FOR f IN UTUseIndexRangeBothInd FILTER f.a == 2 RETURN f) "+ + "LET G = (FOR g IN UTUseIndexRangeBothInd FILTER g.a == 2 RETURN g) "+ + "LET H = (FOR h IN UTUseIndexRangeBothInd FILTER h.a == 2 RETURN h) "+ + "LET I = (FOR i IN UTUseIndexRangeBothInd FILTER i.a == 2 RETURN i) "+ + "LET J = (FOR j IN UTUseIndexRangeBothInd FILTER j.a == 2 RETURN j) "+ + "FOR k IN UTUseIndexRangeBothInd FILTER k.a == 2 "+ + " RETURN {A:A, B:B, C:C, D:D, E:E, F:F, G:G, H:H, I:I, J:J, K:k}"; + + var result = AQL_EXPLAIN(query, { }, paramEnabledAllPlans); + assertTrue(result.plans.length < 40, query); + var result = AQL_EXPLAIN(query, { }, paramAllAllPlans); + assertTrue(result.plans.length < 40, query); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test that plan explosion does not happen +//////////////////////////////////////////////////////////////////////////////// + + testRuleMakesAll : function () { + var query = + "LET A = (FOR a IN UTUseIndexRangeBothInd FILTER a.a == 2 RETURN a) "+ + "LET B = (FOR b IN UTUseIndexRangeBothInd FILTER b.a == 2 RETURN b) "+ + "LET C = (FOR c IN UTUseIndexRangeBothInd FILTER c.a == 2 RETURN c) "+ + "FOR k IN UTUseIndexRangeBothInd FILTER k.a == 2 "+ + " RETURN {A:A, B:B, C:C, K:k}"; + + var result = AQL_EXPLAIN(query, { }, paramEnabledAllPlans); + assertTrue(result.plans.length == 16, query); + var result = AQL_EXPLAIN(query, { }, paramAllAllPlans); + assertTrue(result.plans.length == 16, query); + } + + }; +} + +//////////////////////////////////////////////////////////////////////////////// +/// @brief executes the test suite +//////////////////////////////////////////////////////////////////////////////// + +jsunity.run(optimizerRuleUseIndexRangeTester); + +return jsunity.done(); + +// Local Variables: +// mode: outline-minor +// outline-regexp: "^\\(/// @brief\\|/// @addtogroup\\|// --SECTION--\\|/// @page\\|/// @}\\)" +// End: From 2fbc53f71161581dec6ae33776d4051e0f8364e1 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 16:27:05 +0100 Subject: [PATCH 09/17] fixed potential leaks --- arangod/Aql/Optimizer.cpp | 11 ++++++++++- arangod/Aql/Optimizer.h | 15 ++++++++++++++- arangod/Aql/OptimizerRules.cpp | 17 +++++++++++++---- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/arangod/Aql/Optimizer.cpp b/arangod/Aql/Optimizer.cpp index bb88828dc7..4dcec7a47c 100644 --- a/arangod/Aql/Optimizer.cpp +++ b/arangod/Aql/Optimizer.cpp @@ -186,11 +186,20 @@ int Optimizer::createPlans (ExecutionPlan* plan, int res; try { + // all optimizer rule functions must obey the following guidelines: + // - the original plan passed to the rule function must be deleted if and only + // if it has not been added (back) to the optimizer (using addPlan). + // - if the rule throws, then the original plan will be deleted by the optimizer. + // thus the rule must not have deleted the plan itself or add it back to the + // optimizer res = (*it).second.func(this, p, &(it->second)); ++_stats.rulesExecuted; } catch (...) { - delete p; + if (! _newPlans.isContained(p)) { + // only delete the plan if not yet contained in _newPlans + delete p; + } throw; } diff --git a/arangod/Aql/Optimizer.h b/arangod/Aql/Optimizer.h index c084517f27..a9df3f7cdb 100644 --- a/arangod/Aql/Optimizer.h +++ b/arangod/Aql/Optimizer.h @@ -276,12 +276,25 @@ namespace triagens { } } +//////////////////////////////////////////////////////////////////////////////// +/// @brief check if a plan is contained in the list +//////////////////////////////////////////////////////////////////////////////// + + bool isContained (ExecutionPlan* plan) const { + for (auto p : list) { + if (p == plan) { + return true; + } + } + return false; + } + //////////////////////////////////////////////////////////////////////////////// /// @brief get a plan index pointing before the referenced rule, so it can be /// re-executed //////////////////////////////////////////////////////////////////////////////// - static RuleLevel beforeRule(RuleLevel l) { + static RuleLevel beforeRule (RuleLevel l) { return (RuleLevel) (l - 1); } diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index a51f9c68e1..7ddf7458c6 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1379,6 +1379,7 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, if (possibilities == 1) { try { opt->addPlan(plan, rule->level, modified); + cleanupChanges(); } catch (...) { cleanupChanges(); @@ -1395,10 +1396,7 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, std::function &)> doworkrecursive; std::vector> todo; std::vector work; - work.reserve(i); - for (size_t l = 0; l < i; l++) { - work.push_back(0); - } + doworkrecursive = [&doworkrecursive, &changes, &todo] (size_t index, size_t limit, std::vector& v) { @@ -1415,8 +1413,16 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, } } }; + + // if we get here, we can choose between multiple plans... + TRI_ASSERT(possibilities != 1); try { + work.reserve(i); + for (size_t l = 0; l < i; l++) { + work.push_back(0); + } + doworkrecursive(0, i, work); } catch (...) { @@ -1443,7 +1449,10 @@ int triagens::aql::useIndexRangeRule (Optimizer* opt, cleanupChanges(); throw; } + cleanupChanges(); + // finally delete the original plan. all plans created in this rule will be better(tm) + delete plan; return TRI_ERROR_NO_ERROR; } From 6aaa5f05a455ef5ae365be4a99ac118ce446d2e2 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 16:27:14 +0100 Subject: [PATCH 10/17] nullptr --- arangod/V8Server/v8-voccursor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/V8Server/v8-voccursor.cpp b/arangod/V8Server/v8-voccursor.cpp index 8bbfac6aa1..f9d6cb5786 100644 --- a/arangod/V8Server/v8-voccursor.cpp +++ b/arangod/V8Server/v8-voccursor.cpp @@ -297,7 +297,7 @@ static v8::Handle JS_NextGeneralCursor (v8::Arguments const& argv) { try { TRI_general_cursor_row_t row = cursor->next(cursor); - if (row == 0) { + if (row == nullptr) { value = v8::Undefined(); } else { From 17f9c83540f54ef1692c24982406e759452b37ad Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 9 Dec 2014 16:32:37 +0100 Subject: [PATCH 11/17] Fix issue #1147, must encode dispatcher ID for etcd. --- js/server/modules/org/arangodb/cluster/kickstarter.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/js/server/modules/org/arangodb/cluster/kickstarter.js b/js/server/modules/org/arangodb/cluster/kickstarter.js index 3f1dd326a6..7332bb6fd0 100644 --- a/js/server/modules/org/arangodb/cluster/kickstarter.js +++ b/js/server/modules/org/arangodb/cluster/kickstarter.js @@ -301,9 +301,9 @@ launchActions.startServers = function (dispatchers, cmd, isRelaunch) { var url = endpointToURL(cmd.agency.endpoints[0])+"/v2/keys/"+ cmd.agency.agencyPrefix+"/"; - console.info("Downloading %sLaunchers/%s", url, cmd.name); - var res = download(url+"Launchers/"+cmd.name,"",{method:"GET", - followRedirects:true}); + console.info("Downloading %sLaunchers/%s", url, encode(cmd.name)); + var res = download(url+"Launchers/"+encode(cmd.name),"",{method:"GET", + followRedirects:true}); if (res.code !== 200) { return {"error": true, "isStartServers": true, "suberror": res}; } @@ -620,8 +620,8 @@ upgradeActions.startServers = function (dispatchers, cmd, isRelaunch) { var url = endpointToURL(cmd.agency.endpoints[0])+"/v2/keys/"+ cmd.agency.agencyPrefix+"/"; - console.info("Downloading %sLaunchers/%s", url, cmd.name); - var res = download(url+"Launchers/"+cmd.name,"",{method:"GET", + console.info("Downloading %sLaunchers/%s", url, encode(cmd.name)); + var res = download(url+"Launchers/"+encode(cmd.name),"",{method:"GET", followRedirects:true}); if (res.code !== 200) { return {"error": true, "isStartServers": true, "suberror": res}; From 4320142437c2c4bebcc1c3f5f647c02f9e068ddb Mon Sep 17 00:00:00 2001 From: Max Neunhoeffer Date: Tue, 9 Dec 2014 16:34:00 +0100 Subject: [PATCH 12/17] Fix for #1147 into CHANGELOG. --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index b8fc7a8fa3..a93c870f8a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -61,6 +61,7 @@ v2.3.2 (XXXX-XX-XX) This change provides the `KEEP` clause for `COLLECT ... INTO`. The `KEEP` clause allows controlling which variables will be kept in the variable created by `INTO`. +* fixed issue #1147, must protect dispatcher ID for etcd v2.3.1 (2014-11-28) ------------------- From 1fdf74e09e7913b0cd53e123761a7594a9a4ecad Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 16:44:31 +0100 Subject: [PATCH 13/17] jslint --- .../tests/aql-optimizer-rule-use-index-range.js | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/js/server/tests/aql-optimizer-rule-use-index-range.js b/js/server/tests/aql-optimizer-rule-use-index-range.js index 177f0db66f..7088fff8c1 100644 --- a/js/server/tests/aql-optimizer-rule-use-index-range.js +++ b/js/server/tests/aql-optimizer-rule-use-index-range.js @@ -1,5 +1,5 @@ /*jshint strict: false, maxlen: 500 */ -/*global require, assertEqual, assertTrue, AQL_EXPLAIN, AQL_EXECUTE */ +/*global require, assertEqual, assertTrue, AQL_EXPLAIN */ //////////////////////////////////////////////////////////////////////////////// /// @brief tests for optimizer rule use index-range @@ -38,7 +38,6 @@ var removeAlwaysOnClusterRules = helper.removeAlwaysOnClusterRules; //////////////////////////////////////////////////////////////////////////////// function optimizerRuleUseIndexRangeTester () { - var suiteName = "TestUseIndexRangeRule"; var ruleName = "use-index-range"; var collBaseName = "UTUseIndexRange"; var collNames = ["NoInd", "SkipInd", "HashInd", "BothInd"]; @@ -50,7 +49,6 @@ function optimizerRuleUseIndexRangeTester () { // various choices to control the optimizer: var paramNone = { optimizer: { rules: [ "-all" ] } }; var paramEnabled = { optimizer: { rules: [ "-all", "+" + ruleName ] } }; - var paramDisabled = { optimizer: { rules: [ "+all", "-" + ruleName ] } }; var paramAll = { optimizer: { rules: [ "+all" ] } }; var paramEnabledAllPlans = { optimizer: { rules: [ "-all", "+" + ruleName ]}, allPlans: true }; @@ -66,7 +64,7 @@ function optimizerRuleUseIndexRangeTester () { var n = collNames.map(function(x) { return collBaseName + x; }); var colls = []; for (var i = 0; i < n.length; i++) { - var collName = n[i]; + var collName = n[i], coll; internal.db._drop(collName); coll = internal.db._create(collName); for (var j = 0; j < 10; j++) { @@ -198,7 +196,7 @@ function optimizerRuleUseIndexRangeTester () { var result = AQL_EXPLAIN(query, { }, paramEnabledAllPlans); assertTrue(result.plans.length < 40, query); - var result = AQL_EXPLAIN(query, { }, paramAllAllPlans); + result = AQL_EXPLAIN(query, { }, paramAllAllPlans); assertTrue(result.plans.length < 40, query); }, @@ -215,9 +213,9 @@ function optimizerRuleUseIndexRangeTester () { " RETURN {A:A, B:B, C:C, K:k}"; var result = AQL_EXPLAIN(query, { }, paramEnabledAllPlans); - assertTrue(result.plans.length == 16, query); - var result = AQL_EXPLAIN(query, { }, paramAllAllPlans); - assertTrue(result.plans.length == 16, query); + assertTrue(result.plans.length === 16, query); + result = AQL_EXPLAIN(query, { }, paramAllAllPlans); + assertTrue(result.plans.length === 16, query); } }; From f1f0269eb9d44014fb396571bce039503c0b6740 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 16:51:08 +0100 Subject: [PATCH 14/17] issue #1159: allow --server.request-timeout and --server.connect-timeout of 0 --- arangosh/ArangoShell/ArangoClient.cpp | 14 +++++++++++--- arangosh/ArangoShell/ArangoClient.h | 6 ++++++ 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/arangosh/ArangoShell/ArangoClient.cpp b/arangosh/ArangoShell/ArangoClient.cpp index 0a2b2a03a4..72262f62f3 100644 --- a/arangosh/ArangoShell/ArangoClient.cpp +++ b/arangosh/ArangoShell/ArangoClient.cpp @@ -47,6 +47,8 @@ double const ArangoClient::DEFAULT_CONNECTION_TIMEOUT = 3.0; double const ArangoClient::DEFAULT_REQUEST_TIMEOUT = 300.0; size_t const ArangoClient::DEFAULT_RETRIES = 2; +double const ArangoClient::LONG_TIMEOUT = 86400.0; + namespace { #ifdef _WIN32 bool _newLine () { @@ -459,13 +461,19 @@ void ArangoClient::parse (ProgramOptions& options, if (_serverOptions) { // check connection args - if (_connectTimeout <= 0) { - LOG_FATAL_AND_EXIT("invalid value for --server.connect-timeout, must be positive"); + if (_connectTimeout < 0.0) { + LOG_FATAL_AND_EXIT("invalid value for --server.connect-timeout, must be >= 0"); + } + else if (_connectTimeout == 0.0) { + _connectTimeout = LONG_TIMEOUT; } - if (_requestTimeout <= 0) { + if (_requestTimeout < 0.0) { LOG_FATAL_AND_EXIT("invalid value for --server.request-timeout, must be positive"); } + else if (_requestTimeout == 0.0) { + _requestTimeout = LONG_TIMEOUT; + } // must specify a user name if (_username.size() == 0) { diff --git a/arangosh/ArangoShell/ArangoClient.h b/arangosh/ArangoShell/ArangoClient.h index 7a81411e9c..0fdb6fad42 100644 --- a/arangosh/ArangoShell/ArangoClient.h +++ b/arangosh/ArangoShell/ArangoClient.h @@ -87,6 +87,12 @@ namespace triagens { static double const DEFAULT_CONNECTION_TIMEOUT; +//////////////////////////////////////////////////////////////////////////////// +/// @brief default "long" timeout +//////////////////////////////////////////////////////////////////////////////// + + static double const LONG_TIMEOUT; + //////////////////////////////////////////////////////////////////////////////// /// @brief ignore sequence used for prompt length calculation (starting point) /// From 5c9e52ea2036e4899c49a032127b0f1346ababf9 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 17:16:03 +0100 Subject: [PATCH 15/17] pass query parameters --- js/actions/api-explain.js | 2 +- .../modules/org/arangodb/arango-statement.js | 12 +++++- .../modules/org/arangodb/arango-statement.js | 12 +++++- js/common/tests/shell-statement.js | 39 ++++++++++++++++++- .../modules/org/arangodb/arango-statement.js | 19 ++++++--- 5 files changed, 73 insertions(+), 11 deletions(-) diff --git a/js/actions/api-explain.js b/js/actions/api-explain.js index 065947ee01..5779f0b6d8 100644 --- a/js/actions/api-explain.js +++ b/js/actions/api-explain.js @@ -288,7 +288,7 @@ function post_api_explain (req, res) { return; } - if (result.plans) { + if (result.hasOwnProperty("plans")) { result = { plans: result.plans, warnings: result.warnings, diff --git a/js/apps/system/aardvark/frontend/js/modules/org/arangodb/arango-statement.js b/js/apps/system/aardvark/frontend/js/modules/org/arangodb/arango-statement.js index 023ba7e625..c62708148c 100644 --- a/js/apps/system/aardvark/frontend/js/modules/org/arangodb/arango-statement.js +++ b/js/apps/system/aardvark/frontend/js/modules/org/arangodb/arango-statement.js @@ -118,10 +118,18 @@ ArangoStatement.prototype.parse = function () { //////////////////////////////////////////////////////////////////////////////// ArangoStatement.prototype.explain = function (options) { + var opts = this._options || { }; + if (typeof opts === 'object' && typeof options === 'object') { + Object.keys(options).forEach(function(o) { + // copy options + opts[o] = options[o]; + }); + } + var body = { query: this._query, bindVars: this._bindVars, - options: options || { } + options: opts }; var requestResult = this._database._connection.POST( @@ -130,7 +138,7 @@ ArangoStatement.prototype.explain = function (options) { arangosh.checkRequestResult(requestResult); - if (options && options.allPlans) { + if (opts && opts.allPlans) { return { plans: requestResult.plans, warnings: requestResult.warnings, diff --git a/js/client/modules/org/arangodb/arango-statement.js b/js/client/modules/org/arangodb/arango-statement.js index ddfb02dae6..7f93ce2d6b 100644 --- a/js/client/modules/org/arangodb/arango-statement.js +++ b/js/client/modules/org/arangodb/arango-statement.js @@ -117,10 +117,18 @@ ArangoStatement.prototype.parse = function () { //////////////////////////////////////////////////////////////////////////////// ArangoStatement.prototype.explain = function (options) { + var opts = this._options || { }; + if (typeof opts === 'object' && typeof options === 'object') { + Object.keys(options).forEach(function(o) { + // copy options + opts[o] = options[o]; + }); + } + var body = { query: this._query, bindVars: this._bindVars, - options: options || { } + options: opts }; var requestResult = this._database._connection.POST( @@ -129,7 +137,7 @@ ArangoStatement.prototype.explain = function (options) { arangosh.checkRequestResult(requestResult); - if (options && options.allPlans) { + if (opts && opts.allPlans) { return { plans: requestResult.plans, warnings: requestResult.warnings, diff --git a/js/common/tests/shell-statement.js b/js/common/tests/shell-statement.js index 5082edec09..65000c4607 100644 --- a/js/common/tests/shell-statement.js +++ b/js/common/tests/shell-statement.js @@ -249,6 +249,29 @@ function StatementSuite () { assertTrue(plan.hasOwnProperty("variables")); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief test explain method +//////////////////////////////////////////////////////////////////////////////// + + testExplainAllPlansWithOptions : function () { + var st = db._createStatement({ query : "FOR i IN 1..10 RETURN i", options: { allPlans: true } }); + var result = st.explain(); + + assertEqual([ ], result.warnings); + assertFalse(result.hasOwnProperty("plan")); + assertTrue(result.hasOwnProperty("plans")); + + assertEqual(1, result.plans.length); + var plan = result.plans[0]; + assertTrue(plan.hasOwnProperty("estimatedCost")); + assertTrue(plan.hasOwnProperty("rules")); + assertEqual([ ], plan.rules); + assertTrue(plan.hasOwnProperty("nodes")); + assertTrue(plan.hasOwnProperty("collections")); + assertEqual([ ], plan.collections); + assertTrue(plan.hasOwnProperty("variables")); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief test explain method, bind variables //////////////////////////////////////////////////////////////////////////////// @@ -344,6 +367,20 @@ function StatementSuite () { assertTrue(plan.hasOwnProperty("variables")); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief test explain +//////////////////////////////////////////////////////////////////////////////// + + testExplainWithOptions : function () { + var st = db._createStatement({ query : "for i in _users for j in _users return i", options: { allPlans: true, maxNumberOfPlans: 1 } }); + var result = st.explain(); + + assertEqual([ ], result.warnings); + assertFalse(result.hasOwnProperty("plan")); + assertTrue(result.hasOwnProperty("plans")); + assertEqual(1, result.plans.length); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief test execute method //////////////////////////////////////////////////////////////////////////////// @@ -400,7 +437,7 @@ function StatementSuite () { //////////////////////////////////////////////////////////////////////////////// testExecuteExtra : function () { - var st = db._createStatement({ query : "for i in 1..50 limit 1,2 return i", count: true, options: { fullCount: true } }); + var st = db._createStatement({ query : "for i in 1..50 limit 1, 2 return i", count: true, options: { fullCount: true } }); var result = st.execute(); assertEqual(2, result.count()); diff --git a/js/server/modules/org/arangodb/arango-statement.js b/js/server/modules/org/arangodb/arango-statement.js index 605fd436fc..f239e79cbb 100644 --- a/js/server/modules/org/arangodb/arango-statement.js +++ b/js/server/modules/org/arangodb/arango-statement.js @@ -60,7 +60,15 @@ ArangoStatement.prototype.parse = function () { //////////////////////////////////////////////////////////////////////////////// ArangoStatement.prototype.explain = function (options) { - return AQL_EXPLAIN(this._query, this._bindVars, options || { }); + var opts = this._options || { }; + if (typeof opts === 'object' && typeof options === 'object') { + Object.keys(options).forEach(function(o) { + // copy options + opts[o] = options[o]; + }); + } + + return AQL_EXPLAIN(this._query, this._bindVars, opts); }; //////////////////////////////////////////////////////////////////////////////// @@ -70,11 +78,12 @@ ArangoStatement.prototype.explain = function (options) { //////////////////////////////////////////////////////////////////////////////// ArangoStatement.prototype.execute = function () { - var options = this._options || { }; - if (typeof options === 'object') { - options._doCount = this._doCount; + var opts = this._options || { }; + if (typeof opts === 'object') { + opts._doCount = this._doCount; } - var result = AQL_EXECUTE(this._query, this._bindVars, options); + + var result = AQL_EXECUTE(this._query, this._bindVars, opts); return new GeneralArrayCursor(result.json, 0, null, result); }; From 959631c4d956a84a24c7cd177e5f3b5e69c74f91 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 9 Dec 2014 20:46:53 +0100 Subject: [PATCH 16/17] issue #1173: AQL Editor "Save current query" resets user password Conflicts: CHANGELOG --- CHANGELOG | 8 +++ .../frontend/js/collections/arangoQueries.js | 50 +++---------------- 2 files changed, 16 insertions(+), 42 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a93c870f8a..35528813d0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -39,6 +39,12 @@ v2.4.0 (XXXX-XX-XX) v2.3.2 (XXXX-XX-XX) ------------------- +* fixed issue #1173: AQL Editor "Save current query" resets user password + +* fixed missing makeDirectory when fetching a Foxx application from a zip file + +* put in warning about default changed: fixed issue #1134: Change the default endpoint to localhost + * fixed issue #1163: invalid fullCount value returned from AQL * fixed range operator precedence @@ -56,6 +62,8 @@ v2.3.2 (XXXX-XX-XX) * fixed memleaks +* added AQL optimizer rule for removing `INTO` from a `COLLECT` statement if not needed + * fixed issue #1131 This change provides the `KEEP` clause for `COLLECT ... INTO`. The `KEEP` clause diff --git a/js/apps/system/aardvark/frontend/js/collections/arangoQueries.js b/js/apps/system/aardvark/frontend/js/collections/arangoQueries.js index d88f37eb93..49b9a6c8d3 100644 --- a/js/apps/system/aardvark/frontend/js/collections/arangoQueries.js +++ b/js/apps/system/aardvark/frontend/js/collections/arangoQueries.js @@ -27,14 +27,11 @@ activeUser: 0, - currentExtra: {}, - parse: function(response) { var self = this, toReturn; _.each(response.result, function(val) { if (val.user === self.activeUser) { - self.currentExtra = val.extra; try { if (val.extra.queries) { toReturn = val.extra.queries; @@ -52,12 +49,8 @@ return false; } - var queries = [], - returnValue1 = false, - returnValue2 = false, - returnValue3 = false, - extraBackup = null, - self = this; + var returnValue = false; + var queries = []; this.each(function(query) { queries.push({ @@ -66,33 +59,12 @@ }); }); - extraBackup = self.currentExtra; - extraBackup.queries = []; - - $.ajax({ - cache: false, - type: "PUT", - async: false, - url: "/_api/user/" + this.activeUser, - data: JSON.stringify({ - extra: extraBackup - }), - contentType: "application/json", - processData: false, - success: function() { - returnValue1 = true; - }, - error: function() { - returnValue1 = false; - } - }); - - //save current collection + // save current collection $.ajax({ cache: false, type: "PATCH", async: false, - url: "/_api/user/" + this.activeUser, + url: "/_api/user/" + encodeURIComponent(this.activeUser), data: JSON.stringify({ extra: { queries: queries @@ -101,20 +73,14 @@ contentType: "application/json", processData: false, success: function() { - returnValue2 = true; + returnValue = true; }, error: function() { - returnValue2 = false; + returnValue = false; } }); - if (returnValue1 === true && returnValue2 === true) { - returnValue3 = true; - } - else { - returnValue3 = false; - } - return returnValue3; + return returnValue; }, saveImportQueries: function(file, callback) { @@ -128,7 +94,7 @@ cache: false, type: "POST", async: false, - url: "query/upload/" + this.activeUser, + url: "query/upload/" + encodeURIComponent(this.activeUser), data: file, contentType: "application/json", processData: false, From a3a566bc87eacf5329872cc7282b13807f7ecc03 Mon Sep 17 00:00:00 2001 From: Lucas Dohmen Date: Wed, 10 Dec 2014 15:13:07 +0100 Subject: [PATCH 17/17] Fix bug in the user app's storage --- js/apps/system/users/storage.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/apps/system/users/storage.js b/js/apps/system/users/storage.js index 38fdab29eb..72d561db61 100644 --- a/js/apps/system/users/storage.js +++ b/js/apps/system/users/storage.js @@ -73,7 +73,7 @@ user = new User({ user: username, userData: userData, - authData: {active: true} + authData: authData }); users.save(user); } @@ -150,4 +150,4 @@ exports.delete = deleteUser; exports.errors = errors; exports.repository = users; -}()); \ No newline at end of file +}());