From 7dbc89d7be958f35e3f052d4a1a000d8e2c3039a Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Tue, 9 Feb 2016 16:11:16 +0100 Subject: [PATCH 01/10] Actually register static Foxx model events Fixes #1665. --- .../modules/@arangodb/foxx/repository.js | 3 +- .../shell-foxx-repository-events-auto-spec.js | 123 ++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 js/server/tests/shell/shell-foxx-repository-events-auto-spec.js diff --git a/js/server/modules/@arangodb/foxx/repository.js b/js/server/modules/@arangodb/foxx/repository.js index 8157de3ecf..9f5d19f0c0 100644 --- a/js/server/modules/@arangodb/foxx/repository.js +++ b/js/server/modules/@arangodb/foxx/repository.js @@ -87,7 +87,8 @@ function Repository(collection, opts) { EventEmitter.call(this); - _.each(this.model, function (listener, eventName) { + Object.keys(this.model).forEach(function (eventName) { + const listener = this.model[eventName]; if (EVENTS.indexOf(eventName) === -1 || typeof listener !== 'function') { return; } diff --git a/js/server/tests/shell/shell-foxx-repository-events-auto-spec.js b/js/server/tests/shell/shell-foxx-repository-events-auto-spec.js new file mode 100644 index 0000000000..5c0f545694 --- /dev/null +++ b/js/server/tests/shell/shell-foxx-repository-events-auto-spec.js @@ -0,0 +1,123 @@ +/*global describe, it, beforeEach */ +'use strict'; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief Spec for foxx repository +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2016 ArangoDB 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 ArangoDB GmbH, Cologne, Germany +/// +/// @author Alan Plum +/// @author Copyright 2016, ArangoDB GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +const expect = require('chai').expect; +const sinon = require('sinon'); +const FoxxRepository = require('@arangodb/foxx/repository').Repository; +const FoxxModel = require('@arangodb/foxx/model').Model; + +describe('Repository Events', function () { + let collection, Model; + + beforeEach(function () { + Model = FoxxModel.extend({}, {}); + collection = {}; + }); + + it('should emit beforeCreate and afterCreate events when creating the model', function () { + var model = prepInstance(Model, 'Create'); + collection.type = sinon.stub().returns(2); + collection.save = sinon.stub(); + var repository = new FoxxRepository(collection, {model: Model}); + expect(repository.save(model)).to.equal(model); + expect(model.get('beforeCalled')).to.equal(true); + expect(model.get('afterCalled')).to.equal(true); + }); + + it('should emit beforeSave and afterSave events when creating the model', function () { + var model = prepInstance(Model, 'Save'); + collection.type = sinon.stub().returns(2); + collection.save = sinon.stub(); + var repository = new FoxxRepository(collection, {model: Model}); + expect(repository.save(model)).to.equal(model); + expect(model.get('beforeCalled')).to.equal(true); + expect(model.get('afterCalled')).to.equal(true); + }); + + it('should emit beforeUpdate and afterUpdate events when updating the model', function () { + var newData = {newAttribute: 'test'}; + var model = prepInstance(Model, 'Update', newData); + collection.type = sinon.stub().returns(2); + collection.update = sinon.stub(); + var repository = new FoxxRepository(collection, {model: Model}); + expect(repository.update(model, newData)).to.equal(model); + expect(model.get('beforeCalled')).to.equal(true); + expect(model.get('afterCalled')).to.equal(true); + }); + + it('should emit beforeSave and afterSave events when updating the model', function () { + var newData = {newAttribute: 'test'}; + var model = prepInstance(Model, 'Save', newData); + collection.type = sinon.stub().returns(2); + collection.update = sinon.stub(); + var repository = new FoxxRepository(collection, {model: Model}); + expect(repository.update(model, newData)).to.equal(model); + expect(model.get('beforeCalled')).to.equal(true); + expect(model.get('afterCalled')).to.equal(true); + }); + + it('should emit beforeRemove and afterRemove events when removing the model', function () { + var model = prepInstance(Model, 'Remove'); + collection.type = sinon.stub().returns(2); + collection.remove = sinon.stub(); + var repository = new FoxxRepository(collection, {model: Model}); + repository.remove(model); + expect(model.get('beforeCalled')).to.equal(true); + expect(model.get('afterCalled')).to.equal(true); + }); +}); + +function prepInstance(Model, ev, dataToReceive) { + let model; + const random = String(Math.floor(Math.random() * 1000)); + + Model['before' + ev] = function (instance, data) { + expect(instance).to.equal(model); + expect(data).to.equal(dataToReceive); + instance.set('random', random); + instance.set('beforeCalled', true); + }; + + Model['after' + ev] = function (instance, data) { + expect(instance).to.equal(model); + expect(data).to.equal(dataToReceive); + instance.set('afterCalled', true); + expect(instance.get('beforeCalled')).to.equal(true); + expect(instance.get('random')).to.equal(random); + }; + + model = new Model({ + random: '', + beforeCalled: false, + afterCalled: false + }); + + return model; +} From b769552ce6007210d32ade9299c367dc4e478fd0 Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Tue, 9 Feb 2016 16:13:37 +0100 Subject: [PATCH 02/10] Add Model fix to changelog --- CHANGELOG | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index fb98210709..f2a3fbd0d8 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -79,7 +79,14 @@ v3.0.0 (XXXX-XX-XX) now takes the form $name@$version.zip instead of simply "app.zip" -v2.8.2 (XXXX-XX-XX) +v2.8.3 (XXXX-XX-XX) +------------------- + +* Foxx Model event listeners defined on the model are now correctly invoked by + the Repository methods (issue #1665) + + +v2.8.2 (2016-02-09) ------------------- * the continuous replication applier will now prevent the master's WAL logfiles From 950234aaffe15b6803afe69ca0b0f0881c427f31 Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Tue, 9 Feb 2016 16:27:32 +0100 Subject: [PATCH 03/10] Recreate missing test from 2.7 --- .../shell-foxx-repository-events-spec.js | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 js/server/tests/shell/shell-foxx-repository-events-spec.js diff --git a/js/server/tests/shell/shell-foxx-repository-events-spec.js b/js/server/tests/shell/shell-foxx-repository-events-spec.js new file mode 100644 index 0000000000..4f675d9783 --- /dev/null +++ b/js/server/tests/shell/shell-foxx-repository-events-spec.js @@ -0,0 +1,115 @@ +/*global describe, it, beforeEach */ +'use strict'; + +//////////////////////////////////////////////////////////////////////////////// +/// @brief Spec for foxx repository +/// +/// @file +/// +/// DISCLAIMER +/// +/// Copyright 2015-2016 ArangoDB 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 ArangoDB GmbH, Cologne, Germany +/// +/// @author Alan Plum +/// @author Copyright 2015, ArangoDB GmbH, Cologne, Germany +//////////////////////////////////////////////////////////////////////////////// + +const expect = require('chai').expect; +const sinon = require('sinon'); +const FoxxRepository = require('@arangodb/foxx/repository').Repository; +const Model = require('@arangodb/foxx/model').Model; + +describe('Model Events', function () { + let collection, instance, repository; + + beforeEach(function () { + collection = { + type: sinon.stub().returns(2) + }; + instance = new Model(); + repository = new FoxxRepository(collection, {model: Model, random: '', beforeCalled: false, afterCalled: false}); + }); + + it('should be possible to subscribe and emit events', function () { + expect(repository.on).to.be.a('function'); + expect(repository.emit).to.be.a('function'); + }); + + it('should emit beforeCreate and afterCreate events when creating the model', function () { + collection.save = sinon.stub(); + addHooks(repository, instance, 'Create'); + expect(repository.save(instance)).to.equal(instance); + expect(repository.beforeCalled).to.equal(true); + expect(repository.afterCalled).to.equal(true); + }); + + it('should emit beforeSave and afterSave events when creating the model', function () { + collection.save = sinon.stub(); + addHooks(repository, instance, 'Save'); + expect(repository.save(instance)).to.equal(instance); + expect(repository.beforeCalled).to.equal(true); + expect(repository.afterCalled).to.equal(true); + }); + + it('should emit beforeUpdate and afterUpdate events when updating the model', function () { + var newData = {newAttribute: 'test'}; + collection.update = sinon.stub(); + addHooks(repository, instance, 'Update', newData); + expect(repository.update(instance, newData)).to.equal(instance); + expect(repository.beforeCalled).to.equal(true); + expect(repository.afterCalled).to.equal(true); + }); + + it('should emit beforeSave and afterSave events when updating the model', function () { + var newData = {newAttribute: 'test'}; + collection.update = sinon.stub(); + addHooks(repository, instance, 'Save', newData); + expect(repository.update(instance, newData)).to.equal(instance); + expect(repository.beforeCalled).to.equal(true); + expect(repository.afterCalled).to.equal(true); + }); + + it('should emit beforeRemove and afterRemove events when removing the model', function () { + collection.remove = sinon.stub(); + addHooks(repository, instance, 'Remove'); + repository.remove(instance); + expect(repository.beforeCalled).to.equal(true); + expect(repository.afterCalled).to.equal(true); + }); + +}); + +function addHooks(repo, model, ev, dataToReceive) { + const random = String(Math.floor(Math.random() * 1000)); + + repo.on('before' + ev, function (self, data) { + expect(this).to.equal(repo); + expect(self).to.equal(model); + expect(data).to.equal(dataToReceive); + this.random = random; + this.beforeCalled = true; + }); + + repo.on('after' + ev, function (self, data) { + expect(this).to.equal(repo); + expect(self).to.equal(model); + expect(data).to.equal(dataToReceive); + this.afterCalled = true; + expect(this.beforeCalled).to.equal(true); + expect(this.random).to.equal(random); + }); +} From ac18d26c720af7146141ef44b57de8e4eadccbf8 Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Tue, 9 Feb 2016 17:28:13 +0100 Subject: [PATCH 04/10] Always force when deleting foxxes in aardvark Fixes #1358. --- js/apps/system/_admin/aardvark/APP/foxxes.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/apps/system/_admin/aardvark/APP/foxxes.js b/js/apps/system/_admin/aardvark/APP/foxxes.js index f29f7f8c94..f33b9d1cd5 100644 --- a/js/apps/system/_admin/aardvark/APP/foxxes.js +++ b/js/apps/system/_admin/aardvark/APP/foxxes.js @@ -170,7 +170,8 @@ var mount = validateMount(req); var runTeardown = req.parameters.teardown; var app = FoxxManager.uninstall(mount, { - teardown: runTeardown + teardown: runTeardown, + force: true }); res.json({ error: false, From dab30b050e2e83a7c900308077d9fe2164c7abde Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Tue, 9 Feb 2016 17:29:14 +0100 Subject: [PATCH 05/10] Add delete fix to changelog --- CHANGELOG | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index f2a3fbd0d8..ca96771979 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -85,6 +85,9 @@ v2.8.3 (XXXX-XX-XX) * Foxx Model event listeners defined on the model are now correctly invoked by the Repository methods (issue #1665) +* Deleting a Foxx service in the frontend should now always succeed even if the + files no longer exist on the file system (issue #1358) + v2.8.2 (2016-02-09) ------------------- From 5a44da965a583966fd1687f670a82cd4f400e7df Mon Sep 17 00:00:00 2001 From: Frank Celler Date: Wed, 10 Feb 2016 09:35:09 +0100 Subject: [PATCH 06/10] skip non-conn-keep-alive test by default --- js/client/modules/@arangodb/testing.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/client/modules/@arangodb/testing.js b/js/client/modules/@arangodb/testing.js index 23b7dba3df..8d9704a2f2 100644 --- a/js/client/modules/@arangodb/testing.js +++ b/js/client/modules/@arangodb/testing.js @@ -130,7 +130,7 @@ const optionsDefaults = { "replication": false, "skipAql": false, "skipArangoB": false, - "skipArangoBNonConnKeepAlive": false, + "skipArangoBNonConnKeepAlive": true, "skipBoost": false, "skipGeo": false, "skipLogAnalysis": false, @@ -1807,7 +1807,6 @@ function filterTestcaseByOptions(testname, options, whichFilter) { //////////////////////////////////////////////////////////////////////////////// let allTests = [ - "arangob", "arangosh", "authentication", "authentication_parameters", @@ -1822,7 +1821,8 @@ let allTests = [ "shell_server", "shell_server_aql", "ssl_server", - "upgrade" + "upgrade", + "arangob" ]; //////////////////////////////////////////////////////////////////////////////// From 88fad2e789f0ca39973aae72e31af64502d47bc7 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Wed, 10 Feb 2016 10:36:52 +0100 Subject: [PATCH 07/10] Migrate rspec to persistent connections --- UnitTests/HttpInterface/Gemfile | 1 + UnitTests/HttpInterface/arangodb.rb | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/UnitTests/HttpInterface/Gemfile b/UnitTests/HttpInterface/Gemfile index 625a995d5d..f322d0d228 100644 --- a/UnitTests/HttpInterface/Gemfile +++ b/UnitTests/HttpInterface/Gemfile @@ -1,5 +1,6 @@ source :rubygems gem "httparty", "~> 0.8.1" +gem "persistent_httparty" gem "rspec", "~> 2.14.0" gem "rspec-core", "~> 2.14.0" diff --git a/UnitTests/HttpInterface/arangodb.rb b/UnitTests/HttpInterface/arangodb.rb index f54d51c41f..14fb55e544 100644 --- a/UnitTests/HttpInterface/arangodb.rb +++ b/UnitTests/HttpInterface/arangodb.rb @@ -1,7 +1,7 @@ # coding: utf-8 require 'rubygems' -require 'httparty' +require 'persistent_httparty' require 'json' require 'rspec' require 'rspec/expectations' @@ -54,6 +54,7 @@ end class ArangoDB include HTTParty + persistent_connection_adapter if $ssl == '1' base_uri "https://#{$address}" From 3b12dc0c329593407242c5b5e2dc3964323e6999 Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Wed, 10 Feb 2016 10:48:03 +0100 Subject: [PATCH 08/10] Remove httparty from the bundle - it comes as a dependency from persistent_httparty --- UnitTests/HttpInterface/Gemfile | 1 - 1 file changed, 1 deletion(-) diff --git a/UnitTests/HttpInterface/Gemfile b/UnitTests/HttpInterface/Gemfile index f322d0d228..5bcfa0b728 100644 --- a/UnitTests/HttpInterface/Gemfile +++ b/UnitTests/HttpInterface/Gemfile @@ -1,6 +1,5 @@ source :rubygems -gem "httparty", "~> 0.8.1" gem "persistent_httparty" gem "rspec", "~> 2.14.0" gem "rspec-core", "~> 2.14.0" From d2f8021d8e8ab3759ca8d28c86cfa09002475d9e Mon Sep 17 00:00:00 2001 From: Wilfried Goesgens Date: Wed, 10 Feb 2016 11:21:08 +0100 Subject: [PATCH 09/10] make examples is history now. --- Documentation/Makefile.files | 49 ---------------------------- Documentation/Scripts/allExamples.sh | 8 +++++ README_maintainers.md | 9 +++-- 3 files changed, 12 insertions(+), 54 deletions(-) create mode 100755 Documentation/Scripts/allExamples.sh diff --git a/Documentation/Makefile.files b/Documentation/Makefile.files index 1479cddf86..79368b0cef 100644 --- a/Documentation/Makefile.files +++ b/Documentation/Makefile.files @@ -50,55 +50,6 @@ swagger: @srcdir@/Documentation/DocuBlocks/Rest/ \ ) > @srcdir@/js/apps/system/_admin/aardvark/APP/api-docs.json -## ----------------------------------------------------------------------------- -## --SECTION-- EXAMPLES -## ----------------------------------------------------------------------------- - -################################################################################ -### @brief generate examples -################################################################################ - -.PHONY: examples - -examples: - @rm -f /tmp/arangodb.examples - rm -rf @srcdir@/Documentation/Books/ppbooks - python @srcdir@/Documentation/Scripts/generateExamples.py \ - --outputDir @builddir@/Documentation/Examples \ - --onlyThisOne "$(FILTER_EXAMPLE)" \ - --outputFile /tmp/arangosh.examples.js \ - --arangoshSetup @srcdir@/Documentation/Examples/setup-arangosh.js \ - @srcdir@/Documentation/DocuBlocks \ - @srcdir@/Documentation/Books/Users - - if test -z "$(server.endpoint)"; then \ - $(MAKE) start-server PID=$(PID) \ - SERVER_START="--server.endpoint tcp://$(VOCHOST):$(VOCPORT) --server.disable-authentication true" \ - PROTO=http ;\ - @builddir@/bin/arangosh \ - -s \ - -c @srcdir@/etc/relative/arangosh.conf \ - --server.password "" \ - --server.endpoint tcp://$(VOCHOST):$(VOCPORT) \ - --javascript.execute /tmp/arangosh.examples.js ;\ - else \ - @builddir@/bin/arangosh \ - -s \ - -c @srcdir@/etc/relative/arangosh.conf \ - --server.password "" \ - --server.endpoint $(server.endpoint) \ - --javascript.execute /tmp/arangosh.examples.js ;\ - fi - -examples-inspect-file: - @echo "generating /tmp/allExamples.html" - @(printf "
\n"; \
-	   for thisExample in Documentation/Examples/*.generated; do \
-		printf "
\n

$$thisExample

\n"; \ - cat $$thisExample; \ - done; \ - printf "
") > /tmp/allExamples.html - ## ----------------------------------------------------------------------------- ## --SECTION-- END-OF-FILE ## ----------------------------------------------------------------------------- diff --git a/Documentation/Scripts/allExamples.sh b/Documentation/Scripts/allExamples.sh new file mode 100755 index 0000000000..aa882f11e3 --- /dev/null +++ b/Documentation/Scripts/allExamples.sh @@ -0,0 +1,8 @@ +#!/bin/sh +echo "generating /tmp/allExamples.html" +(printf "
\n";
+ for thisExample in Documentation/Examples/*.generated; do 
+     printf "
\n

$thisExample

\n"; + cat $thisExample; + done; + printf "
") > /tmp/allExamples.html diff --git a/README_maintainers.md b/README_maintainers.md index 51b229275f..7f045a18f3 100644 --- a/README_maintainers.md +++ b/README_maintainers.md @@ -447,18 +447,17 @@ Where to add new... generate -------- - - `make examples` - on top level to generate Documentation/Examples - - `make examples FILTER_EXAMPLE=geoIndexSelect` will only produce one example - *geoIndexSelect* - - `make examples FILTER_EXAMPLE='MOD.*'` will only produce the examples matching that regex; Note that + - `./scripts/generateExamples --onlyThisOne geoIndexSelect` will only produce one example - *geoIndexSelect* + - `./scripts/generateExamples --onlyThisOne 'MOD.*'` will only produce the examples matching that regex; Note that examples with enumerations in their name may base on others in their series - so you should generate the whole group. - - `make examples server.endpoint=tcp://127.0.0.1:8529` will utilize an existing arangod instead of starting a new one. + - `./scripts/generateExamples --server.endpoint tcp://127.0.0.1:8529` will utilize an existing arangod instead of starting a new one. this does seriously cut down the execution time. - alternatively you can use generateExamples (i.e. on windows since the make target is not portable) like that: `./scripts/generateExamples --server.endpoint 'tcp://127.0.0.1:8529' --withPython 3rdParty/V8-4.3.61/third_party/python_26/python26.exe --onlyThisOne 'MOD.*'` - - `make examples-inspect-file` generates a file where you can inspect all examples for readability. + - `./Documentation/Scripts/allExamples.sh` generates a file where you can inspect all examples for readability. - `make swagger` - on top level to generate the documentation interactively with the server; you may use [the swagger editor](https://github.com/swagger-api/swagger-editor) to revalidate whether *js/apps/system/_admin/aardvark/APP/api-docs.json* is accurate. From 5ba2432d78b76b86b6f7e13491a5639d186e7a9d Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Wed, 10 Feb 2016 11:23:11 +0100 Subject: [PATCH 10/10] remove sort in more cases --- arangod/Aql/Condition.cpp | 59 ++++++- arangod/Aql/Condition.h | 7 + arangod/Aql/ConditionFinder.cpp | 4 +- arangod/Aql/OptimizerRules.cpp | 33 ++-- arangod/Aql/SortCondition.cpp | 165 +++++++++--------- arangod/Aql/SortCondition.h | 18 +- .../modules/common/@arangodb/aql/explainer.js | 2 +- js/common/modules/@arangodb/aql/explainer.js | 2 +- .../tests/aql/aql-optimizer-indexes-sort.js | 86 ++++++++- .../aql/aql-optimizer-rule-sort-in-values.js | 10 +- .../aql-optimizer-rule-use-index-for-sort.js | 61 ++++++- 11 files changed, 319 insertions(+), 128 deletions(-) diff --git a/arangod/Aql/Condition.cpp b/arangod/Aql/Condition.cpp index 6a2b0ebdb1..732d57687d 100644 --- a/arangod/Aql/Condition.cpp +++ b/arangod/Aql/Condition.cpp @@ -415,7 +415,13 @@ std::pair Condition::findIndexes( usedIndexes.emplace_back(sortIndex); } - return std::make_pair(false, true); + TRI_ASSERT(usedIndexes.size() == 1); + + if (usedIndexes.back()->sparse) { + // cannot use a sparse index for sorting alone + usedIndexes.clear(); + } + return std::make_pair(false, !usedIndexes.empty()); } canUseForFilter &= canUseIndex.first; @@ -450,6 +456,55 @@ bool Condition::indexSupportsSort(Index const* idx, Variable const* reference, } return false; } + +//////////////////////////////////////////////////////////////////////////////// +/// @brief get the attributes for a sub-condition that are const +/// (i.e. compared with equality) +//////////////////////////////////////////////////////////////////////////////// + +std::vector> Condition::getConstAttributes (Variable const* reference, + bool includeNull) { + std::vector> result; + + if (_root == nullptr) { + return result; + } + + size_t n = _root->numMembers(); + + if (n != 1) { + return result; + } + + AstNode const* node = _root->getMember(0); + n = node->numMembers(); + + for (size_t i = 0; i < n; ++i) { + auto member = node->getMember(i); + + if (member->type == NODE_TYPE_OPERATOR_BINARY_EQ) { + std::pair> parts; + + auto lhs = member->getMember(0); + auto rhs = member->getMember(1); + + if (lhs->isAttributeAccessForVariable(parts) && + parts.first == reference) { + if (includeNull || (rhs->isConstant() && !rhs->isNullValue())) { + result.emplace_back(std::move(parts.second)); + } + } + else if (rhs->isAttributeAccessForVariable(parts) && + parts.first == reference) { + if (includeNull || (lhs->isConstant() && !lhs->isNullValue())) { + result.emplace_back(std::move(parts.second)); + } + } + } + } + + return result; +} //////////////////////////////////////////////////////////////////////////////// /// @brief finds the best index that can match this single node @@ -517,7 +572,7 @@ std::pair Condition::findIndexForAndNode( // now check if the index fields are the same as the sort condition fields // e.g. FILTER c.value1 == 1 && c.value2 == 42 SORT c.value1, c.value2 size_t coveredFields = - sortCondition->coveredAttributes(reference, idx->fields); + sortCondition->coveredAttributes(reference, idx->fields); if (coveredFields == sortCondition->numAttributes() && (idx->isSorted() || diff --git a/arangod/Aql/Condition.h b/arangod/Aql/Condition.h index 17ad20460e..494c4fc610 100644 --- a/arangod/Aql/Condition.h +++ b/arangod/Aql/Condition.h @@ -294,6 +294,13 @@ class Condition { std::vector&, SortCondition const*); + ////////////////////////////////////////////////////////////////////////////// + /// @brief get the attributes for a sub-condition that are const + /// (i.e. compared with equality) + ////////////////////////////////////////////////////////////////////////////// + + std::vector> getConstAttributes (Variable const*, bool); + private: ////////////////////////////////////////////////////////////////////////////// /// @brief sort ORs for the same attribute so they are in ascending value diff --git a/arangod/Aql/ConditionFinder.cpp b/arangod/Aql/ConditionFinder.cpp index 483cecbf75..ef41d32aa8 100644 --- a/arangod/Aql/ConditionFinder.cpp +++ b/arangod/Aql/ConditionFinder.cpp @@ -21,7 +21,7 @@ /// @author Michael Hackstein //////////////////////////////////////////////////////////////////////////////// -#include "Aql/ConditionFinder.h" +#include "ConditionFinder.h" #include "Aql/ExecutionPlan.h" #include "Aql/IndexNode.h" #include "Aql/SortCondition.h" @@ -153,7 +153,7 @@ bool ConditionFinder::before(ExecutionNode* en) { std::unique_ptr sortCondition; if (!en->isInInnerLoop()) { // we cannot optimize away a sort if we're in an inner loop ourselves - sortCondition.reset(new SortCondition(_sorts, _variableDefinitions)); + sortCondition.reset(new SortCondition(_sorts, condition->getConstAttributes(node->outVariable(), false), _variableDefinitions)); } else { sortCondition.reset(new SortCondition); } diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index 969f48c2a2..73fc25a566 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -38,6 +38,7 @@ #include "Aql/TraversalConditionFinder.h" #include "Aql/Variable.h" #include "Aql/types.h" +#include "Basics/AttributeNameParser.h" #include "Basics/json-utilities.h" using namespace arangodb::aql; @@ -1705,7 +1706,7 @@ struct SortToIndexNode final : public WalkerWorker { return true; } - SortCondition sortCondition(_sorts, _variableDefinitions); + SortCondition sortCondition(_sorts, std::vector>(), _variableDefinitions); if (!sortCondition.isEmpty() && sortCondition.isOnlyAttributeAccess() && sortCondition.isUnidirectional()) { @@ -1725,8 +1726,8 @@ struct SortToIndexNode final : public WalkerWorker { continue; } - auto numCovered = - sortCondition.coveredAttributes(outVariable, index->fields); + size_t const numCovered = + sortCondition.coveredAttributes(outVariable, index->fields); if (numCovered == 0) { continue; @@ -1734,14 +1735,15 @@ struct SortToIndexNode final : public WalkerWorker { double estimatedCost = 0.0; if (!index->supportsSortCondition( - &sortCondition, outVariable, + &sortCondition, + outVariable, enumerateCollectionNode->collection()->count(), estimatedCost)) { // should never happen TRI_ASSERT(false); continue; } - + if (bestIndex == nullptr || estimatedCost < bestCost) { bestIndex = index; bestCost = estimatedCost; @@ -1790,6 +1792,10 @@ struct SortToIndexNode final : public WalkerWorker { auto const& indexes = indexNode->getIndexes(); auto cond = indexNode->condition(); + TRI_ASSERT(cond != nullptr); + + Variable const* outVariable = indexNode->outVariable(); + TRI_ASSERT(outVariable != nullptr); if (indexes.size() != 1) { // can only use this index node if it uses exactly one index or multiple @@ -1824,7 +1830,7 @@ struct SortToIndexNode final : public WalkerWorker { auto index = indexes[0]; bool handled = false; - SortCondition sortCondition(_sorts, _variableDefinitions); + SortCondition sortCondition(_sorts, cond->getConstAttributes(outVariable, !index->sparse), _variableDefinitions); bool const isOnlyAttributeAccess = (!sortCondition.isEmpty() && sortCondition.isOnlyAttributeAccess()); @@ -1835,11 +1841,10 @@ struct SortToIndexNode final : public WalkerWorker { // we have found a sort condition, which is unidirectional and in the same // order as the IndexNode... // now check if the sort attributes match the ones of the index - Variable const* outVariable = indexNode->outVariable(); - auto numCovered = - sortCondition.coveredAttributes(outVariable, index->fields); + size_t const numCovered = + sortCondition.coveredAttributes(outVariable, index->fields); - if (numCovered == sortCondition.numAttributes()) { + if (numCovered >= sortCondition.numAttributes()) { // sort condition is fully covered by index... now we can remove the // sort node from the plan _plan->unlinkNode(_plan->getNodeById(_sortNode->id())); @@ -1862,11 +1867,11 @@ struct SortToIndexNode final : public WalkerWorker { // now check if the index fields are the same as the sort condition // fields // e.g. FILTER c.value1 == 1 && c.value2 == 42 SORT c.value1, c.value2 - Variable const* outVariable = indexNode->outVariable(); - size_t coveredFields = - sortCondition.coveredAttributes(outVariable, index->fields); + size_t const numCovered = + sortCondition.coveredAttributes(outVariable, index->fields); - if (coveredFields == sortCondition.numAttributes() && + if (numCovered == sortCondition.numAttributes() && + sortCondition.isUnidirectional() && (index->isSorted() || index->fields.size() == sortCondition.numAttributes())) { // no need to sort diff --git a/arangod/Aql/SortCondition.cpp b/arangod/Aql/SortCondition.cpp index f682525690..055099c5c4 100644 --- a/arangod/Aql/SortCondition.cpp +++ b/arangod/Aql/SortCondition.cpp @@ -21,18 +21,34 @@ /// @author Jan Steemann //////////////////////////////////////////////////////////////////////////////// -#include "Aql/SortCondition.h" +#include "SortCondition.h" #include "Aql/AstNode.h" +#include "Basics/Logger.h" using namespace arangodb::aql; +//////////////////////////////////////////////////////////////////////////////// +/// @brief whether or not an attribute is contained in a vector +//////////////////////////////////////////////////////////////////////////////// + +static bool IsContained (std::vector> const& attributes, + std::vector const& attribute) { + for (auto const& it : attributes) { + if (arangodb::basics::AttributeName::isIdentical(it, attribute, false)) { + return true; + } + } + + return false; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief create an empty condition //////////////////////////////////////////////////////////////////////////////// SortCondition::SortCondition() - : _expressions(), - _fields(), + : _fields(), + _constAttributes(), _unidirectional(false), _onlyAttributeAccess(false), _ascending(true) {} @@ -41,73 +57,22 @@ SortCondition::SortCondition() /// @brief create the sort condition //////////////////////////////////////////////////////////////////////////////// -SortCondition::SortCondition( - std::vector> const& expressions) - : _expressions(expressions), - _fields(), - _unidirectional(true), - _onlyAttributeAccess(true), - _ascending(true) { - size_t const n = _expressions.size(); - - for (size_t i = 0; i < n; ++i) { - if (_unidirectional && i > 0 && - _expressions[i].second != _expressions[i - 1].second) { - _unidirectional = false; - } - - bool handled = false; - auto node = _expressions[i].first; - - if (node != nullptr && node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - std::vector fieldNames; - while (node->type == NODE_TYPE_ATTRIBUTE_ACCESS) { - fieldNames.emplace_back( - arangodb::basics::AttributeName(node->getStringValue())); - node = node->getMember(0); - } - if (node->type == NODE_TYPE_REFERENCE) { - handled = true; - - _fields.emplace_back(std::make_pair( - static_cast(node->getData()), fieldNames)); - } - } - - if (!handled) { - _fields.emplace_back( - std::pair>()); - _onlyAttributeAccess = false; - } - } - - if (n == 0) { - _onlyAttributeAccess = false; - } -} - -//////////////////////////////////////////////////////////////////////////////// -/// @brief create the sort condition -//////////////////////////////////////////////////////////////////////////////// - SortCondition::SortCondition( std::vector> const& sorts, + std::vector> const& constAttributes, std::unordered_map const& variableDefinitions) - : _expressions(), + : _fields(), + _constAttributes(constAttributes), _unidirectional(true), _onlyAttributeAccess(true), _ascending(true) { + + bool foundDirection = false; + size_t const n = sorts.size(); for (size_t i = 0; i < n; ++i) { - if (_unidirectional && i > 0 && sorts[i].second != sorts[i - 1].second) { - _unidirectional = false; - } - if (i == 0) { - _ascending = sorts[i].second; - } - + bool isConst = false; // const attribute? bool handled = false; auto variableId = sorts[i].first; @@ -129,10 +94,30 @@ SortCondition::SortCondition( _fields.emplace_back(std::make_pair( static_cast(node->getData()), fieldNames)); + + for (auto const& it2 : constAttributes) { + if (it2 == fieldNames) { + // const attribute + isConst = true; + break; + } + } } } } + if (!isConst) { + // const attributes can be ignored for sorting + if (!foundDirection) { + // first attribute that we found + foundDirection = true; + _ascending = sorts[i].second; + } + else if (_unidirectional && sorts[i].second != _ascending) { + _unidirectional = false; + } + } + if (!handled) { _fields.emplace_back( std::pair> const& indexAttributes) const { - size_t numAttributes = 0; + size_t numCovered = 0; + size_t fieldsPosition = 0; + + // iterate over all fields of the index definition + size_t const n = indexAttributes.size(); - for (size_t i = 0; i < indexAttributes.size(); ++i) { - if (i >= _fields.size()) { + for (size_t i = 0; i < n; /* no hoisting */) { + if (fieldsPosition >= _fields.size()) { + // done break; } + + auto const& field = _fields[fieldsPosition]; - if (reference != _fields[i].first) { - break; + // ...and check if the field is present in the index definition too + if (reference == field.first && + arangodb::basics::AttributeName::isIdentical(field.second, indexAttributes[i], false)) { + // field match + ++fieldsPosition; + ++numCovered; + ++i; // next index field + continue; } - auto const& fieldNames = _fields[i].second; - if (fieldNames.size() != indexAttributes[i].size()) { - // different attribute path - break; + // no match + bool isConstant = false; + + if (IsContained(_constAttributes, indexAttributes[i])) { + // no field match, but a constant attribute + isConstant = true; + ++i; // next index field } - bool found = true; - for (size_t j = 0; j < indexAttributes[i].size(); ++j) { - if (indexAttributes[i][j].shouldExpand || - fieldNames[j] != indexAttributes[i][j]) { - // expanded attribute or different attribute - found = false; - break; + if (!isConstant) { + if (IsContained(indexAttributes, field.second) && + IsContained(_constAttributes, field.second)) { + // no field match, but a constant attribute + isConstant = true; + ++fieldsPosition; + ++numCovered; } } - - if (!found) { + + if (!isConstant) { break; } - - // same attribute - ++numAttributes; } - return numAttributes; + TRI_ASSERT(numCovered <= _fields.size()); + return numCovered; } diff --git a/arangod/Aql/SortCondition.h b/arangod/Aql/SortCondition.h index 7d35886614..6753c79658 100644 --- a/arangod/Aql/SortCondition.h +++ b/arangod/Aql/SortCondition.h @@ -47,13 +47,8 @@ class SortCondition { /// @brief create the sort condition ////////////////////////////////////////////////////////////////////////////// - explicit SortCondition(std::vector> const&); - - ////////////////////////////////////////////////////////////////////////////// - /// @brief create the sort condition - ////////////////////////////////////////////////////////////////////////////// - SortCondition(std::vector> const&, + std::vector> const&, std::unordered_map const&); ////////////////////////////////////////////////////////////////////////////// @@ -115,11 +110,6 @@ class SortCondition { std::vector> const&) const; private: - ////////////////////////////////////////////////////////////////////////////// - /// @brief sort expressions - ////////////////////////////////////////////////////////////////////////////// - - std::vector> _expressions; ////////////////////////////////////////////////////////////////////////////// /// @brief fields used in the sort conditions @@ -127,6 +117,12 @@ class SortCondition { std::vector>> _fields; + + ////////////////////////////////////////////////////////////////////////////// + /// @brief const attributes + ////////////////////////////////////////////////////////////////////////////// + + std::vector> const _constAttributes; ////////////////////////////////////////////////////////////////////////////// /// @brief whether or not the sort is unidirectional diff --git a/js/apps/system/_admin/aardvark/APP/frontend/js/modules/common/@arangodb/aql/explainer.js b/js/apps/system/_admin/aardvark/APP/frontend/js/modules/common/@arangodb/aql/explainer.js index ef833463ef..9e15c8263a 100644 --- a/js/apps/system/_admin/aardvark/APP/frontend/js/modules/common/@arangodb/aql/explainer.js +++ b/js/apps/system/_admin/aardvark/APP/frontend/js/modules/common/@arangodb/aql/explainer.js @@ -698,7 +698,7 @@ function processQuery (query, explain) { collectionVariables[node.outVariable.id] = node.collection; var types = [ ]; node.indexes.forEach(function (idx, i) { - var what = (idx.reverse ? "reverse " : "") + idx.type + " index scan"; + var what = (node.reverse ? "reverse " : "") + idx.type + " index scan"; if (types.length === 0 || what !== types[types.length - 1]) { types.push(what); } diff --git a/js/common/modules/@arangodb/aql/explainer.js b/js/common/modules/@arangodb/aql/explainer.js index b352c8ce1b..ca804aa733 100644 --- a/js/common/modules/@arangodb/aql/explainer.js +++ b/js/common/modules/@arangodb/aql/explainer.js @@ -697,7 +697,7 @@ function processQuery (query, explain) { collectionVariables[node.outVariable.id] = node.collection; var types = [ ]; node.indexes.forEach(function (idx, i) { - var what = (idx.reverse ? "reverse " : "") + idx.type + " index scan"; + var what = (node.reverse ? "reverse " : "") + idx.type + " index scan"; if (types.length === 0 || what !== types[types.length - 1]) { types.push(what); } diff --git a/js/server/tests/aql/aql-optimizer-indexes-sort.js b/js/server/tests/aql/aql-optimizer-indexes-sort.js index ca897a151d..714cc1b7a2 100644 --- a/js/server/tests/aql/aql-optimizer-indexes-sort.js +++ b/js/server/tests/aql/aql-optimizer-indexes-sort.js @@ -31,7 +31,6 @@ var jsunity = require("jsunity"); var db = require("@arangodb").db; - //////////////////////////////////////////////////////////////////////////////// /// @brief test suite //////////////////////////////////////////////////////////////////////////////// @@ -309,9 +308,7 @@ function optimizerIndexesSortTestSuite () { //////////////////////////////////////////////////////////////////////////////// testCannotUseHashIndexForSortIfConstRanges : function () { - AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: i.value, value3: i.value } IN " + c.name()); - - c.ensureHashIndex("value2", "value3"); + c.ensureIndex({ type: "hash", fields: [ "value2", "value3" ] }); var queries = [ [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value2 ASC, i.value3 ASC RETURN i.value2", false ], @@ -339,6 +336,52 @@ function optimizerIndexesSortTestSuite () { }); }, + //////////////////////////////////////////////////////////////////////////////// + /// @brief test index usage + //////////////////////////////////////////////////////////////////////////////// + + testCannotUseHashIndexForSortIfConstRangesMore : function () { + c.ensureIndex({ type: "hash", fields: [ "value2", "value3", "value4" ] }); + + var queries = [ + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 ASC, i.value4 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 DESC, i.value4 DESC RETURN i.value2", false ], + + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value4 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value4 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value3 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value3 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value3 ASC, i.value4 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value3 DESC, i.value4 DESC RETURN i.value2" ,false ], + + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value3 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value3 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value3 ASC, i.value4 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value3 DESC, i.value4 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value4 ASC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value4 DESC RETURN i.value2", false ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value2 ASC, i.value3 ASC, i.value4 ASC RETURN i.value2", true ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value2 DESC, i.value3 DESC, i.value4 DESC RETURN i.value2", true ], + [ "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 && i.value4 == 2 SORT i.value2 ASC, i.value3 ASC, i.value4 DESC RETURN i.value2", true ] + ]; + + queries.forEach(function(query) { + var plan = AQL_EXPLAIN(query[0]).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + if (query[1]) { + assertEqual(-1, nodeTypes.indexOf("SortNode"), query[0]); + } + else { + assertNotEqual(-1, nodeTypes.indexOf("SortNode"), query[0]); + } + }); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief test index usage //////////////////////////////////////////////////////////////////////////////// @@ -437,17 +480,22 @@ function optimizerIndexesSortTestSuite () { /// @brief test index usage //////////////////////////////////////////////////////////////////////////////// - testCannotUseSkiplistIndexForSortIfConstRanges : function () { + testCanUseSkiplistIndexForSortIfConstRanges : function () { AQL_EXECUTE("FOR i IN " + c.name() + " UPDATE i WITH { value2: i.value, value3: i.value, value4: i.value } IN " + c.name()); c.ensureSkiplist("value2", "value3", "value4"); var queries = [ + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 DESC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC, i.value3 ASC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC, i.value3 DESC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 ASC RETURN i.value2", "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 DESC RETURN i.value2", "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 ASC, i.value4 ASC RETURN i.value2", "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 DESC, i.value4 DESC RETURN i.value2", - + "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value4 ASC RETURN i.value2", "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value4 DESC RETURN i.value2", "FOR i IN " + c.name() + " FILTER i.value2 == 2 && i.value3 == 2 SORT i.value3 ASC RETURN i.value2", @@ -470,10 +518,33 @@ function optimizerIndexesSortTestSuite () { }); assertNotEqual(-1, nodeTypes.indexOf("IndexNode"), query); - assertNotEqual(-1, nodeTypes.indexOf("SortNode"), query); + assertEqual(-1, nodeTypes.indexOf("SortNode"), query); }); }, +//////////////////////////////////////////////////////////////////////////////// +/// @brief test index usage +//////////////////////////////////////////////////////////////////////////////// + + testCannotUseSkiplistIndexForSortIfConstRanges : function () { + c.ensureSkiplist("value2", "value3", "value4"); + + var queries = [ + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value3 ASC, i.value4 DESC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC, i.value4 ASC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC, i.value3 ASC, i.value4 DESC RETURN i.value2", + "FOR i IN " + c.name() + " FILTER i.value2 == 2 SORT i.value2 ASC, i.value3 ASC, i.value4 DESC RETURN i.value2" + ]; + + queries.forEach(function(query) { + var plan = AQL_EXPLAIN(query).plan; + var nodeTypes = plan.nodes.map(function(node) { + return node.type; + }); + + assertNotEqual(-1, nodeTypes.indexOf("SortNode"), query); + }); + }, //////////////////////////////////////////////////////////////////////////////// /// @brief test index usage @@ -750,6 +821,7 @@ function optimizerIndexesSortTestSuite () { "FOR i IN " + c.name() + " FILTER i.value2 == 1 && i.value3 == null SORT i.value2 RETURN i.value2", ]; + queries.forEach(function(query) { var plan = AQL_EXPLAIN(query).plan; var nodeTypes = plan.nodes.map(function(node) { diff --git a/js/server/tests/aql/aql-optimizer-rule-sort-in-values.js b/js/server/tests/aql/aql-optimizer-rule-sort-in-values.js index 211bb220d0..a1d0009cbf 100644 --- a/js/server/tests/aql/aql-optimizer-rule-sort-in-values.js +++ b/js/server/tests/aql/aql-optimizer-rule-sort-in-values.js @@ -203,12 +203,15 @@ function optimizerRuleTestSuite () { //////////////////////////////////////////////////////////////////////////////// testResults : function () { - var numbers = [ ], strings = [ ], reversed = [ ]; - for (var i = 1; i <= 100; ++i) { + var groups = [ ], numbers = [ ], strings = [ ], reversed = [ ], i; + for (i = 1; i <= 100; ++i) { numbers.push(i); strings.push("test" + i); reversed.push("test" + (101 - i)); } + for (i = 0; i < 10; ++i) { + groups.push(i); + } var queries = [ [ "LET values = NOOPT(RANGE(1, 100)) FOR i IN 1..100 FILTER i IN values RETURN i", numbers ], @@ -223,7 +226,8 @@ function optimizerRuleTestSuite () { [ "LET values = NOOPT(" + JSON.stringify(reversed) + ") FOR i IN 1..100 FILTER CONCAT('test', i) IN values RETURN i", numbers ], [ "LET values = NOOPT(" + JSON.stringify(reversed) + ") FOR i IN 1..100 FILTER CONCAT('test', i) NOT IN values RETURN i", [ ] ], [ "LET values = NOOPT(" + JSON.stringify(reversed) + ") FOR i IN 1..100 FILTER i IN values RETURN i", [ ] ], - [ "LET values = NOOPT(" + JSON.stringify(reversed) + ") FOR i IN 1..100 FILTER i NOT IN values RETURN i", numbers ] + [ "LET values = NOOPT(" + JSON.stringify(reversed) + ") FOR i IN 1..100 FILTER i NOT IN values RETURN i", numbers ], + [ "LET values = NOOPT(" + JSON.stringify(numbers) + ") FOR i IN 1..100 FILTER i IN values COLLECT group = i % 10 RETURN group", groups ] ]; queries.forEach(function(query) { diff --git a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js index 5f7a4763ca..c0fb498575 100644 --- a/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js +++ b/js/server/tests/aql/aql-optimizer-rule-use-index-for-sort.js @@ -151,6 +151,57 @@ function optimizerRuleTestSuite() { skiplist = null; }, + testRuleOptimizeWhenEqComparison : function () { + // skiplist: a, b + // skiplist: d + // hash: c + // hash: y,z + skiplist.ensureIndex({ type: "hash", fields: [ "y", "z" ], unique: false }); + + var queries = [ + [ "FOR v IN " + colName + " FILTER v.u == 1 SORT v.u RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.c == 1 SORT v.c RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.c == 1 SORT v.z RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.c == 1 SORT v.f RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.y == 1 SORT v.z RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.y == 1 SORT v.y RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.z == 1 SORT v.y RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.z == 1 SORT v.z RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.y == 1 && v.z == 1 SORT v.y RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.y == 1 && v.z == 1 SORT v.z RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.y == 1 && v.z == 1 SORT v.y, v.z RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.y == 1 && v.z == 1 SORT v.z, v.y RETURN 1", false ], // not supported yet + [ "FOR v IN " + colName + " FILTER v.d == 1 SORT v.d RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.d == 1 && v.e == 1 SORT v.d RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.d == 1 SORT v.e RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a, v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a, v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 SORT v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.b == 1 SORT v.a, v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.b == 1 SORT v.b RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.b == 1 SORT v.b, v.a RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.b, v.a RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.b RETURN 1", true ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.a, v.c RETURN 1", false ], + [ "FOR v IN " + colName + " FILTER v.a == 1 && v.b == 1 SORT v.b, v.a RETURN 1", false ] + ]; + + queries.forEach(function(query) { + var result = AQL_EXPLAIN(query[0]); + if (query[1]) { + assertNotEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]); + hasNoSortNode(result); + } + else { + assertEqual(-1, removeAlwaysOnClusterRules(result.plan.rules).indexOf(ruleName), query[0]); + hasSortNode(result); + } + }); + }, + //////////////////////////////////////////////////////////////////////////////// /// @brief test that rule has no effect //////////////////////////////////////////////////////////////////////////////// @@ -162,7 +213,7 @@ function optimizerRuleTestSuite() { ["FOR v IN " + colName + " SORT v.c RETURN [v.a, v.b]", true], ["FOR v IN " + colName + " SORT v.b, v.a RETURN [v.a]", true], ["FOR v IN " + colName + " SORT v.c RETURN [v.a, v.b]", true], - ["FOR v IN " + colName + " SORT v.a + 1 RETURN [v.a]", false],// this will throw... + ["FOR v IN " + colName + " SORT v.a + 1 RETURN [v.a]", false], ["FOR v IN " + colName + " SORT CONCAT(TO_STRING(v.a), \"lol\") RETURN [v.a]", true], // TODO: limit blocks sort atm. ["FOR v IN " + colName + " FILTER v.a > 2 LIMIT 3 SORT v.a RETURN [v.a]", false], @@ -371,7 +422,7 @@ function optimizerRuleTestSuite() { hasIndexNode(XPresult); // -> combined use-index-for-sort and use-index-range - // use-index-range superseedes use-index-for-sort + // use-index-range supersedes use-index-for-sort QResults[2] = AQL_EXECUTE(query, { }, paramIndexFromSort_IndexRange).json; XPresult = AQL_EXPLAIN(query, { }, paramIndexFromSort_IndexRange); @@ -417,7 +468,8 @@ function optimizerRuleTestSuite() { /// @brief test in detail that an index range fullfills everything the sort does, // and thus the sort is removed. //////////////////////////////////////////////////////////////////////////////// - testRangeSuperseedsSort: function () { + + testRangeSupersedesSort: function () { var query = "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a RETURN [v.a, v.b, v.c]"; @@ -496,7 +548,8 @@ function optimizerRuleTestSuite() { /// @brief test in detail that an index range fullfills everything the sort does, // and thus the sort is removed; multi-dimensional indexes are utilized. //////////////////////////////////////////////////////////////////////////////// - testRangeSuperseedsSort2: function () { + + testRangeSupersedesSort2: function () { var query = "FOR v IN " + colName + " FILTER v.a == 1 SORT v.a, v.b RETURN [v.a, v.b, v.c]"; var XPresult;