From d83e3acfc9eb6db15ac53a25f88626eef4715976 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Mon, 2 Dec 2019 17:02:26 +0100 Subject: [PATCH] Final modification of ShadowRows in FilterExecutor. All but profiling tests pass. This needs some discussion about overfetching of data, and propagation of hardLimit --- arangod/Aql/AqlItemBlockInputRange.cpp | 41 ++++++------------------ arangod/Aql/AqlItemBlockInputRange.h | 4 --- arangod/Aql/ExecutionBlockImpl.cpp | 7 ++-- arangod/Aql/FilterExecutor.cpp | 8 ++++- tests/Aql/AqlItemBlockInputRangeTest.cpp | 1 + 5 files changed, 23 insertions(+), 38 deletions(-) diff --git a/arangod/Aql/AqlItemBlockInputRange.cpp b/arangod/Aql/AqlItemBlockInputRange.cpp index 974f4c0ee3..3acf0bd191 100644 --- a/arangod/Aql/AqlItemBlockInputRange.cpp +++ b/arangod/Aql/AqlItemBlockInputRange.cpp @@ -39,22 +39,20 @@ AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state) AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state, SharedAqlItemBlockPtr const& block, - std::size_t index, std::size_t endIndex) - : _block{block}, _rowIndex{index}, _endIndex(endIndex), _finalState{state} { - TRI_ASSERT(index <= endIndex); - TRI_ASSERT(endIndex <= block->size()); + std::size_t index, std::size_t) + : _block{block}, _rowIndex{index}, _endIndex(_block->size()), _finalState{state} { + TRI_ASSERT(index <= _block->size()); } AqlItemBlockInputRange::AqlItemBlockInputRange(ExecutorState state, SharedAqlItemBlockPtr&& block, - std::size_t index, std::size_t endIndex) noexcept - : _block{std::move(block)}, _rowIndex{index}, _endIndex(endIndex), _finalState{state} { - TRI_ASSERT(index <= endIndex); - TRI_ASSERT(endIndex <= block->size()); + std::size_t index, std::size_t) noexcept + : _block{std::move(block)}, _rowIndex{index}, _endIndex(_block->size()), _finalState{state} { + TRI_ASSERT(index <= _block->size()); } std::pair AqlItemBlockInputRange::peek() { - if (indexIsValid()) { + if (hasMore()) { return std::make_pair(nextState(), InputAqlItemRow{_block, _rowIndex}); } @@ -70,14 +68,8 @@ std::pair AqlItemBlockInputRange::next() { return res; } -bool AqlItemBlockInputRange::indexIsValid() const noexcept { - return _block != nullptr && _rowIndex < _endIndex; -} - -bool AqlItemBlockInputRange::hasMore() const noexcept { return indexIsValid(); } - -bool AqlItemBlockInputRange::hasMoreAfterThis() const noexcept { - return indexIsValid() && _rowIndex + 1 < _endIndex; +bool AqlItemBlockInputRange::hasMore() const noexcept { + return isIndexValid(_rowIndex) && !isShadowRowAtIndex(_rowIndex); } ExecutorState AqlItemBlockInputRange::state() const noexcept { @@ -108,20 +100,7 @@ std::pair AqlItemBlockInputRange::peekShadowRow std::pair AqlItemBlockInputRange::nextShadowRow() { auto res = peekShadowRow(); - if (hasShadowRow()) { - auto const& shadowRowIndexes = _block->getShadowRowIndexes(); - auto it = std::find(shadowRowIndexes.begin(), shadowRowIndexes.end(), _rowIndex); - // We have a shadow row in this index, so we cannot be at the end now. - TRI_ASSERT(it != shadowRowIndexes.end()); - // Go to next ShadowRow. - it++; - if (it == shadowRowIndexes.end()) { - // No more shadow row here. - _endIndex = _block->size(); - } else { - // Set endIndex to the next ShadowRowIndex. - _endIndex = *it; - } + if (res.second.isInitialized()) { // Advance the current row. _rowIndex++; } diff --git a/arangod/Aql/AqlItemBlockInputRange.h b/arangod/Aql/AqlItemBlockInputRange.h index 85b1cafed5..e0a6a5827c 100644 --- a/arangod/Aql/AqlItemBlockInputRange.h +++ b/arangod/Aql/AqlItemBlockInputRange.h @@ -61,10 +61,6 @@ class AqlItemBlockInputRange { private: bool isIndexValid(std::size_t index) const noexcept; - bool indexIsValid() const noexcept; - - bool hasMoreAfterThis() const noexcept; - bool isShadowRowAtIndex(std::size_t index) const noexcept; enum LookAhead { NOW, NEXT }; diff --git a/arangod/Aql/ExecutionBlockImpl.cpp b/arangod/Aql/ExecutionBlockImpl.cpp index b3bde87744..50eb4f63ff 100644 --- a/arangod/Aql/ExecutionBlockImpl.cpp +++ b/arangod/Aql/ExecutionBlockImpl.cpp @@ -1017,7 +1017,7 @@ ExecutionBlockImpl::executeWithoutTrace(AqlCallStack stack) { } AqlCall executorRequest; - while (execState != ExecState::DONE) { + while (execState != ExecState::DONE && !_outputItemRow->isFull()) { switch (execState) { case ExecState::SKIP: { auto [state, skippedLocal, call] = @@ -1028,6 +1028,7 @@ ExecutionBlockImpl::executeWithoutTrace(AqlCallStack stack) { if (state == ExecutorState::DONE) { execState = ExecState::SHADOWROWS; } else if (myCall.getOffset() > 0) { + TRI_ASSERT(_upstreamState != ExecutionState::DONE); // We need to request more executorRequest = call; execState = ExecState::UPSTREAM; @@ -1040,15 +1041,17 @@ ExecutionBlockImpl::executeWithoutTrace(AqlCallStack stack) { case ExecState::PRODUCE: { auto linesBefore = _outputItemRow->numRowsWritten(); TRI_ASSERT(myCall.getLimit() > 0); + auto limit = (std::min)(myCall.getLimit(), _outputItemRow->numRowsLeft()); // Execute getSome auto const [state, stats, call] = - _executor.produceRows(myCall.getLimit(), _lastRange, *_outputItemRow); + _executor.produceRows(limit, _lastRange, *_outputItemRow); auto written = _outputItemRow->numRowsWritten() - linesBefore; _engine->_stats += stats; myCall.didProduce(written); if (state == ExecutorState::DONE) { execState = ExecState::SHADOWROWS; } else if (myCall.getLimit() > 0) { + TRI_ASSERT(_upstreamState != ExecutionState::DONE); // We need to request more executorRequest = call; execState = ExecState::UPSTREAM; diff --git a/arangod/Aql/FilterExecutor.cpp b/arangod/Aql/FilterExecutor.cpp index e96a83e038..dd40309be6 100644 --- a/arangod/Aql/FilterExecutor.cpp +++ b/arangod/Aql/FilterExecutor.cpp @@ -35,6 +35,8 @@ #include "Aql/SingleRowFetcher.h" #include "Aql/Stats.h" +#include "Logger/LogMacros.h" + #include using namespace arangodb; @@ -143,6 +145,10 @@ std::tuple FilterExecutor::produceRows( } AqlCall upstreamCall{}; - upstreamCall.softLimit = limit; + /* We can use this value as a heuristic on overfetching. + * by default we do not skip, and do not set any soft or hardLimit + * on upstream + * upstreamCall.softLimit = limit; + */ return {inputRange.state(), stats, upstreamCall}; } diff --git a/tests/Aql/AqlItemBlockInputRangeTest.cpp b/tests/Aql/AqlItemBlockInputRangeTest.cpp index 614c70ea7f..c5180e23ec 100644 --- a/tests/Aql/AqlItemBlockInputRangeTest.cpp +++ b/tests/Aql/AqlItemBlockInputRangeTest.cpp @@ -128,6 +128,7 @@ class InputRangeTest : public ::testing::TestWithParam { ASSERT_NE(rowIndexBefore, testee.getRowIndex()) << "Did not go to next row."; } + EXPECT_EQ(expectedState, testee.state()); } void validateNextIsShadowRow(AqlItemBlockInputRange& testee, ExecutorState expectedState,