From 9bb7c0dec635d76f45cad93b12ece464e8979751 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Fri, 21 Aug 2015 11:04:04 +0200 Subject: [PATCH] The skiplist index is now able to index arrays as well --- UnitTests/Basics/skiplist-test.cpp | 10 +++--- arangod/Indexes/SkiplistIndex2.cpp | 37 +++++++++++++++------- arangod/SkipLists/skiplistIndex.cpp | 49 ++++------------------------- arangod/SkipLists/skiplistIndex.h | 6 +--- arangod/V8Server/v8-query.cpp | 4 +-- lib/Basics/SkipList.h | 13 +++++++- 6 files changed, 52 insertions(+), 67 deletions(-) diff --git a/UnitTests/Basics/skiplist-test.cpp b/UnitTests/Basics/skiplist-test.cpp index 760a5d39fe..528949496b 100644 --- a/UnitTests/Basics/skiplist-test.cpp +++ b/UnitTests/Basics/skiplist-test.cpp @@ -91,7 +91,7 @@ BOOST_FIXTURE_TEST_SUITE(CSkipListTest, CSkipListSetup) //////////////////////////////////////////////////////////////////////////////// BOOST_AUTO_TEST_CASE (tst_unique_forward) { - triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true); + triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true, false); // check start node BOOST_CHECK_EQUAL((void*) 0, skiplist.startNode()->nextNode()); @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE (tst_unique_forward) { //////////////////////////////////////////////////////////////////////////////// BOOST_AUTO_TEST_CASE (tst_unique_reverse) { - triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true); + triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true, false); // check start node BOOST_CHECK_EQUAL((void*) 0, skiplist.startNode()->nextNode()); @@ -251,7 +251,7 @@ BOOST_AUTO_TEST_CASE (tst_unique_reverse) { //////////////////////////////////////////////////////////////////////////////// BOOST_AUTO_TEST_CASE (tst_unique_lookup) { - triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true); + triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true, false); std::vector values; for (int i = 0; i < 100; ++i) { @@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE (tst_unique_lookup) { //////////////////////////////////////////////////////////////////////////////// BOOST_AUTO_TEST_CASE (tst_unique_remove) { - triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true); + triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true, false); std::vector values; for (int i = 0; i < 100; ++i) { @@ -399,7 +399,7 @@ BOOST_AUTO_TEST_CASE (tst_unique_remove) { //////////////////////////////////////////////////////////////////////////////// BOOST_AUTO_TEST_CASE (tst_unique_remove_all) { - triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true); + triagens::basics::SkipList skiplist(CmpElmElm, CmpKeyElm, nullptr, FreeElm, true, false); std::vector values; for (int i = 0; i < 100; ++i) { diff --git a/arangod/Indexes/SkiplistIndex2.cpp b/arangod/Indexes/SkiplistIndex2.cpp index 37a0a0accf..98b10ed5dc 100644 --- a/arangod/Indexes/SkiplistIndex2.cpp +++ b/arangod/Indexes/SkiplistIndex2.cpp @@ -139,10 +139,18 @@ SkiplistIndex2::SkiplistIndex2 (TRI_idx_iid_t iid, TRI_ASSERT(! fields.empty()); TRI_ASSERT(iid != 0); + bool useExpansion = false; + for (auto& list: fields) { + if (TRI_AttributeNamesHaveExpansion(list)) { + useExpansion = true; + break; + } + } _skiplistIndex = SkiplistIndex_new(collection, _paths.size(), - unique); + unique, + useExpansion); } SkiplistIndex2::~SkiplistIndex2 () { @@ -189,13 +197,23 @@ int SkiplistIndex2::insert (TRI_doc_mptr_t const* doc, // insert into the index. the memory for the element will be owned or freed // by the index - for (auto& skiplistElement : elements) { - res = SkiplistIndex_insert(_skiplistIndex, skiplistElement); + size_t count = elements.size(); + for (size_t i = 0; i < count; ++i) { + res = _skiplistIndex->skiplist->insert(elements[i]); + if (res != TRI_ERROR_NO_ERROR) { - // TODO FIXME + TRI_index_element_t::free(elements[i]); + // Note: this element is freed already + for (size_t j = i + 1; j < count; ++j) { + TRI_index_element_t::free(elements[j]); + } + for (size_t j = 0; j < i; ++j) { + _skiplistIndex->skiplist->remove(elements[j]); + // No need to free elements[j] skiplist has taken over already + } + return res; } } - // TODO FIXME return res; } @@ -216,13 +234,10 @@ int SkiplistIndex2::remove (TRI_doc_mptr_t const* doc, // attempt the removal for skiplist indexes // ownership for the index element is transferred to the index - for (auto& skiplistElement : elements) { - res = SkiplistIndex_remove(_skiplistIndex, skiplistElement); - if (res != TRI_ERROR_NO_ERROR) { - // TODO FIXME - } + size_t count = elements.size(); + for (size_t i = 0; i < count; ++i) { + res = _skiplistIndex->skiplist->remove(elements[i]); } - // TODO FIXME return res; } diff --git a/arangod/SkipLists/skiplistIndex.cpp b/arangod/SkipLists/skiplistIndex.cpp index 9095563f4f..3c3b461e1d 100644 --- a/arangod/SkipLists/skiplistIndex.cpp +++ b/arangod/SkipLists/skiplistIndex.cpp @@ -123,12 +123,12 @@ static int CmpElmElm (void* sli, // The document could be the same -- so no further comparison is required. // .......................................................................... + SkiplistIndex* skiplistindex = static_cast(sli); if (leftElement == rightElement || - leftElement->document() == rightElement->document()) { + (!skiplistindex->skiplist->isArray() && leftElement->document() == rightElement->document())) { return 0; } - SkiplistIndex* skiplistindex = static_cast(sli); auto shaper = skiplistindex->_collection->getShaper(); // ONLY IN INDEX, PROTECTED by RUNTIME for (size_t j = 0; j < skiplistindex->_numFields; j++) { int compareResult = CompareElementElement(leftElement, @@ -204,7 +204,6 @@ static int CmpKeyElm (void* sli, static void FreeElm (void* e) { auto element = static_cast(e); TRI_index_element_t::free(element); - // TRI_Free(TRI_UNKNOWN_MEM_ZONE, element); } //////////////////////////////////////////////////////////////////////////////// @@ -438,7 +437,8 @@ void SkiplistIndex_free (SkiplistIndex* slIndex) { SkiplistIndex* SkiplistIndex_new (TRI_document_collection_t* document, size_t numFields, - bool unique) { + bool unique, + bool isArray) { SkiplistIndex* skiplistIndex = static_cast(TRI_Allocate(TRI_CORE_MEM_ZONE, sizeof(SkiplistIndex), true)); if (skiplistIndex == nullptr) { @@ -451,7 +451,7 @@ SkiplistIndex* SkiplistIndex_new (TRI_document_collection_t* document, try { skiplistIndex->skiplist = new triagens::basics::SkipList( CmpElmElm, CmpKeyElm, skiplistIndex, - FreeElm, unique); + FreeElm, unique, isArray); } catch (...) { TRI_Free(TRI_CORE_MEM_ZONE, skiplistIndex); @@ -515,7 +515,7 @@ static bool skiplistIndex_findHelperIntervalValid( compareResult = CmpElmElm( skiplistIndex, lNode->document(), rNode->document(), - triagens::basics::SKIPLIST_CMP_TOTORDER ); + triagens::basics::SKIPLIST_CMP_TOTORDER); return (compareResult == -1); // Since we know that the nodes are not neighbours, we can guarantee // at least one document in the interval. @@ -973,43 +973,6 @@ TRI_skiplist_iterator_t* SkiplistIndex_find (SkiplistIndex* skiplistIndex, return results; } -//////////////////////////////////////////////////////////////////////////////// -/// @brief inserts a data element into the skip list -/// ownership for the element is transferred to the index -//////////////////////////////////////////////////////////////////////////////// - -int SkiplistIndex_insert (SkiplistIndex* skiplistIndex, - TRI_index_element_t* element) { - int res = skiplistIndex->skiplist->insert(element); - - if (res != TRI_ERROR_NO_ERROR) { - TRI_index_element_t::free(element); - } - - return res; -} - -//////////////////////////////////////////////////////////////////////////////// -/// @brief removes an entry from the skip list -/// ownership for the element is transferred to the index -//////////////////////////////////////////////////////////////////////////////// - -int SkiplistIndex_remove (SkiplistIndex* skiplistIndex, - TRI_index_element_t* element) { - int res = skiplistIndex->skiplist->remove(element); - - TRI_index_element_t::free(element); - - if (res == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND) { - // This is for the case of a rollback in an aborted transaction. - // We silently ignore the fact that the document was not there. - // This could also be useful for the case of a sparse index. - return TRI_ERROR_NO_ERROR; - } - - return res; -} - //////////////////////////////////////////////////////////////////////////////// /// @brief returns the number of elements in the skip list index //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/SkipLists/skiplistIndex.h b/arangod/SkipLists/skiplistIndex.h index 5f0a7d2ce4..fc92875fc9 100644 --- a/arangod/SkipLists/skiplistIndex.h +++ b/arangod/SkipLists/skiplistIndex.h @@ -132,7 +132,7 @@ int SkiplistIndex_assignMethod (void*, TRI_index_method_assignment_type_e); //------------------------------------------------------------------------------ SkiplistIndex* SkiplistIndex_new (struct TRI_document_collection_t*, - size_t, bool); + size_t, bool, bool); TRI_skiplist_iterator_t* SkiplistIndex_find (SkiplistIndex*, TRI_vector_t const*, @@ -143,10 +143,6 @@ TRI_skiplist_iterator_t* SkiplistIndex_find (SkiplistIndex*, TRI_index_operator_t const*, bool); -int SkiplistIndex_insert (SkiplistIndex*, TRI_index_element_t*); - -int SkiplistIndex_remove (SkiplistIndex*, TRI_index_element_t*); - bool SkiplistIndex_update (SkiplistIndex*, const TRI_index_element_t*, const TRI_index_element_t*); diff --git a/arangod/V8Server/v8-query.cpp b/arangod/V8Server/v8-query.cpp index 1e0ec7f54f..eeed8057de 100644 --- a/arangod/V8Server/v8-query.cpp +++ b/arangod/V8Server/v8-query.cpp @@ -200,7 +200,7 @@ static TRI_index_operator_t* SetupConditionsSkiplist (v8::Isolate* isolate, size_t i = 0; for (auto const& field : fields) { std::string fieldString; - TRI_AttributeNamesToString(field, fieldString); + TRI_AttributeNamesToString(field, fieldString, true); v8::Handle key = TRI_V8_STD_STRING(fieldString); if (! conditions->HasOwnProperty(key)) { @@ -407,7 +407,7 @@ static TRI_index_operator_t* SetupExampleSkiplist (v8::Isolate* isolate, for (auto const& field : fields) { std::string fieldString; - TRI_AttributeNamesToString(field, fieldString); + TRI_AttributeNamesToString(field, fieldString, true); v8::Handle key = TRI_V8_STD_STRING(fieldString); if (! example->HasOwnProperty(key)) { diff --git a/lib/Basics/SkipList.h b/lib/Basics/SkipList.h index 9b006e9eb9..bd725f8c80 100644 --- a/lib/Basics/SkipList.h +++ b/lib/Basics/SkipList.h @@ -124,6 +124,8 @@ namespace triagens { bool _unique; // indicates whether multiple entries that // are equal in the preorder are allowed in uint64_t _nrUsed; + bool _isArray; // indicates whether this index is used to + // index arrays. size_t _memoryUsed; public: @@ -143,7 +145,8 @@ namespace triagens { SkipListCmpKeyElm cmp_key_elm, void* cmpdata, SkipListFreeFunc freefunc, - bool unique); + bool unique, + bool isArray); //////////////////////////////////////////////////////////////////////////////// /// @brief frees a skiplist and all its documents @@ -232,6 +235,14 @@ namespace triagens { return _memoryUsed; } +//////////////////////////////////////////////////////////////////////////////// +/// @brief returns if this indexed is used for arrays +//////////////////////////////////////////////////////////////////////////////// + + bool isArray () const { + return _isArray; + } + //////////////////////////////////////////////////////////////////////////////// /// @brief looks up doc in the skiplist using the proper order /// comparison.