From 8f99e59747bf4f8afa9ac6cdae1c0ca02a596fce Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Fri, 5 Dec 2014 19:02:51 +0100 Subject: [PATCH] issue #1163: fullcount was sometimes used for wrong LIMIT node --- UnitTests/Makefile.unittests | 1 + arangod/Aql/ExecutionNode.h | 9 ++- arangod/Aql/ExecutionPlan.cpp | 17 ++--- arangod/Aql/ExecutionPlan.h | 17 +++-- js/server/tests/aql-fullcount.js | 123 +++++++++++++++++++++++++++++++ 5 files changed, 148 insertions(+), 19 deletions(-) create mode 100644 js/server/tests/aql-fullcount.js diff --git a/UnitTests/Makefile.unittests b/UnitTests/Makefile.unittests index 32502e266e..578045a35d 100755 --- a/UnitTests/Makefile.unittests +++ b/UnitTests/Makefile.unittests @@ -539,6 +539,7 @@ SHELL_SERVER_AQL = @top_srcdir@/js/server/tests/aql-arithmetic.js \ @top_srcdir@/js/server/tests/aql-edges-noncluster.js \ @top_srcdir@/js/server/tests/aql-escaping.js \ @top_srcdir@/js/server/tests/aql-explain-noncluster.js \ + @top_srcdir@/js/server/tests/aql-fullcount.js \ @top_srcdir@/js/server/tests/aql-functions.js \ @top_srcdir@/js/server/tests/aql-functions-date.js \ @top_srcdir@/js/server/tests/aql-functions-list.js \ diff --git a/arangod/Aql/ExecutionNode.h b/arangod/Aql/ExecutionNode.h index 402c139590..e5b8ee2d91 100644 --- a/arangod/Aql/ExecutionNode.h +++ b/arangod/Aql/ExecutionNode.h @@ -1268,6 +1268,7 @@ namespace triagens { _offset(offset), _limit(limit), _fullCount(false) { + } LimitNode (ExecutionPlan* plan, @@ -1277,6 +1278,7 @@ namespace triagens { _offset(0), _limit(limit), _fullCount(false) { + } LimitNode (ExecutionPlan*, triagens::basics::Json const& base); @@ -1303,10 +1305,13 @@ namespace triagens { virtual ExecutionNode* clone (ExecutionPlan* plan, bool withDependencies, - bool withProperties) const { + bool withProperties) const override final { auto c = new LimitNode(plan, _id, _offset, _limit); + if (_fullCount) { + c->setFullCount(); + } - CloneHelper (c, plan, withDependencies, withProperties); + CloneHelper(c, plan, withDependencies, withProperties); return static_cast(c); } diff --git a/arangod/Aql/ExecutionPlan.cpp b/arangod/Aql/ExecutionPlan.cpp index 3b0922d013..d9c6264b5a 100644 --- a/arangod/Aql/ExecutionPlan.cpp +++ b/arangod/Aql/ExecutionPlan.cpp @@ -56,11 +56,11 @@ ExecutionPlan::ExecutionPlan (Ast* ast) : _ids(), _root(nullptr), _varUsageComputed(false), - _mustSetFullCount(false), _nextId(0), - _ast(ast) { - _lastSubqueryNodeId = (size_t) -1; + _ast(ast), + _lastLimitNode(nullptr) { + _lastSubqueryNodeId = (size_t) -1; } //////////////////////////////////////////////////////////////////////////////// @@ -90,10 +90,12 @@ ExecutionPlan* ExecutionPlan::instanciateFromAst (Ast* ast) { auto plan = new ExecutionPlan(ast); - plan->_mustSetFullCount = ast->query()->getBooleanOption("fullCount", false); - try { plan->_root = plan->fromNode(root); + // insert fullCount flag + if (plan->_lastLimitNode != nullptr && ast->query()->getBooleanOption("fullCount", false)) { + static_cast(plan->_lastLimitNode)->setFullCount(); + } plan->findVarUsage(); return plan; // just for debugging @@ -635,10 +637,7 @@ ExecutionNode* ExecutionPlan::fromNodeLimit (ExecutionNode* previous, auto en = registerNode(new LimitNode(this, nextId(), static_cast(offset->getIntValue()), static_cast(count->getIntValue()))); - if (_mustSetFullCount) { - static_cast(en)->setFullCount(); - _mustSetFullCount = false; - } + _lastLimitNode = en; return addDependency(previous, en); } diff --git a/arangod/Aql/ExecutionPlan.h b/arangod/Aql/ExecutionPlan.h index 1cdbd6a20b..36ce22ee02 100644 --- a/arangod/Aql/ExecutionPlan.h +++ b/arangod/Aql/ExecutionPlan.h @@ -467,13 +467,6 @@ namespace triagens { bool _varUsageComputed; -//////////////////////////////////////////////////////////////////////////////// -/// @brief whether or not the next LIMIT node will get its fullCount attribute -/// set -//////////////////////////////////////////////////////////////////////////////// - - bool _mustSetFullCount; - //////////////////////////////////////////////////////////////////////////////// /// @brief auto-increment sequence for node ids //////////////////////////////////////////////////////////////////////////////// @@ -485,12 +478,20 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// size_t _lastSubqueryNodeId; - + //////////////////////////////////////////////////////////////////////////////// /// @brief the ast //////////////////////////////////////////////////////////////////////////////// Ast* _ast; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief whether or not the next LIMIT node will get its fullCount attribute +/// set +//////////////////////////////////////////////////////////////////////////////// + + ExecutionNode* _lastLimitNode; + }; } diff --git a/js/server/tests/aql-fullcount.js b/js/server/tests/aql-fullcount.js new file mode 100644 index 0000000000..ad3d79e72d --- /dev/null +++ b/js/server/tests/aql-fullcount.js @@ -0,0 +1,123 @@ +/*jshint strict: false, maxlen: 500 */ +/*global require, assertUndefined, assertEqual, AQL_EXECUTE */ + +//////////////////////////////////////////////////////////////////////////////// +/// @brief tests for index usage +/// +/// @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 db = require("org/arangodb").db; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test suite +//////////////////////////////////////////////////////////////////////////////// + +function optimizerFullcountTestSuite () { + var c; + + return { + setUp : function () { + db._drop("UnitTestsCollection"); + c = db._create("UnitTestsCollection"); + + c.save({ values: [ "foo", "bar" ]}); + c.save({ values: [ "bar" ]}); + c.save({ values: [ "baz" ]}); + }, + + tearDown : function () { + db._drop("UnitTestsCollection"); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test no fullcount +//////////////////////////////////////////////////////////////////////////////// + + testNoFullcount : function () { + var result = AQL_EXECUTE("FOR doc IN UnitTestsCollection LET values = doc.values LET found = (FOR value IN values FILTER value == 'bar' || value == 'foo' LIMIT 1 RETURN true) FILTER LENGTH(found) > 0 LIMIT 10 RETURN doc", null, { }); + + assertUndefined(result.stats.fullCount); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test with fullcount +//////////////////////////////////////////////////////////////////////////////// + + testWithFullcount : function () { + var result = AQL_EXECUTE("FOR doc IN UnitTestsCollection LET values = doc.values LET found = (FOR value IN values FILTER value == 'bar' || value == 'foo' LIMIT 1 RETURN true) FILTER LENGTH(found) > 0 LIMIT 10 RETURN doc", null, { fullCount: true }); + + assertEqual(2, result.stats.fullCount); + assertEqual(2, result.json.length); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test with fullcount +//////////////////////////////////////////////////////////////////////////////// + + testWithFullcountUsingLimit : function () { + var result = AQL_EXECUTE("FOR doc IN UnitTestsCollection LET values = doc.values LET found = (FOR value IN values FILTER value == 'bar' || value == 'foo' LIMIT 1 RETURN true) FILTER LENGTH(found) > 0 LIMIT 1 RETURN doc", null, { fullCount: true }); + + assertEqual(2, result.stats.fullCount); + assertEqual(1, result.json.length); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test with fullcount +//////////////////////////////////////////////////////////////////////////////// + + testWithFullCountSingleLevel : function () { + var result = AQL_EXECUTE("FOR doc IN UnitTestsCollection FILTER 'bar' IN doc.values LIMIT 10 RETURN doc", null, { fullCount: true }); + + assertEqual(2, result.stats.fullCount); + assertEqual(2, result.json.length); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test with fullcount +//////////////////////////////////////////////////////////////////////////////// + + testWithFullCountSingleLevelUsingLimit : function () { + var result = AQL_EXECUTE("FOR doc IN UnitTestsCollection FILTER 'bar' IN doc.values LIMIT 1 RETURN doc", null, { fullCount: true }); + + assertEqual(2, result.stats.fullCount); + assertEqual(1, result.json.length); + } + + }; +} + +//////////////////////////////////////////////////////////////////////////////// +/// @brief executes the test suite +//////////////////////////////////////////////////////////////////////////////// + +jsunity.run(optimizerFullcountTestSuite); + +return jsunity.done(); + +// Local Variables: +// mode: outline-minor +// outline-regexp: "^\\(/// @brief\\|/// @addtogroup\\|// --SECTION--\\|/// @page\\|/// @}\\)" +// End: