The skiplist index is now able to index arrays as well

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<int*> 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<int*> 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<int*> values; 
   for (int i = 0; i < 100; ++i) {

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) {
+  size_t count = elements.size();
-    res = SkiplistIndex_insert(_skiplistIndex, skiplistElement);
+  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) {
+  size_t count = elements.size();
-    res = SkiplistIndex_remove(_skiplistIndex, skiplistElement);
+  for (size_t i = 0; i < count; ++i) {
-    if (res != TRI_ERROR_NO_ERROR) {
+    res = _skiplistIndex->skiplist->remove(elements[i]);
-      // TODO FIXME
   }
-  }
-  // TODO FIXME
   return res;
 }
 

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<SkiplistIndex*>(sli);
   if (leftElement == rightElement ||
-      leftElement->document() == rightElement->document()) {
+      (!skiplistindex->skiplist->isArray() && leftElement->document() == rightElement->document())) {
     return 0;
   }
 
-  SkiplistIndex* skiplistindex = static_cast<SkiplistIndex*>(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<TRI_index_element_t*>(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<SkiplistIndex*>(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);
@@ -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
 ////////////////////////////////////////////////////////////////////////////////

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*);
 

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<v8::String> 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<v8::String> key = TRI_V8_STD_STRING(fieldString);
 
     if (! example->HasOwnProperty(key)) {

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.