From 8761e5abded512e5f2665dd16b0c91d38fdf2814 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Tue, 6 Oct 2015 09:41:30 +0200 Subject: [PATCH 1/2] Fixed broken nested loop. --- arangod/Aql/IndexBlock.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arangod/Aql/IndexBlock.cpp b/arangod/Aql/IndexBlock.cpp index cc1e2e73dd..5a2a3cefc9 100644 --- a/arangod/Aql/IndexBlock.cpp +++ b/arangod/Aql/IndexBlock.cpp @@ -353,7 +353,7 @@ int IndexBlock::initialize () { for (size_t i = 0; i < _condition->numMembers(); ++i) { auto andCond = _condition->getMember(i); for (size_t j = 0; j < andCond->numMembers(); ++j) { - auto leaf = andCond->getMember(i); + auto leaf = andCond->getMember(j); // We only support binary conditions TRI_ASSERT(leaf->numMembers() == 2); From 37a4ecfbb363f95743a022caf63b36e54dc0349a Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Tue, 6 Oct 2015 10:40:02 +0200 Subject: [PATCH 2/2] Modified the IndexIterator API to return TRI_doc_mptr_t instead of *_copy_t. Index Range node now manages a list of already returned documents, making the resulting documents distinct --- arangod/Aql/IndexBlock.cpp | 13 ++++++++++--- arangod/Aql/IndexBlock.h | 6 ++++++ arangod/Indexes/EdgeIndex.cpp | 6 ++---- arangod/Indexes/EdgeIndex.h | 2 +- arangod/Indexes/HashIndex.cpp | 11 +++++------ arangod/Indexes/HashIndex.h | 6 +++--- arangod/Indexes/Index.cpp | 19 +------------------ arangod/Indexes/Index.h | 6 +----- arangod/Indexes/PrimaryIndex.cpp | 4 ++-- arangod/Indexes/PrimaryIndex.h | 2 +- arangod/V8Server/v8-query.cpp | 4 ++-- 11 files changed, 34 insertions(+), 45 deletions(-) diff --git a/arangod/Aql/IndexBlock.cpp b/arangod/Aql/IndexBlock.cpp index 5a2a3cefc9..fb84b49ef8 100644 --- a/arangod/Aql/IndexBlock.cpp +++ b/arangod/Aql/IndexBlock.cpp @@ -312,6 +312,7 @@ int IndexBlock::initialize () { } _nonConstExpressions.clear(); + _alreadyReturned.clear(); // instantiate expressions: auto instantiateExpression = [&] (size_t i, size_t j, size_t k, AstNode const* a) -> void { @@ -404,6 +405,9 @@ bool IndexBlock::initIndexes () { ENTER_BLOCK _flag = true; + + // We start with a different context. Return documents found in the previous context again. + _alreadyReturned.clear(); // Find out about the actual values for the bounds in the variable bound case: if (! _nonConstExpressions.empty()) { @@ -522,7 +526,7 @@ bool IndexBlock::readIndex (size_t atMost) { try { size_t nrSent = 0; while (nrSent < atMost && _iterator != nullptr) { - TRI_doc_mptr_copy_t* indexElement = _iterator->next(); + TRI_doc_mptr_t* indexElement = _iterator->next(); if (indexElement == nullptr) { startNextIterator(); } @@ -530,9 +534,12 @@ bool IndexBlock::readIndex (size_t atMost) { TRI_IF_FAILURE("IndexBlock::readIndex") { THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); } + if (_alreadyReturned.find(indexElement) == _alreadyReturned.end()) { + _alreadyReturned.emplace(indexElement); - _documents.emplace_back(*indexElement); - ++nrSent; + _documents.emplace_back(*indexElement); + ++nrSent; + } ++_engine->_stats.scannedIndex; } } diff --git a/arangod/Aql/IndexBlock.h b/arangod/Aql/IndexBlock.h index 0ed9c959de..b74706d0ce 100644 --- a/arangod/Aql/IndexBlock.h +++ b/arangod/Aql/IndexBlock.h @@ -273,6 +273,12 @@ namespace triagens { AstNode const* _condition; +//////////////////////////////////////////////////////////////////////////////// +/// @brief set of already returned documents. Used to make the result distinct. +//////////////////////////////////////////////////////////////////////////////// + + std::unordered_set _alreadyReturned; + //////////////////////////////////////////////////////////////////////////////// /// @brief _condition: holds the complete condition this Block can serve for //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/Indexes/EdgeIndex.cpp b/arangod/Indexes/EdgeIndex.cpp index 912ed56130..189c5044eb 100644 --- a/arangod/Indexes/EdgeIndex.cpp +++ b/arangod/Indexes/EdgeIndex.cpp @@ -341,7 +341,7 @@ static bool IsEqualElementEdgeToByKey (void const* left, // ----------------------------------------------------------------------------- -TRI_doc_mptr_copy_t* EdgeIndexIterator::next () { +TRI_doc_mptr_t* EdgeIndexIterator::next () { if (_buffer == nullptr) { // We start a new lookup _buffer = _index->lookupByKey(&_searchValue, _batchSize); @@ -358,9 +358,7 @@ TRI_doc_mptr_copy_t* EdgeIndexIterator::next () { if (_buffer->empty()) { return nullptr; } - TRI_doc_mptr_copy_t* res = static_cast(_buffer->at(_posInBuffer)); - _posInBuffer++; - return res; + return _buffer->at(_posInBuffer++); } void EdgeIndexIterator::initCursor () { diff --git a/arangod/Indexes/EdgeIndex.h b/arangod/Indexes/EdgeIndex.h index 0257ef9e62..75155e3487 100644 --- a/arangod/Indexes/EdgeIndex.h +++ b/arangod/Indexes/EdgeIndex.h @@ -54,7 +54,7 @@ namespace triagens { typedef triagens::basics::AssocMulti TRI_EdgeIndexHash_t; - TRI_doc_mptr_copy_t* next () override; + TRI_doc_mptr_t* next () override; void initCursor () override; diff --git a/arangod/Indexes/HashIndex.cpp b/arangod/Indexes/HashIndex.cpp index 0ba6c3914c..f4bf33ccd4 100644 --- a/arangod/Indexes/HashIndex.cpp +++ b/arangod/Indexes/HashIndex.cpp @@ -121,7 +121,7 @@ static bool IsEqualKeyElementHash (TRI_index_search_value_t const* left, // --SECTION-- public methods // ----------------------------------------------------------------------------- -TRI_doc_mptr_copy_t* HashIndexIterator::next () { +TRI_doc_mptr_t* HashIndexIterator::next () { if (_buffer.empty()) { _index->lookup(&_searchValue, _buffer); if (_buffer.empty()) { @@ -129,8 +129,7 @@ TRI_doc_mptr_copy_t* HashIndexIterator::next () { } } if (_posInBuffer < _buffer.size()) { - _posInBuffer++; - return &(_buffer[_posInBuffer - 1]); + return _buffer[_posInBuffer++]; } return nullptr; } @@ -400,14 +399,14 @@ int HashIndex::sizeHint (size_t size) { //////////////////////////////////////////////////////////////////////////////// int HashIndex::lookup (TRI_index_search_value_t* searchValue, - std::vector& documents) const { + std::vector& documents) const { if (_unique) { TRI_index_element_t* found = _uniqueArray->_hashArray->findByKey(searchValue); if (found != nullptr) { // unique hash index: maximum number is 1 - documents.emplace_back(*(found->document())); + documents.emplace_back(found->document()); } return TRI_ERROR_NO_ERROR; } @@ -422,7 +421,7 @@ int HashIndex::lookup (TRI_index_search_value_t* searchValue, if (results != nullptr) { try { for (size_t i = 0; i < results->size(); i++) { - documents.emplace_back(*((*results)[i]->document())); + documents.emplace_back((*results)[i]->document()); } delete results; } diff --git a/arangod/Indexes/HashIndex.h b/arangod/Indexes/HashIndex.h index f45a1acdc7..a6f029aa65 100644 --- a/arangod/Indexes/HashIndex.h +++ b/arangod/Indexes/HashIndex.h @@ -65,7 +65,7 @@ namespace triagens { ~HashIndexIterator() {}; - TRI_doc_mptr_copy_t* next () override; + TRI_doc_mptr_t* next () override; void initCursor () override; @@ -73,7 +73,7 @@ namespace triagens { HashIndex const* _index; TRI_index_search_value_t _searchValue; - std::vector _buffer; + std::vector _buffer; size_t _posInBuffer; }; @@ -143,7 +143,7 @@ namespace triagens { //////////////////////////////////////////////////////////////////////////////// int lookup (TRI_index_search_value_t*, - std::vector&) const; + std::vector&) const; //////////////////////////////////////////////////////////////////////////////// /// @brief locates entries in the hash index given shaped json objects diff --git a/arangod/Indexes/Index.cpp b/arangod/Indexes/Index.cpp index 215cb97d49..5026a6bf9f 100644 --- a/arangod/Indexes/Index.cpp +++ b/arangod/Indexes/Index.cpp @@ -472,28 +472,11 @@ IndexIterator* Index::iteratorForCondition ( IndexIterator::~IndexIterator () { } -//////////////////////////////////////////////////////////////////////////////// -/// @brief default implementation for size -//////////////////////////////////////////////////////////////////////////////// - -size_t IndexIterator::size () const { - return 0; -} - -//////////////////////////////////////////////////////////////////////////////// -/// @brief default implementation for hasNext -//////////////////////////////////////////////////////////////////////////////// - -bool IndexIterator::hasNext () const { - return false; -} - - //////////////////////////////////////////////////////////////////////////////// /// @brief default implementation for next //////////////////////////////////////////////////////////////////////////////// -TRI_doc_mptr_copy_t* IndexIterator::next () { +TRI_doc_mptr_t* IndexIterator::next () { return nullptr; } diff --git a/arangod/Indexes/Index.h b/arangod/Indexes/Index.h index 5b4053b978..288dba17de 100644 --- a/arangod/Indexes/Index.h +++ b/arangod/Indexes/Index.h @@ -165,11 +165,7 @@ namespace triagens { virtual ~IndexIterator (); - virtual size_t size () const; - - virtual bool hasNext () const; - - virtual TRI_doc_mptr_copy_t* next (); + virtual TRI_doc_mptr_t* next (); virtual void initCursor (); }; diff --git a/arangod/Indexes/PrimaryIndex.cpp b/arangod/Indexes/PrimaryIndex.cpp index 8ceb587153..e9961b9eec 100644 --- a/arangod/Indexes/PrimaryIndex.cpp +++ b/arangod/Indexes/PrimaryIndex.cpp @@ -82,12 +82,12 @@ static bool IsEqualElementElement (TRI_doc_mptr_t const* left, // --SECTION-- public methods // ----------------------------------------------------------------------------- -TRI_doc_mptr_copy_t* PrimaryIndexIterator::next () { +TRI_doc_mptr_t* PrimaryIndexIterator::next () { if (_hasReturned) { return nullptr; } _hasReturned = true; - return static_cast(_index->lookupKey(_key)); + return _index->lookupKey(_key); } void PrimaryIndexIterator::initCursor () { diff --git a/arangod/Indexes/PrimaryIndex.h b/arangod/Indexes/PrimaryIndex.h index d11ae8f187..a16c74a6e1 100644 --- a/arangod/Indexes/PrimaryIndex.h +++ b/arangod/Indexes/PrimaryIndex.h @@ -65,7 +65,7 @@ namespace triagens { ~PrimaryIndexIterator () {}; - TRI_doc_mptr_copy_t* next () override; + TRI_doc_mptr_t* next () override; void initCursor () override; diff --git a/arangod/V8Server/v8-query.cpp b/arangod/V8Server/v8-query.cpp index fe13ac9d65..716f5431d1 100644 --- a/arangod/V8Server/v8-query.cpp +++ b/arangod/V8Server/v8-query.cpp @@ -1244,7 +1244,7 @@ static void ByExampleHashIndexQuery (SingleCollectionReadOnlyTransaction& trx, } // find the matches - std::vector list; + std::vector list; static_cast(idx)->lookup(&searchValue, list); DestroySearchValue(shaper->memoryZone(), searchValue); @@ -1261,7 +1261,7 @@ static void ByExampleHashIndexQuery (SingleCollectionReadOnlyTransaction& trx, for (uint64_t i = s; i < e; ++i) { v8::Handle doc = WRAP_SHAPED_JSON(trx, collection->_cid, - list[i].getDataPtr()); + list[i]->getDataPtr()); if (doc.IsEmpty()) { error = true;