diff --git a/CHANGELOG b/CHANGELOG index f8234827f3..bf40939b74 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,8 @@ v1.4 ------ +* issue #547: Javascript error in the web interface + * issue #550: Make AQL graph functions support key in addition to id * issue #526: Unable to escape when an errorneous command is entered into the js shell @@ -15,6 +17,8 @@ v1.4 v1.3.2 (2013-XX-XX) ------------------- +* issue #545: AQL FILTER unnecessary (?) loop + * issue #549: wrong return code with --daemon diff --git a/arangod/Ahuacatl/ahuacatl-access-optimiser.c b/arangod/Ahuacatl/ahuacatl-access-optimiser.c index 09aaf3359d..2af5a0d8c4 100644 --- a/arangod/Ahuacatl/ahuacatl-access-optimiser.c +++ b/arangod/Ahuacatl/ahuacatl-access-optimiser.c @@ -2350,6 +2350,32 @@ static TRI_aql_attribute_name_t* GetAttributeName (TRI_aql_context_t* const cont return NULL; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief check if two attribute access nodes refer to the same base variable +/// e.g. FILTER a.x == a.y +//////////////////////////////////////////////////////////////////////////////// + +static bool IsSameAttributeAccess (const TRI_aql_node_t* const lhs, + const TRI_aql_node_t* const rhs) { + assert(lhs != NULL); + assert(rhs != NULL); + + if (lhs->_type == TRI_AQL_NODE_ATTRIBUTE_ACCESS && + rhs->_type == TRI_AQL_NODE_ATTRIBUTE_ACCESS) { + + TRI_aql_node_t* lNode = TRI_AQL_NODE_MEMBER(lhs, 0); + TRI_aql_node_t* rNode = TRI_AQL_NODE_MEMBER(rhs, 0); + + if (lNode->_type == TRI_AQL_NODE_REFERENCE && + rNode->_type == TRI_AQL_NODE_REFERENCE && + TRI_EqualString(TRI_AQL_NODE_STRING(lNode), TRI_AQL_NODE_STRING(rNode))) { + return true; + } + } + + return false; +} + //////////////////////////////////////////////////////////////////////////////// /// @brief process a condition node and recurse into its subnodes /// @@ -2437,6 +2463,11 @@ static TRI_vector_pointer_t* ProcessNode (TRI_aql_context_t* const context, node2 = rhs; operator = node->_type; + if (IsSameAttributeAccess(lhs, rhs)) { + // we must not optimise something like FILTER a.x == a.x + return NULL; + } + if (rhs->_type == TRI_AQL_NODE_REFERENCE || rhs->_type == TRI_AQL_NODE_ATTRIBUTE_ACCESS || rhs->_type == TRI_AQL_NODE_FCALL) { // expression of type reference|attribute access|fcall operator reference|attribute access|fcall useBoth = true; @@ -2448,6 +2479,12 @@ static TRI_vector_pointer_t* ProcessNode (TRI_aql_context_t* const context, node1 = rhs; node2 = lhs; operator = TRI_ReverseOperatorRelationalAql(node->_type); + + if (IsSameAttributeAccess(lhs, rhs)) { + // we must not optimise something like FILTER a.x == a.x + return NULL; + } + TRI_ASSERT_MAINTAINER(operator != TRI_AQL_NODE_NOP); diff --git a/js/server/tests/ahuacatl-hash.js b/js/server/tests/ahuacatl-hash.js index f36ff11e63..b9ca96304f 100644 --- a/js/server/tests/ahuacatl-hash.js +++ b/js/server/tests/ahuacatl-hash.js @@ -27,6 +27,7 @@ var internal = require("internal"); var jsunity = require("jsunity"); +var EXPLAIN = internal.AQL_EXPLAIN; var QUERY = internal.AQL_QUERY; //////////////////////////////////////////////////////////////////////////////// @@ -46,6 +47,14 @@ function ahuacatlHashTestSuite () { return cursor; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief explain a given query +//////////////////////////////////////////////////////////////////////////////// + + function explainQuery (query, bindVars) { + return EXPLAIN(query, bindVars); + } + //////////////////////////////////////////////////////////////////////////////// /// @brief execute a given query and return the results as an array //////////////////////////////////////////////////////////////////////////////// @@ -101,10 +110,15 @@ function ahuacatlHashTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingle1 : function () { + var query = "FOR v IN " + hash.name() + " FILTER v.c == 1 SORT v.b RETURN [ v.b ]"; var expected = [ [ 1 ], [ 2 ], [ 3 ], [ 4 ], [ 5 ] ]; - var actual = getQueryResults("FOR v IN " + hash.name() + " FILTER v.c == 1 SORT v.b RETURN [ v.b ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -112,10 +126,15 @@ function ahuacatlHashTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingle2 : function () { + var query = "FOR v IN " + hash.name() + " FILTER 1 == v.c SORT v.b RETURN [ v.b ]"; var expected = [ [ 1 ], [ 2 ], [ 3 ], [ 4 ], [ 5 ] ]; - var actual = getQueryResults("FOR v IN " + hash.name() + " FILTER 1 == v.c SORT v.b RETURN [ v.b ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -147,10 +166,15 @@ function ahuacatlHashTestSuite () { testEqMultiAll1 : function () { for (var i = 1; i <= 5; ++i) { for (var j = 1; j <=5; ++j) { + var query = "FOR v IN " + hash.name() + " FILTER v.a == @a && v.b == @b RETURN [ v.a, v.b ]"; var expected = [ [ i, j ] ]; - var actual = getQueryResults("FOR v IN " + hash.name() + " FILTER v.a == @a && v.b == @b RETURN [ v.a, v.b ]", { "a" : i, "b" : j }); + var actual = getQueryResults(query, { "a": i, "b": j }); assertEqual(expected, actual); + + var explain = explainQuery(query, { "a": i, "b": j }); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); } } }, @@ -162,10 +186,15 @@ function ahuacatlHashTestSuite () { testEqMultiAll2 : function () { for (var i = 1; i <= 5; ++i) { for (var j = 1; j <=5; ++j) { + var query = "FOR v IN " + hash.name() + " FILTER @a == v.a && @b == v.b RETURN [ v.a, v.b ]"; var expected = [ [ i, j ] ]; - var actual = getQueryResults("FOR v IN " + hash.name() + " FILTER @a == v.a && @b == v.b RETURN [ v.a, v.b ]", { "a" : i, "b" : j }); + var actual = getQueryResults(query, { "a": i, "b": j }); assertEqual(expected, actual); + + var explain = explainQuery(query, { "a": i, "b": j }); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); } } }, @@ -175,10 +204,16 @@ function ahuacatlHashTestSuite () { //////////////////////////////////////////////////////////////////////////////// testRefConst1 : function () { + var query = "LET x = 4 FOR v IN " + hash.name() + " FILTER v.c == x SORT v.b RETURN [ v.b, v.c ]"; var expected = [ [ 1, 4 ], [ 2, 4 ], [ 3, 4 ], [ 4, 4 ], [ 5, 4 ] ]; - var actual = getQueryResults("LET x = 4 FOR v IN " + hash.name() + " FILTER v.c == x SORT v.b RETURN [ v.b, v.c ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("let", explain[0].type); + assertEqual("for", explain[1].type); + assertEqual("index", explain[1].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -255,7 +290,103 @@ function ahuacatlHashTestSuite () { assertEqual(expected, actual); } - } + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access +//////////////////////////////////////////////////////////////////////////////// + + testRefMulti3 : function () { + var query = "FOR v1 IN " + hash.name() + " FILTER @a == v1.a && @b == v1.b RETURN [ v1.a, v1.b ]"; + var expected = [ [ 2, 3 ] ]; + var actual = getQueryResults(query, { "a": 2, "b": 3 }); + + assertEqual(expected, actual); + + var explain = explainQuery(query, { "a": 2, "b": 3 }); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefFilterSame1 : function () { + var query = "FOR a IN " + hash.name() + " FILTER a.a == a.a SORT a.a RETURN a.a"; + var expected = [ 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5 ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefFilterSame2 : function () { + var query = "FOR a IN " + hash.name() + " FILTER a.a == a.c SORT a.a RETURN a.a"; + var expected = [ 1, 1, 1, 1, 1, 2, 2, 2, 2, 2, 3, 3, 3, 3, 3, 4, 4, 4, 4, 4, 5, 5, 5, 5, 5 ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefNon1 : function () { + var query = "FOR a IN " + hash.name() + " FILTER a.a == 1 RETURN a.a"; + var expected = [ 1, 1, 1, 1, 1 ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefNon2 : function () { + var query = "FOR a IN " + hash.name() + " FILTER a.d == a.a SORT a.a RETURN a.a"; + var expected = [ ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefNon3 : function () { + var query = "FOR a IN " + hash.name() + " FILTER a.d == 1 SORT a.a RETURN a.a"; + var expected = [ ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, }; } diff --git a/js/server/tests/ahuacatl-skiplist.js b/js/server/tests/ahuacatl-skiplist.js index 34023d7ceb..b4e9657aa9 100644 --- a/js/server/tests/ahuacatl-skiplist.js +++ b/js/server/tests/ahuacatl-skiplist.js @@ -27,6 +27,7 @@ var internal = require("internal"); var jsunity = require("jsunity"); +var EXPLAIN = internal.AQL_EXPLAIN; var QUERY = internal.AQL_QUERY; //////////////////////////////////////////////////////////////////////////////// @@ -46,6 +47,14 @@ function ahuacatlSkiplistTestSuite () { return cursor; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief explain a given query +//////////////////////////////////////////////////////////////////////////////// + + function explainQuery (query) { + return EXPLAIN(query); + } + //////////////////////////////////////////////////////////////////////////////// /// @brief execute a given query and return the results as an array //////////////////////////////////////////////////////////////////////////////// @@ -99,10 +108,15 @@ function ahuacatlSkiplistTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingleVoid1 : function () { + var query = "FOR v IN " + skiplist.name() + " FILTER v.a == 99 RETURN v"; var expected = [ ]; - var actual = getQueryResults("FOR v IN " + skiplist.name() + " FILTER v.a == 99 RETURN v"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -110,10 +124,15 @@ function ahuacatlSkiplistTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingleVoid2 : function () { + var query = "FOR v IN " + skiplist.name() + " FILTER 99 == v.a RETURN v"; var expected = [ ]; - var actual = getQueryResults("FOR v IN " + skiplist.name() + " FILTER 99 == v.a RETURN v"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -121,10 +140,15 @@ function ahuacatlSkiplistTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingle1 : function () { + var query = "FOR v IN " + skiplist.name() + " FILTER v.a == 1 SORT v.b RETURN [ v.a, v.b ]"; var expected = [ [ 1, 1 ], [ 1, 2 ], [ 1, 3 ], [ 1, 4 ], [ 1, 5 ] ]; - var actual = getQueryResults("FOR v IN " + skiplist.name() + " FILTER v.a == 1 SORT v.b RETURN [ v.a, v.b ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -132,10 +156,15 @@ function ahuacatlSkiplistTestSuite () { //////////////////////////////////////////////////////////////////////////////// testEqSingle2 : function () { + var query = "FOR v IN " + skiplist.name() + " FILTER 1 == v.a SORT v.b RETURN [ v.a, v.b ]"; var expected = [ [ 1, 1 ], [ 1, 2 ], [ 1, 3 ], [ 1, 4 ], [ 1, 5 ] ]; - var actual = getQueryResults("FOR v IN " + skiplist.name() + " FILTER 1 == v.a SORT v.b RETURN [ v.a, v.b ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -187,10 +216,15 @@ function ahuacatlSkiplistTestSuite () { //////////////////////////////////////////////////////////////////////////////// testGeSingle1 : function () { + var query = "FOR v IN " + skiplist.name() + " FILTER v.a >= 5 SORT v.b RETURN [ v.a, v.b ]"; var expected = [ [ 5, 1 ], [ 5, 2 ], [ 5, 3 ], [ 5, 4 ], [ 5, 5 ] ]; - var actual = getQueryResults("FOR v IN " + skiplist.name() + " FILTER v.a >= 5 SORT v.b RETURN [ v.a, v.b ]"); + var actual = getQueryResults(query); assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("index", explain[0].expression.extra.accessType); }, //////////////////////////////////////////////////////////////////////////////// @@ -1012,6 +1046,49 @@ function ahuacatlSkiplistTestSuite () { var actual = getQueryResults("FOR v1 IN " + skiplist.name() + " FOR v2 IN " + skiplist.name() + " FILTER 1 == v1.a && v1.a == v2.a && 1 == v1.b SORT v1.a, v2.b RETURN [ v1.a, v2.b ]"); assertEqual(expected, actual); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefFilterSame : function () { + skiplist.ensureSkiplist("c"); + skiplist.ensureSkiplist("d"); + + skiplist.truncate(); + + for (var i = 1; i <= 5; ++i) { + for (var j = 1; j <= 5; ++j) { + skiplist.save({ "c" : i, "d": j }); + } + } + + var query = "FOR a IN " + skiplist.name() + " FILTER a.c == a.d SORT a.c RETURN [ a.c, a.d ]"; + var expected = [ [ 1, 1 ], [ 2, 2 ], [ 3, 3 ], [ 4, 4 ], [ 5, 5 ] ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); + }, + +//////////////////////////////////////////////////////////////////////////////// +/// @brief test ref access with filters on the same attribute +//////////////////////////////////////////////////////////////////////////////// + + testRefFilterNonExisting : function () { + var query = "FOR a IN " + skiplist.name() + " FILTER a.e == a.f SORT a.a, a.b RETURN [ a.a, a.b ]"; + var expected = [ [ 1, 1 ], [ 1, 2 ], [ 1, 3 ], [ 1, 4 ], [ 1, 5 ], [ 2, 1 ], [ 2, 2 ], [ 2, 3 ], [ 2, 4 ], [ 2, 5 ], [ 3, 1 ], [ 3, 2 ], [ 3, 3 ], [ 3, 4 ], [ 3, 5 ], [ 4, 1 ], [ 4, 2 ], [ 4, 3 ], [ 4, 4 ], [ 4, 5 ], [ 5, 1 ], [ 5, 2 ], [ 5, 3 ], [ 5, 4 ], [ 5, 5 ] ]; + var actual = getQueryResults(query); + + assertEqual(expected, actual); + + var explain = explainQuery(query); + assertEqual("for", explain[0].type); + assertEqual("all", explain[0].expression.extra.accessType); } };