Replaced IndexOperators using TRI_json_t by Operators using VPack.

arangod/IndexOperators/index-operator.cpp

@@ -26,9 +26,7 @@
 #include "VocBase/VocShaper.h"
 
 TRI_relation_index_operator_t::~TRI_relation_index_operator_t() {
-  if (_parameters != nullptr) {
+  // _parameters fill automatically free
-    TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, _parameters);
-  }
   if (_fields != nullptr) {
     // _fields contains _numFields shapedJson objects
     for (size_t i = 0; i < _numFields; ++i) {
@@ -47,7 +45,7 @@ TRI_relation_index_operator_t::~TRI_relation_index_operator_t() {
 
 TRI_index_operator_t* TRI_CreateIndexOperator(
     TRI_index_operator_type_e operatorType, TRI_index_operator_t* leftOperand,
-    TRI_index_operator_t* rightOperand, TRI_json_t* parameters,
+    TRI_index_operator_t* rightOperand, std::shared_ptr<VPackBuilder> parameters,
     VocShaper* shaper, size_t numFields) {
   switch (operatorType) {
     case TRI_AND_INDEX_OPERATOR: {
@@ -68,13 +66,3 @@ TRI_index_operator_t* TRI_CreateIndexOperator(
     default: { return nullptr; }
   }  // end of switch statement
 }
-
-TRI_index_operator_t* TRI_CreateIndexOperator(
-    TRI_index_operator_type_e operatorType, TRI_index_operator_t* leftOperand,
-    TRI_index_operator_t* rightOperand, VPackSlice const& parameters,
-    VocShaper* shaper, size_t numFields) {
-  std::unique_ptr<TRI_json_t> tmp(
-      triagens::basics::VelocyPackHelper::velocyPackToJson(parameters));
-  return TRI_CreateIndexOperator(operatorType, leftOperand, rightOperand,
-                                 tmp.release(), shaper, numFields);
-}

arangod/IndexOperators/index-operator.h

@@ -31,7 +31,7 @@
 
 namespace arangodb {
   namespace velocypack {
-    class Slice;
+    class Builder;
   }
 }
 class VocShaper;
@@ -132,19 +132,23 @@ class TRI_logical_index_operator_t : public TRI_index_operator_t {
 
 class TRI_relation_index_operator_t : public TRI_index_operator_t {
  public:
-  TRI_json_t*
+  std::shared_ptr<arangodb::velocypack::Builder>
       _parameters;  // parameters with which this relation was called with
   TRI_shaped_json_t* _fields;  // actual data from the parameters converted from
                                // a json array to a shaped json array
   size_t _numFields;           // number of fields in the array above
 
   TRI_relation_index_operator_t(TRI_index_operator_type_e const type,
-                                VocShaper const* shaper, TRI_json_t* parameters,
+                                VocShaper const* shaper, std::shared_ptr<arangodb::velocypack::Builder> parameters,
                                 TRI_shaped_json_t* fields, size_t numFields)
       : TRI_index_operator_t(type, shaper),
         _parameters(parameters),
         _fields(fields),
-        _numFields(numFields) {}
+        _numFields(numFields) {
+          // We can only take a complete closed Array
+          TRI_ASSERT(parameters->isClosed());
+          TRI_ASSERT(parameters->slice().isArray());
+        }
 
   ~TRI_relation_index_operator_t();
 };
@@ -153,16 +157,10 @@ class TRI_relation_index_operator_t : public TRI_index_operator_t {
 /// @brief create a new index operator of the specified type
 ///
 /// note that the index which uses these operators  will take ownership of the
-/// json parameters passed to it
+/// VelocyPack parameters passed to it
 ////////////////////////////////////////////////////////////////////////////////
 
-TRI_index_operator_t* TRI_CreateIndexOperator(TRI_index_operator_type_e,
+TRI_index_operator_t* TRI_CreateIndexOperator(
-                                              TRI_index_operator_t*,
+    TRI_index_operator_type_e, TRI_index_operator_t*, TRI_index_operator_t*,
-                                              TRI_index_operator_t*,
+    std::shared_ptr<arangodb::velocypack::Builder>, VocShaper*, size_t);
-                                              TRI_json_t*, VocShaper*, size_t);
-
-TRI_index_operator_t* TRI_CreateIndexOperator(TRI_index_operator_type_e,
-                                              TRI_index_operator_t*,
-                                              TRI_index_operator_t*,
-                                              arangodb::velocypack::Slice const&, VocShaper*, size_t);
 #endif

arangod/Indexes/SkiplistIndex.cpp

@@ -84,23 +84,23 @@ static TRI_index_operator_t* buildBoundOperator(VPackSlice const& bound,
     }
   }
 
-  VPackBuilder builder;
+  auto builder = std::make_shared<VPackBuilder>();
   try {
-    VPackArrayBuilder b(&builder);
+    VPackArrayBuilder b(builder.get());
     if (parameters.isArray()) {
       // Everything else is to be ignored.
       // Copy content of array
       for (auto const& e : VPackArrayIterator(parameters)) {
-        builder.add(e);
+        builder->add(e);
       }
     }
-    builder.add(bound);
+    builder->add(bound);
   } catch (...) {
     // Out of memory. Cannot build operator.
     return nullptr;
   }
 
-  return TRI_CreateIndexOperator(type, nullptr, nullptr, builder.slice(),
+  return TRI_CreateIndexOperator(type, nullptr, nullptr, builder,
                                  shaper, 1);
 }
 
@@ -129,11 +129,10 @@ static TRI_index_operator_t* buildRangeOperator(VPackSlice const& lowerBound,
     return lowerOperator.release();
   }
 
-  VPackSlice empty;
   // And combine both
   std::unique_ptr<TRI_index_operator_t> rangeOperator(
       TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lowerOperator.get(),
-                              upperOperator.get(), empty, nullptr, 2));
+                              upperOperator.get(), std::make_shared<VPackBuilder>(), nullptr, 2));
   lowerOperator.release();
   upperOperator.release();
   return rangeOperator.release();
@@ -233,20 +232,18 @@ static int FillLookupOperator(TRI_index_operator_t* slOperator,
     case TRI_LT_INDEX_OPERATOR: {
       TRI_relation_index_operator_t* relationOperator =
           (TRI_relation_index_operator_t*)slOperator;
-      relationOperator->_numFields =
+      VPackSlice const params = relationOperator->_parameters->slice();
-          TRI_LengthVector(&relationOperator->_parameters->_value._objects);
+      relationOperator->_numFields = static_cast<size_t>(params.length());
       relationOperator->_fields = static_cast<TRI_shaped_json_t*>(TRI_Allocate(
           TRI_UNKNOWN_MEM_ZONE,
           sizeof(TRI_shaped_json_t) * relationOperator->_numFields, false));
 
       if (relationOperator->_fields != nullptr) {
         for (size_t j = 0; j < relationOperator->_numFields; ++j) {
-          TRI_json_t const* jsonObject =
+          VPackSlice const element = params.at(j);
-              static_cast<TRI_json_t* const>(TRI_AtVector(
-                  &(relationOperator->_parameters->_value._objects), j));
 
           // find out if the search value is a list or an array
-          if ((TRI_IsArrayJson(jsonObject) || TRI_IsObjectJson(jsonObject)) &&
+          if ((element.isArray() || element.isObject()) &&
               slOperator->_type != TRI_EQ_INDEX_OPERATOR) {
             // non-equality operator used on list or array data type, this is
             // disallowed
@@ -268,8 +265,8 @@ static int FillLookupOperator(TRI_index_operator_t* slOperator,
           }
 
           // now shape the search object (but never create any new shapes)
-          TRI_shaped_json_t* shapedObject =
+          TRI_shaped_json_t* shapedObject = TRI_ShapedJsonVelocyPack(
-              TRI_ShapedJsonJson(document->getShaper(), jsonObject,
+              document->getShaper(), element,
               false);  // ONLY IN INDEX, PROTECTED by RUNTIME
 
           if (shapedObject != nullptr) {
@@ -1260,11 +1257,14 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
   // Create the skiplistOperator for the IndexLookup
   if (node == nullptr) {
     // We have no condition, we just use sort
-    Json nullArray(Json::Array);
+    auto builder = std::make_shared<VPackBuilder>();
-    nullArray.add(Json(Json::Null));
+    {
+      VPackArrayBuilder b(builder.get());
+      builder->add(VPackValue(VPackValueType::Null));
+    }
     std::unique_ptr<TRI_index_operator_t> unboundOperator(
         TRI_CreateIndexOperator(TRI_GE_INDEX_OPERATOR, nullptr, nullptr,
-                                nullArray.steal(), _shaper, 1));
+                                builder, _shaper, 1));
     std::vector<TRI_index_operator_t*> searchValues({unboundOperator.get()});
     unboundOperator.release();
 
@@ -1368,8 +1368,8 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
   // Now handle the next element, which might be a range
   bool includeLower = false;
   bool includeUpper = false;
-  std::shared_ptr<VPackBuilder> lower;
+  auto lower = std::make_shared<VPackBuilder>();
-  std::shared_ptr<VPackBuilder> upper;
+  auto upper = std::make_shared<VPackBuilder>();
   if (usedFields < _fields.size()) {
     auto it = found.find(usedFields);
     if (it != found.end()) {
@@ -1385,12 +1385,12 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
         auto setBorder = [&](bool isLower, bool includeBound) -> void {
           if (isLower == isReverseOrder) {
             // We set an upper bound
-            TRI_ASSERT(upper == nullptr);
+            TRI_ASSERT(upper->isEmpty());
             upper = value->toVelocyPackValue();
             includeUpper = includeBound;
           } else {
             // We set an lower bound
-            TRI_ASSERT(lower == nullptr);
+            TRI_ASSERT(lower->isEmpty());
             lower = value->toVelocyPackValue();
             includeLower = includeBound;
           }
@@ -1442,11 +1442,11 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
       bool done = false;
       // create all permutations
       while (!done) {
-        VPackBuilder parameter;
+        auto parameter = std::make_shared<VPackBuilder>();
         bool valid = true;
 
         try {
-          VPackArrayBuilder b(&parameter);
+          VPackArrayBuilder b(parameter.get());
           for (size_t i = 0; i < usedFields; ++i) {
             TRI_ASSERT(i < permutationStates.size());
             auto& state = permutationStates[i];
@@ -1459,7 +1459,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
               valid = false;
               break;
             }
-            parameter.add(value);
+            parameter->add(value);
           }
         } catch (...) {
           // Out of Memory
@@ -1469,16 +1469,16 @@ IndexIterator* SkiplistIndex::iteratorForCondition(
         if (valid) {
           std::unique_ptr<TRI_index_operator_t> tmpOp(
               TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR, nullptr, nullptr,
-                                      parameter.slice(), _shaper, usedFields));
+                                      parameter, _shaper, usedFields));
           // Note we create a new RangeOperator always.
           std::unique_ptr<TRI_index_operator_t> rangeOperator(
               buildRangeOperator(lower->slice(), includeLower, upper->slice(),
-                                 includeUpper, parameter.slice(), _shaper));
+                                 includeUpper, parameter->slice(), _shaper));
 
           if (rangeOperator != nullptr) {
             std::unique_ptr<TRI_index_operator_t> combinedOp(
                 TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, tmpOp.get(),
-                                        rangeOperator.get(), emptySlice, _shaper,
+                                        rangeOperator.get(), std::make_shared<VPackBuilder>(), _shaper,
                                         2));
             rangeOperator.release();
             tmpOp.release();

arangod/V8Server/v8-query.cpp

@@ -37,6 +37,7 @@
 #include "V8/v8-globals.h"
 #include "V8/v8-conv.h"
 #include "V8/v8-utils.h"
+#include "V8/v8-vpack.h"
 #include "V8Server/v8-shape-conv.h"
 #include "V8Server/v8-vocbase.h"
 #include "V8Server/v8-vocindex.h"
@@ -163,17 +164,14 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
     v8::Isolate* isolate,
     std::vector<std::vector<triagens::basics::AttributeName>> const& fields,
     VocShaper* shaper, v8::Handle<v8::Object> conditions) {
-  TRI_index_operator_t* lastOperator = nullptr;
   size_t numEq = 0;
   size_t lastNonEq = 0;
+  std::unique_ptr<TRI_index_operator_t> lastOperator;
 
-  TRI_json_t* parameters = TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE);
+  VPackBuilder parameters;
+  try {
+    VPackArrayBuilder b(&parameters);
 
-  if (parameters == nullptr) {
-    return nullptr;
-  }
-
-  // iterate over all index fields
     size_t i = 0;
     for (auto const& field : fields) {
       std::string fieldString;
@@ -197,7 +195,7 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
 
         if (!fieldCondition->IsArray()) {
           // wrong data type for single condition
-        goto MEM_ERROR;
+          return nullptr;
         }
 
         v8::Handle<v8::Array> condition =
@@ -205,7 +203,7 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
 
         if (condition->Length() != 2) {
           // wrong number of values in single condition
-        goto MEM_ERROR;
+          return nullptr;
         }
 
         v8::Handle<v8::Value> op = condition->Get(0);
@@ -213,13 +211,14 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
 
         if (!op->IsString() && !op->IsStringObject()) {
           // wrong operator type
-        goto MEM_ERROR;
+          return nullptr;
         }
 
-      TRI_json_t* json = TRI_ObjectToJson(isolate, value);
+        VPackBuilder element;
-
+        int res = TRI_V8ToVPack(isolate, element, value, false);
-      if (json == nullptr) {
+        if (res != TRI_ERROR_NO_ERROR) {
-        goto MEM_ERROR;
+          // Failed to parse or Out of Memory
+          return nullptr;
         }
 
         std::string&& opValue = TRI_ObjectToString(op);
@@ -227,11 +226,10 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
           // equality comparison
 
           if (lastNonEq > 0) {
-          TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json);
+            return nullptr;
-          goto MEM_ERROR;
           }
 
-        TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, parameters, json);
+          parameters.add(element.slice());
           // creation of equality operator is deferred until it is finally needed
           ++numEq;
           break;
@@ -240,8 +238,7 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
             // if we already had a range condition and a previous field, we cannot
             // continue
             // because the skiplist interface does not support such queries
-          TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json);
+            return nullptr;
-          goto MEM_ERROR;
           }
 
           TRI_index_operator_type_e opType;
@@ -255,101 +252,111 @@ static TRI_index_operator_t* SetupConditionsSkiplist(
             opType = TRI_LE_INDEX_OPERATOR;
           } else {
             // wrong operator type
-          TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json);
+            return nullptr;
-          goto MEM_ERROR;
           }
 
           lastNonEq = i;
 
-        TRI_json_t* cloned = TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters);
+          if (numEq > 0) {
+            TRI_ASSERT(!parameters.isClosed());
+            // TODO, check if this actually worked
+            auto cloned = std::make_shared<VPackBuilder>(parameters);
 
             if (cloned == nullptr) {
-          TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json);
+              // Out of memory could not copy Builder
-          goto MEM_ERROR;
+              return nullptr;
             }
+            TRI_ASSERT(!cloned->isClosed());
+            cloned->close();
 
-        TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, cloned, json);
+            // Assert that the buffer is actualy copied and we can work with both Builders
+            TRI_ASSERT(cloned->isClosed());
+            TRI_ASSERT(!parameters.isClosed());
 
-        if (numEq) {
+            VPackSlice tmp = cloned->slice();
-          // create equality operator if one is in queue
+            lastOperator.reset(TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR,
-          TRI_json_t* clonedParams =
+                                                       nullptr, nullptr, cloned,
-              TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters);
+                                                       shaper, tmp.length()));
-
-          if (clonedParams == nullptr) {
-            TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, cloned);
-            goto MEM_ERROR;
-          }
-
-          lastOperator = TRI_CreateIndexOperator(
-              TRI_EQ_INDEX_OPERATOR, nullptr, nullptr, clonedParams, shaper,
-              TRI_LengthVector(&clonedParams->_value._objects));
             numEq = 0;
           }
 
-        TRI_index_operator_t* current;
+          std::unique_ptr<TRI_index_operator_t> current;
 
-        // create the operator for the current condition
+          {
-        current =
+            TRI_ASSERT(!parameters.isClosed());
-            TRI_CreateIndexOperator(opType, nullptr, nullptr, cloned, shaper,
+            // TODO, check if this actually worked
-                                    TRI_LengthVector(&cloned->_value._objects));
+            auto cloned = std::make_shared<VPackBuilder>(parameters);
+
+            if (cloned == nullptr) {
+              // Out of memory could not copy Builder
+              return nullptr;
+            }
+            TRI_ASSERT(!cloned->isClosed());
+
+            cloned->add(element.slice());
+
+            cloned->close();
+
+            // Assert that the buffer is actualy copied and we can work with both Builders
+            TRI_ASSERT(cloned->isClosed());
+            TRI_ASSERT(!parameters.isClosed());
+            VPackSlice tmp = cloned->slice();
+
+            current.reset(TRI_CreateIndexOperator(
+                opType, nullptr, nullptr, cloned, shaper, tmp.length()));
 
             if (current == nullptr) {
-          TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, cloned);
+              return nullptr;
-          goto MEM_ERROR;
+            }
           }
 
           if (lastOperator == nullptr) {
-          lastOperator = current;
+            lastOperator.swap(current);
           } else {
             // merge the current operator with previous operators using logical
             // AND
-          TRI_index_operator_t* newOperator =
+
-              TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lastOperator,
+            std::unique_ptr<TRI_index_operator_t> newOperator(
-                                      current, nullptr, shaper, 2);
+                TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lastOperator.get(),
+                                        current.get(), nullptr, shaper, 2));
 
             if (newOperator == nullptr) {
-            delete current;
+              // current and lastOperator are still responsible and will free
-            goto MEM_ERROR;
+              return nullptr;
             } else {
-            lastOperator = newOperator;
+              // newOperator is now responsible for current and lastOperator. release them
+              current.release();
+              lastOperator.release();
+              lastOperator.swap(newOperator);
             }
           }
         }
       }
-
       ++i;
     }
+  } catch (...) {
+    // Out of Memory
+    return nullptr;
+  }
 
-  if (numEq) {
+  if (numEq > 0) {
     // create equality operator if one is in queue
     TRI_ASSERT(lastOperator == nullptr);
     TRI_ASSERT(lastNonEq == 0);
 
-    TRI_json_t* clonedParams = TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters);
+    auto clonedParams = std::make_shared<VPackBuilder>(parameters);
 
     if (clonedParams == nullptr) {
-      goto MEM_ERROR;
-    }
-
-    lastOperator = TRI_CreateIndexOperator(
-        TRI_EQ_INDEX_OPERATOR, nullptr, nullptr, clonedParams, shaper,
-        TRI_LengthVector(&clonedParams->_value._objects));
-  }
-
-  TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, parameters);
-
-  return lastOperator;
-
-MEM_ERROR:
-  TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, parameters);
-
-  if (lastOperator != nullptr) {
-    delete lastOperator;
-    lastOperator = nullptr;
-  }
-
       return nullptr;
     }
+    VPackSlice tmp = clonedParams->slice();
+
+    lastOperator.reset(TRI_CreateIndexOperator(
+        TRI_EQ_INDEX_OPERATOR, nullptr, nullptr, clonedParams, shaper,
+        tmp.length()));
+  }
+  return lastOperator.release();
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief sets up the skiplist operator for a skiplist example query
@@ -392,8 +399,10 @@ static TRI_index_operator_t* SetupExampleSkiplist(
 
   if (TRI_LengthArrayJson(parameters) > 0) {
     // example means equality comparisons only
-    return TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR, nullptr, nullptr,
+    // // TODO FIX parameters to be VelocyPack in the first place
-                                   parameters, shaper,
+    return TRI_CreateIndexOperator(
+        TRI_EQ_INDEX_OPERATOR, nullptr, nullptr,
+        triagens::basics::JsonHelper::toVelocyPack(parameters), shaper,
         TRI_LengthArrayJson(parameters));
   }