fixed array range accesses

arangod/Aql/Expression.cpp

@@ -278,9 +278,91 @@ void Expression::invalidate () {
 }
 
 // -----------------------------------------------------------------------------
-// --SECTION--                                                 private functions
+// --SECTION--                                                   private methods
 // -----------------------------------------------------------------------------
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief extracts a range from an array
+////////////////////////////////////////////////////////////////////////////////
+
+AqlValue Expression::extractRange (AqlValue const& result, 
+                                   AqlValue const& indexResult, 
+                                   TRI_document_collection_t const* collection, 
+                                   triagens::arango::AqlTransaction* trx) const {
+  TRI_ASSERT(result.isArray());
+
+  size_t const length = result.arraySize();
+
+  if (length == 0) {
+    // cannot extract anything from an empty array
+    return AqlValue(new Json(Json::Array));
+  }
+
+  int64_t low  = indexResult.getRange()->_low; 
+  int64_t high = indexResult.getRange()->_high; 
+
+  // negative positions are translated into their positive counterparts
+  if (low < 0) {
+    low = static_cast<int64_t>(length) + low;
+  }
+  if (high < 0) {
+    high = static_cast<int64_t>(length) + high;
+  }
+   
+  std::unique_ptr<Json> array(new Json(Json::Array));
+
+  if (low <= high) {
+    // forward iteration
+    // adjust bounds
+    if (low < 0) {
+      low = 0;
+    }
+    if (high >= static_cast<int64_t>(length)) {
+      high = static_cast<int64_t>(length);
+    }
+    else {
+      // increase by one so we include the last specified element
+      ++high;
+    }
+    if (low <= high) {
+      array->reserve(static_cast<size_t>(high - low));
+      // forward iteration 
+      for (int64_t position = low; position < high; ++position) {
+        auto j = result.extractArrayMember(trx, collection, position, true);
+        array->add(j);
+      }
+    }
+  }
+  else {
+    // backward iteration
+    // swap low and high
+    int64_t tmp = low;
+    low = high;
+    high = tmp;
+
+    // backward iteration
+    if (low < 0) {
+      low = 0;
+    }
+    if (high < 0) {
+      high = 0;
+    }
+    else if (high >= static_cast<int64_t>(length)) {
+      high = static_cast<int64_t>(length) - 1;
+    }
+
+    if (high >= low) { 
+      array->reserve(static_cast<size_t>(high - low + 1));
+      for (int64_t position = high; position >= low; --position) {
+        auto j = result.extractArrayMember(trx, collection, position, true);
+        array->add(j);
+      }
+    }
+  }
+       
+  return AqlValue(array.release());
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief find a value in an AQL list node
 /// this performs either a binary search (if the node is sorted) or a
@@ -521,78 +603,14 @@ AqlValue Expression::executeSimpleExpression (AstNode const* node,
         }
       }
       else if (indexResult.isRange()) { 
-        size_t const length = result.arraySize();
-
-        if (length == 0) {
-          // cannot extract anything from an empty array
-          result.destroy();
-          return AqlValue(new Json(Json::Array));
-        }
-
-        int64_t low  = indexResult.getRange()->_low; 
-        int64_t high = indexResult.getRange()->_high; 
-        indexResult.destroy();
-
-        // negative positions are translated into their positive counterparts
-        if (low < 0) {
-          low = static_cast<int64_t>(length) + low;
-        }
-        if (high < 0) {
-          high = static_cast<int64_t>(length) + high;
-        }
-   
         try {
-          std::unique_ptr<Json> array(new Json(Json::Array));
+          auto range = extractRange(result, indexResult, myCollection, trx);
-
+          indexResult.destroy();
-          if (low <= high) {
-            // adjust bounds
-            if (low < 0) {
-              low = 0;
-            }
-            if (high >= static_cast<int64_t>(length)) {
-              high = static_cast<int64_t>(length);
-            }
-            else {
-              // increase by one so we include the last specified element
-              ++high;
-            }
-            if (low <= high) {
-              array->reserve(static_cast<size_t>(high - low));
-              // forward iteration 
-              for (int64_t position = low; position < high; ++position) {
-                auto j = result.extractArrayMember(trx, myCollection, position, true);
-                array->add(j);
-              }
-            }
-          }
-          else {
-            // swap
-            int64_t tmp = low;
-            low = high;
-            high = tmp;
-
-            // backward iteration
-            if (low < 0) {
-              low = 0;
-            }
-            if (high < 0) {
-              high = 0;
-            }
-            else if (high >= static_cast<int64_t>(length)) {
-              high = static_cast<int64_t>(length) - 1;
-            }
-
-            array->reserve(static_cast<size_t>(high - low + 1));
-            for (int64_t position = high; position >= low; --position) {
-              auto j = result.extractArrayMember(trx, myCollection, position, true);
-              array->add(j);
-            }
-          }
           result.destroy();
-         
+          return range;
-          return AqlValue(array.release());
         }
         catch (...) {
+          indexResult.destroy();
           result.destroy();
           throw;
         }

arangod/Aql/Expression.h

@@ -291,11 +291,20 @@ namespace triagens {
         void invalidate ();
 
 // -----------------------------------------------------------------------------
-// --SECTION--                                                 private functions
+// --SECTION--                                                   private methods
 // -----------------------------------------------------------------------------
 
       private:
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief extracts a range from an array
+////////////////////////////////////////////////////////////////////////////////
+
+        AqlValue extractRange (AqlValue const&, 
+                               AqlValue const&, 
+                               TRI_document_collection_t const*, 
+                               triagens::arango::AqlTransaction*) const;
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief find a value in an array
 ////////////////////////////////////////////////////////////////////////////////

js/server/modules/org/arangodb/aql.js

@@ -994,7 +994,7 @@ function GET_RANGE (value, low, high) {
   'use strict';
 
   if (TYPEWEIGHT(value) !== TYPEWEIGHT_ARRAY) {
-    return [ ];
+    return null;
   }
 
   low  = parseInt(low, 10);
@@ -1039,10 +1039,12 @@ function GET_RANGE (value, low, high) {
     else if (high >= value.length) {
       high = value.length - 1;
     }
+    if (high >= low) {
       for (position = high; position >= low; --position) {
         result.push(value[position]);
       }
     }
+  }
 
   return result;
 }

js/server/tests/aql-array-access.js

@@ -131,6 +131,24 @@ function arrayAccessTestSuite () {
       assertEqual([ null ], result);
     },
 
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test non-array access
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8NonArrayRange1 : function () {
+      var result = AQL_EXECUTE("RETURN NOOPT(V8({ foo: 'bar' }))[0..1]").json;
+      assertEqual([ null ], result);
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test non-array access
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8NonArrayRange2 : function () {
+      var result = AQL_EXECUTE("RETURN NOOPT(V8({ foo: 'bar' }))[-2..-1]").json;
+      assertEqual([ null ], result);
+    },
+
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief test subquery result
 ////////////////////////////////////////////////////////////////////////////////
@@ -143,7 +161,7 @@ function arrayAccessTestSuite () {
     },
 
 ////////////////////////////////////////////////////////////////////////////////
-/// @brief test subquery result
+/// @brief test range result
 ////////////////////////////////////////////////////////////////////////////////
 
     testSubqueryResultRangeForward1 : function () {
@@ -158,7 +176,7 @@ function arrayAccessTestSuite () {
     },
 
 ////////////////////////////////////////////////////////////////////////////////
-/// @brief test subquery result
+/// @brief test range result
 ////////////////////////////////////////////////////////////////////////////////
 
     testSubqueryResultRangeForward2 : function () {
@@ -170,6 +188,96 @@ function arrayAccessTestSuite () {
         }
         assertEqual([ expected ], result);
       }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testSubqueryResultRangeBackward1 : function () {
+      for (var to = 0; to < 10; ++to) {
+        var result = AQL_EXECUTE("RETURN (FOR value IN @values RETURN value)[10.." + to + "]", { values: values }).json;
+        var expected = [ ];
+        for (var i = Math.min(10, values.length - 1); i >= to; --i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testSubqueryResultRangeBackward2 : function () {
+      for (var from = 0; from < 10; ++from) {
+        var result = AQL_EXECUTE("RETURN (FOR value IN @values RETURN value)[" + from + "..0]", { values: values }).json;
+        var expected = [ ];
+        for (var i = Math.min(from, values.length - 1); i >= 0; --i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8SubqueryResultRangeForward1 : function () {
+      for (var to = 0; to < 10; ++to) {
+        var result = AQL_EXECUTE("RETURN NOOPT(V8(FOR value IN @values RETURN value))[0.." + to + "]", { values: values }).json;
+        var expected = [ ];
+        for (var i = 0; i < Math.min(to + 1, values.length); ++i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8SubqueryResultRangeForward2 : function () {
+      for (var from = 0; from < 10; ++from) {
+        var result = AQL_EXECUTE("RETURN NOOPT(V8(FOR value IN @values RETURN value))[" + from + "..99]", { values: values }).json;
+        var expected = [ ];
+        for (var i = Math.min(from, values.length); i < values.length; ++i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8SubqueryResultRangeBackward1 : function () {
+      for (var to = 0; to < 10; ++to) {
+        var result = AQL_EXECUTE("RETURN NOOPT(V8(FOR value IN @values RETURN value))[10.." + to + "]", { values: values }).json;
+        var expected = [ ];
+        for (var i = Math.min(10, values.length - 1); i >= to; --i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
+    },
+
+////////////////////////////////////////////////////////////////////////////////
+/// @brief test range result
+////////////////////////////////////////////////////////////////////////////////
+
+    testV8SubqueryResultRangeBackward2 : function () {
+      for (var from = 0; from < 10; ++from) {
+        var result = AQL_EXECUTE("RETURN NOOPT(V8(FOR value IN @values RETURN value))[" + from + "..0]", { values: values }).json;
+        var expected = [ ];
+        for (var i = Math.min(from, values.length - 1); i >= 0; --i) {
+          expected.push(values[i]);
+        }
+        assertEqual([ expected ], result);
+      }
     }
 
   };