From b04f1bf9e2f80436a132f40afca9d4e4324637cc Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Thu, 21 Jan 2016 14:07:36 +0100 Subject: [PATCH] Replaced IndexOperators using TRI_json_t by Operators using VPack. --- arangod/IndexOperators/index-operator.cpp | 16 +- arangod/IndexOperators/index-operator.h | 26 +- arangod/Indexes/SkiplistIndex.cpp | 58 ++-- arangod/V8Server/v8-query.cpp | 353 +++++++++++----------- 4 files changed, 224 insertions(+), 229 deletions(-) diff --git a/arangod/IndexOperators/index-operator.cpp b/arangod/IndexOperators/index-operator.cpp index ace65c3341..e74d0089a8 100644 --- a/arangod/IndexOperators/index-operator.cpp +++ b/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) { - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, _parameters); - } + // _parameters fill automatically free 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 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 tmp( - triagens::basics::VelocyPackHelper::velocyPackToJson(parameters)); - return TRI_CreateIndexOperator(operatorType, leftOperand, rightOperand, - tmp.release(), shaper, numFields); -} diff --git a/arangod/IndexOperators/index-operator.h b/arangod/IndexOperators/index-operator.h index 1d231f1a12..041710fc08 100644 --- a/arangod/IndexOperators/index-operator.h +++ b/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 _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 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_index_operator_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); +TRI_index_operator_t* TRI_CreateIndexOperator( + TRI_index_operator_type_e, TRI_index_operator_t*, TRI_index_operator_t*, + std::shared_ptr, VocShaper*, size_t); #endif diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index e2a245cf75..5416aa70eb 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -84,23 +84,23 @@ static TRI_index_operator_t* buildBoundOperator(VPackSlice const& bound, } } - VPackBuilder builder; + auto builder = std::make_shared(); 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 rangeOperator( TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lowerOperator.get(), - upperOperator.get(), empty, nullptr, 2)); + upperOperator.get(), std::make_shared(), 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 = - TRI_LengthVector(&relationOperator->_parameters->_value._objects); + VPackSlice const params = relationOperator->_parameters->slice(); + relationOperator->_numFields = static_cast(params.length()); relationOperator->_fields = static_cast(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 = - static_cast(TRI_AtVector( - &(relationOperator->_parameters->_value._objects), j)); + VPackSlice const element = params.at(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,9 +265,9 @@ 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_ShapedJsonJson(document->getShaper(), jsonObject, - false); // ONLY IN INDEX, PROTECTED by RUNTIME + TRI_shaped_json_t* shapedObject = TRI_ShapedJsonVelocyPack( + document->getShaper(), element, + false); // ONLY IN INDEX, PROTECTED by RUNTIME if (shapedObject != nullptr) { // found existing shape @@ -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); - nullArray.add(Json(Json::Null)); + auto builder = std::make_shared(); + { + VPackArrayBuilder b(builder.get()); + builder->add(VPackValue(VPackValueType::Null)); + } std::unique_ptr unboundOperator( TRI_CreateIndexOperator(TRI_GE_INDEX_OPERATOR, nullptr, nullptr, - nullArray.steal(), _shaper, 1)); + builder, _shaper, 1)); std::vector 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 lower; - std::shared_ptr upper; + auto lower = std::make_shared(); + auto upper = std::make_shared(); 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(); bool valid = true; try { - VPackArrayBuilder b(¶meter); + 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 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 rangeOperator( buildRangeOperator(lower->slice(), includeLower, upper->slice(), - includeUpper, parameter.slice(), _shaper)); + includeUpper, parameter->slice(), _shaper)); if (rangeOperator != nullptr) { std::unique_ptr combinedOp( TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, tmpOp.get(), - rangeOperator.get(), emptySlice, _shaper, + rangeOperator.get(), std::make_shared(), _shaper, 2)); rangeOperator.release(); tmpOp.release(); diff --git a/arangod/V8Server/v8-query.cpp b/arangod/V8Server/v8-query.cpp index e6def585c1..b73242651b 100644 --- a/arangod/V8Server/v8-query.cpp +++ b/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,192 +164,198 @@ static TRI_index_operator_t* SetupConditionsSkiplist( v8::Isolate* isolate, std::vector> const& fields, VocShaper* shaper, v8::Handle conditions) { - TRI_index_operator_t* lastOperator = nullptr; size_t numEq = 0; size_t lastNonEq = 0; + std::unique_ptr lastOperator; - TRI_json_t* parameters = TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE); + VPackBuilder parameters; + try { + VPackArrayBuilder b(¶meters); - if (parameters == nullptr) { + size_t i = 0; + for (auto const& field : fields) { + std::string fieldString; + TRI_AttributeNamesToString(field, fieldString, true); + v8::Handle key = TRI_V8_STD_STRING(fieldString); + + if (!conditions->HasOwnProperty(key)) { + break; + } + v8::Handle fieldConditions = conditions->Get(key); + + if (!fieldConditions->IsArray()) { + // wrong data type for field conditions + break; + } + + // iterator over all conditions + v8::Handle values = v8::Handle::Cast(fieldConditions); + for (uint32_t j = 0; j < values->Length(); ++j) { + v8::Handle fieldCondition = values->Get(j); + + if (!fieldCondition->IsArray()) { + // wrong data type for single condition + return nullptr; + } + + v8::Handle condition = + v8::Handle::Cast(fieldCondition); + + if (condition->Length() != 2) { + // wrong number of values in single condition + return nullptr; + } + + v8::Handle op = condition->Get(0); + v8::Handle value = condition->Get(1); + + if (!op->IsString() && !op->IsStringObject()) { + // wrong operator type + return nullptr; + } + + VPackBuilder element; + int res = TRI_V8ToVPack(isolate, element, value, false); + if (res != TRI_ERROR_NO_ERROR) { + // Failed to parse or Out of Memory + return nullptr; + } + + std::string&& opValue = TRI_ObjectToString(op); + if (opValue == "==") { + // equality comparison + + if (lastNonEq > 0) { + return nullptr; + } + + parameters.add(element.slice()); + // creation of equality operator is deferred until it is finally needed + ++numEq; + break; + } else { + if (lastNonEq > 0 && lastNonEq != i) { + // if we already had a range condition and a previous field, we cannot + // continue + // because the skiplist interface does not support such queries + return nullptr; + } + + TRI_index_operator_type_e opType; + if (opValue == ">") { + opType = TRI_GT_INDEX_OPERATOR; + } else if (opValue == ">=") { + opType = TRI_GE_INDEX_OPERATOR; + } else if (opValue == "<") { + opType = TRI_LT_INDEX_OPERATOR; + } else if (opValue == "<=") { + opType = TRI_LE_INDEX_OPERATOR; + } else { + // wrong operator type + return nullptr; + } + + lastNonEq = i; + + if (numEq > 0) { + TRI_ASSERT(!parameters.isClosed()); + // TODO, check if this actually worked + auto cloned = std::make_shared(parameters); + + if (cloned == nullptr) { + // Out of memory could not copy Builder + return nullptr; + } + TRI_ASSERT(!cloned->isClosed()); + 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(); + lastOperator.reset(TRI_CreateIndexOperator(TRI_EQ_INDEX_OPERATOR, + nullptr, nullptr, cloned, + shaper, tmp.length())); + numEq = 0; + } + + std::unique_ptr current; + + { + TRI_ASSERT(!parameters.isClosed()); + // TODO, check if this actually worked + auto cloned = std::make_shared(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) { + return nullptr; + } + } + + if (lastOperator == nullptr) { + lastOperator.swap(current); + } else { + // merge the current operator with previous operators using logical + // AND + + std::unique_ptr newOperator( + TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lastOperator.get(), + current.get(), nullptr, shaper, 2)); + + if (newOperator == nullptr) { + // current and lastOperator are still responsible and will free + return nullptr; + } else { + // newOperator is now responsible for current and lastOperator. release them + current.release(); + lastOperator.release(); + lastOperator.swap(newOperator); + } + } + } + } + ++i; + } + } catch (...) { + // Out of Memory return nullptr; } - // iterate over all index fields - size_t i = 0; - for (auto const& field : fields) { - std::string fieldString; - TRI_AttributeNamesToString(field, fieldString, true); - v8::Handle key = TRI_V8_STD_STRING(fieldString); - - if (!conditions->HasOwnProperty(key)) { - break; - } - v8::Handle fieldConditions = conditions->Get(key); - - if (!fieldConditions->IsArray()) { - // wrong data type for field conditions - break; - } - - // iterator over all conditions - v8::Handle values = v8::Handle::Cast(fieldConditions); - for (uint32_t j = 0; j < values->Length(); ++j) { - v8::Handle fieldCondition = values->Get(j); - - if (!fieldCondition->IsArray()) { - // wrong data type for single condition - goto MEM_ERROR; - } - - v8::Handle condition = - v8::Handle::Cast(fieldCondition); - - if (condition->Length() != 2) { - // wrong number of values in single condition - goto MEM_ERROR; - } - - v8::Handle op = condition->Get(0); - v8::Handle value = condition->Get(1); - - if (!op->IsString() && !op->IsStringObject()) { - // wrong operator type - goto MEM_ERROR; - } - - TRI_json_t* json = TRI_ObjectToJson(isolate, value); - - if (json == nullptr) { - goto MEM_ERROR; - } - - std::string&& opValue = TRI_ObjectToString(op); - if (opValue == "==") { - // equality comparison - - if (lastNonEq > 0) { - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); - goto MEM_ERROR; - } - - TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, parameters, json); - // creation of equality operator is deferred until it is finally needed - ++numEq; - break; - } else { - if (lastNonEq > 0 && lastNonEq != i) { - // 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); - goto MEM_ERROR; - } - - TRI_index_operator_type_e opType; - if (opValue == ">") { - opType = TRI_GT_INDEX_OPERATOR; - } else if (opValue == ">=") { - opType = TRI_GE_INDEX_OPERATOR; - } else if (opValue == "<") { - opType = TRI_LT_INDEX_OPERATOR; - } else if (opValue == "<=") { - opType = TRI_LE_INDEX_OPERATOR; - } else { - // wrong operator type - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); - goto MEM_ERROR; - } - - lastNonEq = i; - - TRI_json_t* cloned = TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters); - - if (cloned == nullptr) { - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, json); - goto MEM_ERROR; - } - - TRI_PushBack3ArrayJson(TRI_UNKNOWN_MEM_ZONE, cloned, json); - - if (numEq) { - // create equality operator if one is in queue - TRI_json_t* clonedParams = - TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters); - - 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; - - // create the operator for the current condition - current = - TRI_CreateIndexOperator(opType, nullptr, nullptr, cloned, shaper, - TRI_LengthVector(&cloned->_value._objects)); - - if (current == nullptr) { - TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, cloned); - goto MEM_ERROR; - } - - if (lastOperator == nullptr) { - lastOperator = current; - } else { - // merge the current operator with previous operators using logical - // AND - TRI_index_operator_t* newOperator = - TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, lastOperator, - current, nullptr, shaper, 2); - - if (newOperator == nullptr) { - delete current; - goto MEM_ERROR; - } else { - lastOperator = newOperator; - } - } - } - } - - ++i; - } - - 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(parameters); if (clonedParams == nullptr) { - goto MEM_ERROR; + return nullptr; } + VPackSlice tmp = clonedParams->slice(); - lastOperator = TRI_CreateIndexOperator( + lastOperator.reset(TRI_CreateIndexOperator( TRI_EQ_INDEX_OPERATOR, nullptr, nullptr, clonedParams, shaper, - TRI_LengthVector(&clonedParams->_value._objects)); + tmp.length())); } - - 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; + return lastOperator.release(); } //////////////////////////////////////////////////////////////////////////////// @@ -392,9 +399,11 @@ 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, - parameters, shaper, - TRI_LengthArrayJson(parameters)); + // // TODO FIX parameters to be VelocyPack in the first place + return TRI_CreateIndexOperator( + TRI_EQ_INDEX_OPERATOR, nullptr, nullptr, + triagens::basics::JsonHelper::toVelocyPack(parameters), shaper, + TRI_LengthArrayJson(parameters)); } TRI_FreeJson(TRI_UNKNOWN_MEM_ZONE, parameters);