issue #545

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
 
 

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;
@@ -2449,6 +2480,12 @@ static TRI_vector_pointer_t* ProcessNode (TRI_aql_context_t* const context,
       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);
 
       if (lhs->_type == TRI_AQL_NODE_REFERENCE || lhs->_type == TRI_AQL_NODE_ATTRIBUTE_ACCESS || lhs->_type == TRI_AQL_NODE_FCALL) {

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);
+    },
 
   };
 }

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);
     }
 
   };