From 79117675feabd88b7a6030194ffb01cca60bfa11 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Wed, 20 Nov 2019 11:17:24 +0100 Subject: [PATCH] single row fetcher, execute + tests --- arangod/Aql/AqlItemBlock.cpp | 19 ++++++- arangod/Aql/AqlItemBlock.h | 8 +++ arangod/Aql/AqlItemBlockInputRange.h | 7 ++- arangod/Aql/DependencyProxy.h | 2 +- arangod/Aql/SingleRowFetcher.cpp | 18 ++++--- tests/Aql/DependencyProxyMock.cpp | 11 ++++ tests/Aql/DependencyProxyMock.h | 4 ++ tests/Aql/SingleRowFetcherTest.cpp | 79 ++++++++++++++++++++++++++++ 8 files changed, 136 insertions(+), 12 deletions(-) diff --git a/arangod/Aql/AqlItemBlock.cpp b/arangod/Aql/AqlItemBlock.cpp index d3bd739c5c..7192a4a70f 100644 --- a/arangod/Aql/AqlItemBlock.cpp +++ b/arangod/Aql/AqlItemBlock.cpp @@ -69,7 +69,7 @@ inline void CopyValueOver(std::unordered_set& cache, AqlValue const& a /// @brief create the block AqlItemBlock::AqlItemBlock(AqlItemBlockManager& manager, size_t nrItems, RegisterId nrRegs) - : _nrItems(nrItems), _nrRegs(nrRegs), _manager(manager), _refCount(0) { + : _nrItems(nrItems), _nrRegs(nrRegs), _manager(manager), _refCount(0), _rowIndex(0) { TRI_ASSERT(nrItems > 0); // empty AqlItemBlocks are not allowed! // check that the nrRegs value is somewhat sensible // this compare value is arbitrary, but having so many registers in a single @@ -855,6 +855,23 @@ RegisterId AqlItemBlock::getNrRegs() const noexcept { return _nrRegs; } size_t AqlItemBlock::size() const noexcept { return _nrItems; } +std::tuple AqlItemBlock::getRelevantRange() { + size_t startIndex = _rowIndex; + size_t endIndex = 0; + + for (; _rowIndex < this->size(); _rowIndex++) { + if (isShadowRow(_rowIndex)) { + endIndex = _rowIndex - 1; + break; + } + if (_rowIndex - 1 != this->size()) { + endIndex = _rowIndex; + } + } + + return std::make_pair(startIndex, endIndex); +} + size_t AqlItemBlock::numEntries() const { return internalNrRegs() * _nrItems; } size_t AqlItemBlock::capacity() const noexcept { return _data.capacity(); } diff --git a/arangod/Aql/AqlItemBlock.h b/arangod/Aql/AqlItemBlock.h index d8d3288f88..df3b7c03ce 100644 --- a/arangod/Aql/AqlItemBlock.h +++ b/arangod/Aql/AqlItemBlock.h @@ -166,6 +166,9 @@ class AqlItemBlock { /// @brief getter for _nrItems size_t size() const noexcept; + /// @brief get the relevant consumable range of the block + std::tuple getRelevantRange(); + /// @brief Number of entries in the matrix. If this changes, the memory usage /// must be / in- or decreased appropriately as well. /// All entries _data[i] for numEntries() <= i < _data.size() always have to @@ -287,6 +290,11 @@ class AqlItemBlock { /// @brief A list of indexes with all shadowRows within /// this ItemBlock. Used to easier split data based on them. std::set _shadowRowIndexes; + + /// @brief current row index we want to read from. This will be increased after + /// getRelevantRange function will be called, which will return a tuple of the + /// old _rowIndex and the newly calculated _rowIndex - 1 + size_t _rowIndex; }; } // namespace aql diff --git a/arangod/Aql/AqlItemBlockInputRange.h b/arangod/Aql/AqlItemBlockInputRange.h index 01eef15124..bfb264835d 100644 --- a/arangod/Aql/AqlItemBlockInputRange.h +++ b/arangod/Aql/AqlItemBlockInputRange.h @@ -34,9 +34,9 @@ class AqlItemBlockInputRange { explicit AqlItemBlockInputRange(ExecutorState state); AqlItemBlockInputRange(ExecutorState, arangodb::aql::SharedAqlItemBlockPtr const&, - std::size_t, std::size_t endIndex); + std::size_t startIndex, std::size_t endIndex); AqlItemBlockInputRange(ExecutorState, arangodb::aql::SharedAqlItemBlockPtr&&, - std::size_t, std::size_t endIndex) noexcept; + std::size_t startIndex, std::size_t endIndex) noexcept; bool hasMore() const noexcept; @@ -46,6 +46,9 @@ class AqlItemBlockInputRange { std::pair next(); + std::size_t getRowIndex() noexcept { return _rowIndex; }; + std::size_t getEndIndex() noexcept { return _endIndex; }; + private: bool indexIsValid() const noexcept; diff --git a/arangod/Aql/DependencyProxy.h b/arangod/Aql/DependencyProxy.h index 36d9467985..bbee1df2ae 100644 --- a/arangod/Aql/DependencyProxy.h +++ b/arangod/Aql/DependencyProxy.h @@ -74,7 +74,7 @@ class DependencyProxy { TEST_VIRTUAL ~DependencyProxy() = default; // TODO Implement and document properly! - std::tuple execute(AqlCallStack& stack); + TEST_VIRTUAL std::tuple execute(AqlCallStack& stack); // This is only TEST_VIRTUAL, so we ignore this lint warning: // NOLINTNEXTLINE google-default-arguments diff --git a/arangod/Aql/SingleRowFetcher.cpp b/arangod/Aql/SingleRowFetcher.cpp index 34219bde20..cdf502e2ef 100644 --- a/arangod/Aql/SingleRowFetcher.cpp +++ b/arangod/Aql/SingleRowFetcher.cpp @@ -24,6 +24,7 @@ //////////////////////////////////////////////////////////////////////////////// #include "SingleRowFetcher.h" +#include #include "Aql/AqlItemBlock.h" #include "Aql/DependencyProxy.h" @@ -76,21 +77,22 @@ SingleRowFetcher::fetchBlockForPassthrough(size_t atMost) { template std::tuple SingleRowFetcher::execute(AqlCallStack& stack) { - auto const [state, skipped, block] = _dependencyProxy->execute(stack); + auto [state, skipped, block] = _dependencyProxy->execute(stack); if (state == ExecutionState::WAITING) { // On waiting we have nothing to return return {state, 0, AqlItemBlockInputRange{ExecutorState::HASMORE}}; } - if (state == ExecutionState::HASMORE) { - TRI_ASSERT(block != nullptr); - return {state, skipped, - AqlItemBlockInputRange{ExecutorState::HASMORE, block, 0, block->size()}}; - } if (block == nullptr) { return {state, skipped, AqlItemBlockInputRange{ExecutorState::DONE}}; } - return {state, skipped, - AqlItemBlockInputRange{ExecutorState::DONE, block, 0, block->size()}}; + + auto [start, end] = block->getRelevantRange(); + if (state == ExecutionState::HASMORE) { + TRI_ASSERT(block != nullptr); + return {state, skipped, + AqlItemBlockInputRange{ExecutorState::HASMORE, block, start, end}}; + } + return {state, skipped, AqlItemBlockInputRange{ExecutorState::DONE, block, start, end}}; } template diff --git a/tests/Aql/DependencyProxyMock.cpp b/tests/Aql/DependencyProxyMock.cpp index f17bcd9098..ecec7d319c 100644 --- a/tests/Aql/DependencyProxyMock.cpp +++ b/tests/Aql/DependencyProxyMock.cpp @@ -21,6 +21,7 @@ //////////////////////////////////////////////////////////////////////////////// #include "DependencyProxyMock.h" +#include #include "gtest/gtest.h" @@ -104,6 +105,9 @@ DependencyProxyMock& DependencyProxyMock:: for (RegisterId i = 0; i < this->getNrInputRegisters(); i++) { inputRegisters->emplace(i); } + // keep the block address + _block = block; + return andThenReturn({state, block}); } @@ -125,6 +129,13 @@ DependencyProxyMock& DependencyProxyMock:: return *this; } +template +std::tuple +DependencyProxyMock::execute(AqlCallStack& stack) { + TRI_ASSERT(_block != nullptr); + return {arangodb::aql::ExecutionState::DONE, 0, _block}; +} + template bool DependencyProxyMock::allBlocksFetched() const { return _itemsToReturn.empty(); diff --git a/tests/Aql/DependencyProxyMock.h b/tests/Aql/DependencyProxyMock.h index 2692dd344e..9fd2f8ea2d 100644 --- a/tests/Aql/DependencyProxyMock.h +++ b/tests/Aql/DependencyProxyMock.h @@ -51,6 +51,9 @@ class DependencyProxyMock : public ::arangodb::aql::DependencyProxy skipSome(size_t atMost) override; + std::tuple execute( + arangodb::aql::AqlCallStack& stack) override; + private: using FetchBlockReturnItem = std::pair; @@ -76,6 +79,7 @@ class DependencyProxyMock : public ::arangodb::aql::DependencyProxy diff --git a/tests/Aql/SingleRowFetcherTest.cpp b/tests/Aql/SingleRowFetcherTest.cpp index 07a5ab6ab1..4dcb098f34 100644 --- a/tests/Aql/SingleRowFetcherTest.cpp +++ b/tests/Aql/SingleRowFetcherTest.cpp @@ -28,6 +28,7 @@ #include "RowFetcherHelper.h" #include "gtest/gtest.h" +#include "Aql/AqlCallStack.h" #include "Aql/AqlItemBlock.h" #include "Aql/DependencyProxy.h" #include "Aql/ExecutionBlock.h" @@ -1147,6 +1148,84 @@ TEST_F(SingleRowFetcherTestPassBlocks, handling_consecutive_shadowrows) { ASSERT_EQ(dependencyProxyMock.numFetchBlockCalls(), 1); } +TEST_F(SingleRowFetcherTestPassBlocks, handling_shadowrows_in_execute_oneAndDone) { + DependencyProxyMock dependencyProxyMock{monitor, 1}; + InputAqlItemRow row{CreateInvalidInputRowHint{}}; + ShadowAqlItemRow shadow{CreateInvalidShadowRowHint{}}; + + { + SharedAqlItemBlockPtr block{new AqlItemBlock(itemBlockManager, 7, 1)}; + block->emplaceValue(0, 0, "a"); + block->emplaceValue(1, 0, "b"); + block->emplaceValue(2, 0, "c"); + block->emplaceValue(3, 0, "d"); + block->emplaceValue(4, 0, "e"); // first shadowrow + block->setShadowRowDepth(4, AqlValue(AqlValueHintUInt(1ull))); + block->emplaceValue(5, 0, "f"); + block->setShadowRowDepth(5, AqlValue(AqlValueHintUInt(0ull))); + block->emplaceValue(6, 0, "g"); + block->setShadowRowDepth(6, AqlValue(AqlValueHintUInt(0ull))); + dependencyProxyMock.shouldReturn(ExecutionState::DONE, std::move(block)); + } + + { + SingleRowFetcher testee(dependencyProxyMock); + AqlCall call; + AqlCallStack stack = {call}; + + // First no data row + auto [state, skipped, input] = testee.execute(stack); + EXPECT_EQ(input.getRowIndex(), 0); + EXPECT_EQ(input.getEndIndex(), 3); + EXPECT_EQ(skipped, 0); + EXPECT_EQ(state, ExecutionState::DONE); + } // testee is destroyed here +} + +TEST_F(SingleRowFetcherTestPassBlocks, handling_shadowrows_in_execute_twoAndHasMore) { + DependencyProxyMock dependencyProxyMock{monitor, 1}; + InputAqlItemRow row{CreateInvalidInputRowHint{}}; + ShadowAqlItemRow shadow{CreateInvalidShadowRowHint{}}; + + { + SharedAqlItemBlockPtr block{new AqlItemBlock(itemBlockManager, 9, 1)}; + block->emplaceValue(0, 0, "a"); + block->emplaceValue(1, 0, "b"); + block->emplaceValue(2, 0, "c"); + block->emplaceValue(3, 0, "d"); + block->emplaceValue(4, 0, "e"); // first shadowrow + block->setShadowRowDepth(4, AqlValue(AqlValueHintUInt(1ull))); + block->emplaceValue(5, 0, "f"); + block->setShadowRowDepth(5, AqlValue(AqlValueHintUInt(0ull))); + block->emplaceValue(6, 0, "g"); + block->setShadowRowDepth(6, AqlValue(AqlValueHintUInt(0ull))); + block->emplaceValue(7, 0, "h"); + block->emplaceValue(8, 0, "i"); + dependencyProxyMock.shouldReturn(ExecutionState::DONE, std::move(block)); + } + + { + SingleRowFetcher testee(dependencyProxyMock); + AqlCall call; + AqlCallStack stack = {call}; + + { + auto [state, skipped, input] = testee.execute(stack); + EXPECT_EQ(input.getRowIndex(), 0); + EXPECT_EQ(input.getEndIndex(), 3); + EXPECT_EQ(state, ExecutionState::HASMORE); + // EXPECT_EQ(skipped, 0); + } + + { + auto [state, skipped, input] = testee.execute(stack); + EXPECT_EQ(input.getRowIndex(), 7); + EXPECT_EQ(input.getEndIndex(), 8); + EXPECT_EQ(state, ExecutionState::DONE); + } + } // testee is destroyed here +} + class SingleRowFetcherWrapper : public fetcherHelper::PatternTestWrapper> { public: