From 9639a9e669a76e3dc76b052f5521e4925b46ce88 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Thu, 8 Oct 2015 13:35:45 +0200 Subject: [PATCH 1/3] Skiplist index now can handle upper and lower bounds and combined with equal and any. hunt leaks now --- arangod/Indexes/SkiplistIndex.cpp | 136 +++++++++++++++--------------- 1 file changed, 70 insertions(+), 66 deletions(-) diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index a8aab22734..919d1e2fd4 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -1186,17 +1186,17 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex // This is not an equalityCheck, set lower or upper switch (comp->type) { case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LT: - setBorder(false, false); - break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LE: - setBorder(false, true); - break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GT: setBorder(true, false); break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GE: + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LE: setBorder(true, true); break; + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GT: + setBorder(false, false); + break; + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GE: + setBorder(false, true); + break; default: // unsupported right now. Should have been rejected by supportsFilterCondition TRI_ASSERT(false); @@ -1210,71 +1210,73 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex searchValues.reserve(maxPermutations); - std::unique_ptr rangeOperator(nullptr); + auto buildRangeOperator = [&] () -> TRI_index_operator_t* { + std::unique_ptr rangeOperator(nullptr); - if (lower != nullptr) { - TRI_index_operator_type_e type; - if (includeLower) { - type = TRI_LE_INDEX_OPERATOR; - } - else { - type = TRI_LT_INDEX_OPERATOR; - } + if (lower != nullptr) { + TRI_index_operator_type_e type; + if (includeLower) { + type = TRI_LE_INDEX_OPERATOR; + } + else { + type = TRI_LT_INDEX_OPERATOR; + } - std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); - if (parameter == nullptr) { - return nullptr; - } - TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, lower)); - std::cout << parameter.get() << std::endl; - rangeOperator.reset(TRI_CreateIndexOperator(type, - nullptr, - nullptr, - parameter.get(), - _shaper, - 1)); - parameter.release(); - } - - if (upper != nullptr) { - TRI_index_operator_type_e type; - if (includeUpper) { - type = TRI_GE_INDEX_OPERATOR; - } - else { - type = TRI_GT_INDEX_OPERATOR; - } - - std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); - if (parameter == nullptr) { - return nullptr; - } - TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, upper)); - std::unique_ptr tmpOp(TRI_CreateIndexOperator(type, - nullptr, - nullptr, - parameter.get(), - _shaper, - 1)); - parameter.release(); - if (rangeOperator == nullptr) { - rangeOperator.reset(tmpOp.release()); - } - else { - rangeOperator.reset(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, - rangeOperator.release(), - tmpOp.get(), + std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); + if (parameter == nullptr) { + return nullptr; + } + TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, lower)); + std::cout << parameter.get() << std::endl; + rangeOperator.reset(TRI_CreateIndexOperator(type, nullptr, - _shaper, - 2)); - tmpOp.release(); + nullptr, + parameter.get(), + _shaper, + 1)); + parameter.release(); } - } + + if (upper != nullptr) { + TRI_index_operator_type_e type; + if (includeUpper) { + type = TRI_GE_INDEX_OPERATOR; + } + else { + type = TRI_GT_INDEX_OPERATOR; + } + + std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); + if (parameter == nullptr) { + return nullptr; + } + TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, upper)); + std::unique_ptr tmpOp(TRI_CreateIndexOperator(type, + nullptr, + nullptr, + parameter.get(), + _shaper, + 1)); + parameter.release(); + if (rangeOperator == nullptr) { + rangeOperator.reset(tmpOp.release()); + } + else { + rangeOperator.reset(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, + rangeOperator.release(), + tmpOp.get(), + nullptr, + _shaper, + 2)); + tmpOp.release(); + } + } + return rangeOperator.release(); + }; if (usedFields == 0) { // We have a range query based on the first _field - searchValues.emplace_back(rangeOperator.get()); - rangeOperator.release(); + searchValues.emplace_back(buildRangeOperator()); } else { bool done = false; @@ -1302,14 +1304,16 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex _shaper, usedFields)); parameter.release(); + // Note we create a new RangeOperator always. + std::unique_ptr rangeOperator(buildRangeOperator()); if (rangeOperator != nullptr) { - // NOTE: We might have to clone the rangeOperator here! std::unique_ptr combinedOp(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, rangeOperator.get(), tmpOp.get(), nullptr, _shaper, 2)); + rangeOperator.release(); tmpOp.release(); searchValues.emplace_back(combinedOp.get()); combinedOp.release(); From 345725bdf9a752e4e14c165359b6cccffe17eaf7 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Thu, 8 Oct 2015 14:58:52 +0200 Subject: [PATCH 2/3] Fixed skiplistindex with equality and range conditions --- arangod/Indexes/SkiplistIndex.cpp | 115 ++++++++++++++++++------------ lib/Basics/JsonHelper.cpp | 3 + 2 files changed, 71 insertions(+), 47 deletions(-) diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index 919d1e2fd4..bf2ae6a71f 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -1164,6 +1164,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex TRI_ASSERT(rangeConditions.size() <= 2); for (auto& comp : rangeConditions) { + std::cout << "Range Condition" << std::endl; TRI_ASSERT(comp->numMembers() == 2); triagens::aql::AstNode const* access = nullptr; triagens::aql::AstNode const* value = nullptr; @@ -1180,23 +1181,23 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex // We set an lower bound TRI_ASSERT(lower == nullptr); lower = value->toJsonValue(TRI_UNKNOWN_MEM_ZONE); - includeUpper = includeBound; + includeLower = includeBound; } }; // This is not an equalityCheck, set lower or upper switch (comp->type) { case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LT: - setBorder(true, false); - break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LE: - setBorder(true, true); - break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GT: setBorder(false, false); break; - case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GE: + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_LE: setBorder(false, true); break; + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GT: + setBorder(true, false); + break; + case triagens::aql::NODE_TYPE_OPERATOR_BINARY_GE: + setBorder(true, true); + break; default: // unsupported right now. Should have been rejected by supportsFilterCondition TRI_ASSERT(false); @@ -1210,54 +1211,73 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex searchValues.reserve(maxPermutations); - auto buildRangeOperator = [&] () -> TRI_index_operator_t* { - std::unique_ptr rangeOperator(nullptr); + auto buildRangeOperator = [] (TRI_json_t* lowerBound, + bool lowerBoundInclusive, + TRI_json_t* upperBound, + bool upperBoundInclusive, + TRI_json_t* parameters, + VocShaper* shaper) -> TRI_index_operator_t* { + std::unique_ptr rangeOperator; - if (lower != nullptr) { + std::cout << lowerBound << ".." << lowerBoundInclusive << " < " << upperBound << ".." << upperBoundInclusive << std::endl; + if (lowerBound != nullptr) { TRI_index_operator_type_e type; - if (includeLower) { - type = TRI_LE_INDEX_OPERATOR; - } - else { - type = TRI_LT_INDEX_OPERATOR; - } - - std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); - if (parameter == nullptr) { - return nullptr; - } - TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, lower)); - std::cout << parameter.get() << std::endl; - rangeOperator.reset(TRI_CreateIndexOperator(type, - nullptr, - nullptr, - parameter.get(), - _shaper, - 1)); - parameter.release(); - } - - if (upper != nullptr) { - TRI_index_operator_type_e type; - if (includeUpper) { + if (lowerBoundInclusive) { type = TRI_GE_INDEX_OPERATOR; } else { type = TRI_GT_INDEX_OPERATOR; } - std::unique_ptr parameter(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); - if (parameter == nullptr) { + std::unique_ptr paramCopy; + if (parameters == nullptr) { + paramCopy.reset(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); + } + else { + paramCopy.reset(TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters)); + } + if (paramCopy == nullptr) { return nullptr; } - TRI_PushBack2ArrayJson(parameter.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, upper)); + + TRI_PushBack2ArrayJson(paramCopy.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, lowerBound)); + rangeOperator.reset(TRI_CreateIndexOperator(type, + nullptr, + nullptr, + paramCopy.get(), + shaper, + 1)); + paramCopy.release(); + } + + if (upperBound != nullptr) { + TRI_index_operator_type_e type; + if (upperBoundInclusive) { + type = TRI_LE_INDEX_OPERATOR; + } + else { + type = TRI_LT_INDEX_OPERATOR; + } + + std::unique_ptr paramCopy; + if (parameters == nullptr) { + paramCopy.reset(TRI_CreateArrayJson(TRI_UNKNOWN_MEM_ZONE, 1)); + } + else { + paramCopy.reset(TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, parameters)); + } + if (paramCopy == nullptr) { + return nullptr; + } + + TRI_PushBack2ArrayJson(paramCopy.get(), TRI_CopyJson(TRI_UNKNOWN_MEM_ZONE, upperBound)); std::unique_ptr tmpOp(TRI_CreateIndexOperator(type, nullptr, nullptr, - parameter.get(), - _shaper, + paramCopy.get(), + shaper, 1)); - parameter.release(); + paramCopy.release(); if (rangeOperator == nullptr) { rangeOperator.reset(tmpOp.release()); } @@ -1266,7 +1286,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex rangeOperator.release(), tmpOp.get(), nullptr, - _shaper, + shaper, 2)); tmpOp.release(); } @@ -1276,7 +1296,7 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex if (usedFields == 0) { // We have a range query based on the first _field - searchValues.emplace_back(buildRangeOperator()); + searchValues.emplace_back(buildRangeOperator(lower, includeLower, upper, includeUpper, nullptr, _shaper)); } else { bool done = false; @@ -1303,13 +1323,14 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex parameter.get(), _shaper, usedFields)); - parameter.release(); // Note we create a new RangeOperator always. - std::unique_ptr rangeOperator(buildRangeOperator()); + std::unique_ptr rangeOperator(buildRangeOperator(lower, includeLower, upper, includeUpper, parameter.get(), _shaper)); + parameter.release(); + if (rangeOperator != nullptr) { std::unique_ptr combinedOp(TRI_CreateIndexOperator(TRI_AND_INDEX_OPERATOR, - rangeOperator.get(), tmpOp.get(), + rangeOperator.get(), nullptr, _shaper, 2)); diff --git a/lib/Basics/JsonHelper.cpp b/lib/Basics/JsonHelper.cpp index 723ed1f970..e34d2c899d 100644 --- a/lib/Basics/JsonHelper.cpp +++ b/lib/Basics/JsonHelper.cpp @@ -213,6 +213,9 @@ TRI_json_t* JsonHelper::fromString (char const* data, //////////////////////////////////////////////////////////////////////////////// std::string JsonHelper::toString (TRI_json_t const* json) { + if (json == nullptr) { + return ""; + } TRI_string_buffer_t buffer; TRI_InitStringBuffer(&buffer, TRI_UNKNOWN_MEM_ZONE); From 24b41e986055ae82a1b83603d5b3ee51721ddbdd Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Thu, 8 Oct 2015 15:03:47 +0200 Subject: [PATCH 3/3] Removed debug output --- arangod/Indexes/SkiplistIndex.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/arangod/Indexes/SkiplistIndex.cpp b/arangod/Indexes/SkiplistIndex.cpp index bf2ae6a71f..152feca77e 100644 --- a/arangod/Indexes/SkiplistIndex.cpp +++ b/arangod/Indexes/SkiplistIndex.cpp @@ -1164,7 +1164,6 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex TRI_ASSERT(rangeConditions.size() <= 2); for (auto& comp : rangeConditions) { - std::cout << "Range Condition" << std::endl; TRI_ASSERT(comp->numMembers() == 2); triagens::aql::AstNode const* access = nullptr; triagens::aql::AstNode const* value = nullptr; @@ -1219,7 +1218,6 @@ IndexIterator* SkiplistIndex::iteratorForCondition (IndexIteratorContext* contex VocShaper* shaper) -> TRI_index_operator_t* { std::unique_ptr rangeOperator; - std::cout << lowerBound << ".." << lowerBoundInclusive << " < " << upperBound << ".." << upperBoundInclusive << std::endl; if (lowerBound != nullptr) { TRI_index_operator_type_e type; if (lowerBoundInclusive) {