From 066ed742af1e8fdc796508ad67b44297386a753f Mon Sep 17 00:00:00 2001 From: Heiko Date: Thu, 4 Apr 2019 15:57:41 +0200 Subject: [PATCH] Bug fix/aql line cleanup (#8598) --- arangod/Aql/CollectNode.h | 3 - arangod/Aql/ConstrainedSortExecutor.h | 1 - arangod/Aql/DistinctCollectExecutor.h | 3 - arangod/Aql/DistributeExecutor.h | 2 - arangod/Aql/DocumentProducingBlock.cpp | 199 ----------------------- arangod/Aql/DocumentProducingBlock.h | 77 --------- arangod/Aql/DocumentProducingNode.h | 4 - arangod/Aql/ExecutionBlock.h | 2 +- arangod/Aql/Functions.cpp | 1 - arangod/Aql/HashedCollectExecutor.h | 5 - arangod/Aql/IdExecutor.h | 1 - arangod/Aql/IndexExecutor.cpp | 32 ++-- arangod/Aql/IndexExecutor.h | 15 +- arangod/Aql/LimitExecutor.h | 1 - arangod/Aql/ModificationExecutorTraits.h | 2 - arangod/Aql/Query.h | 3 - arangod/Aql/RestAqlHandler.cpp | 2 +- arangod/Aql/ScatterExecutor.h | 2 - arangod/CMakeLists.txt | 1 - 19 files changed, 28 insertions(+), 328 deletions(-) delete mode 100644 arangod/Aql/DocumentProducingBlock.cpp delete mode 100644 arangod/Aql/DocumentProducingBlock.h diff --git a/arangod/Aql/CollectNode.h b/arangod/Aql/CollectNode.h index e8f6e2256a..4cd0be5cab 100644 --- a/arangod/Aql/CollectNode.h +++ b/arangod/Aql/CollectNode.h @@ -102,9 +102,6 @@ class CollectNode : public ExecutionNode { _options.method = method; } - /// @brief getOptions - CollectOptions const& getOptions() const { return _options; } - /// @brief getOptions CollectOptions& getOptions() { return _options; } diff --git a/arangod/Aql/ConstrainedSortExecutor.h b/arangod/Aql/ConstrainedSortExecutor.h index a244712056..6376038483 100644 --- a/arangod/Aql/ConstrainedSortExecutor.h +++ b/arangod/Aql/ConstrainedSortExecutor.h @@ -89,7 +89,6 @@ class ConstrainedSortExecutor { Infos& _infos; Fetcher& _fetcher; ExecutionState _state; - std::vector _sortedIndexes; size_t _returnNext; std::vector _rows; size_t _rowsPushed; diff --git a/arangod/Aql/DistinctCollectExecutor.h b/arangod/Aql/DistinctCollectExecutor.h index 59a9f9830f..70b4209c27 100644 --- a/arangod/Aql/DistinctCollectExecutor.h +++ b/arangod/Aql/DistinctCollectExecutor.h @@ -71,9 +71,6 @@ class DistinctCollectExecutorInfos : public ExecutorInfos { /// @brief pairs, consisting of out register and in register std::vector> _groupRegisters; - /// @brief input/output variables for the collection (out, in) - std::vector> _groupVariables; - /// @brief the transaction for this query transaction::Methods* _trxPtr; }; diff --git a/arangod/Aql/DistributeExecutor.h b/arangod/Aql/DistributeExecutor.h index bbcab1319e..f62370d36d 100644 --- a/arangod/Aql/DistributeExecutor.h +++ b/arangod/Aql/DistributeExecutor.h @@ -102,8 +102,6 @@ class ExecutionBlockImpl : public BlockWithClients { ExecutorInfos const& infos() const { return _infos; } - Query const& getQuery() const { return _query; } - private: ExecutorInfos _infos; diff --git a/arangod/Aql/DocumentProducingBlock.cpp b/arangod/Aql/DocumentProducingBlock.cpp deleted file mode 100644 index 48edb2a90c..0000000000 --- a/arangod/Aql/DocumentProducingBlock.cpp +++ /dev/null @@ -1,199 +0,0 @@ -//////////////////////////////////////////////////////////////////////////////// -/// DISCLAIMER -/// -/// Copyright 2014-2016 ArangoDB GmbH, Cologne, Germany -/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany -/// -/// Licensed under the Apache License, Version 2.0 (the "License"); -/// you may not use this file except in compliance with the License. -/// You may obtain a copy of the License at -/// -/// http://www.apache.org/licenses/LICENSE-2.0 -/// -/// Unless required by applicable law or agreed to in writing, software -/// distributed under the License is distributed on an "AS IS" BASIS, -/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -/// See the License for the specific language governing permissions and -/// limitations under the License. -/// -/// Copyright holder is ArangoDB GmbH, Cologne, Germany -/// -/// @author Jan Steemann -//////////////////////////////////////////////////////////////////////////////// - -#include "DocumentProducingBlock.h" -#include "Aql/AqlItemBlock.h" -#include "Aql/DocumentProducingNode.h" -#include "Aql/ExecutionNode.h" -#include "Aql/IndexNode.h" -#include "Aql/Variable.h" -#include "Basics/Exceptions.h" -#include "Basics/StaticStrings.h" -#include "StorageEngine/EngineSelectorFeature.h" -#include "StorageEngine/StorageEngine.h" -#include "Transaction/Helpers.h" - -using namespace arangodb; -using namespace arangodb::aql; - -namespace { -inline void handleProjections(DocumentProducingNode const* node, - transaction::Methods const* trxPtr, VPackSlice slice, - VPackBuilder& b, bool useRawDocumentPointers) { - for (auto const& it : node->projections()) { - if (it == StaticStrings::IdString) { - VPackSlice found = transaction::helpers::extractIdFromDocument(slice); - if (found.isCustom()) { - // _id as a custom type needs special treatment - b.add(it, VPackValue(transaction::helpers::extractIdString(trxPtr->resolver(), - found, slice))); - } else { - b.add(it, found); - } - } else if (it == StaticStrings::KeyString) { - VPackSlice found = transaction::helpers::extractKeyFromDocument(slice); - if (useRawDocumentPointers) { - b.add(VPackValue(it)); - b.addExternal(found.begin()); - } else { - b.add(it, found); - } - } else { - VPackSlice found = slice.get(it); - if (found.isNone()) { - // attribute not found - b.add(it, VPackValue(VPackValueType::Null)); - } else { - if (useRawDocumentPointers) { - b.add(VPackValue(it)); - b.addExternal(found.begin()); - } else { - b.add(it, found); - } - } - } - } -} - -} // namespace - -DocumentProducingBlock::DocumentProducingBlock(DocumentProducingNode const* node, - transaction::Methods* trx) - : _trxPtr(trx), - _node(node), - _produceResult(dynamic_cast(_node)->isVarUsedLater( - _node->outVariable())), - _useRawDocumentPointers(EngineSelectorFeature::ENGINE->useRawDocumentPointers()), - _allowCoveringIndexOptimization(true) {} - -void DocumentProducingBlock::buildCallback() { - if (!_produceResult) { - // no result needed - _documentProducer = [](AqlItemBlock* res, VPackSlice, size_t registerId, - size_t& row, size_t fromRow) { - if (row != fromRow) { - // re-use already copied AQLValues - res->copyValuesFromRow(row, static_cast(registerId), fromRow); - } - ++row; - }; - return; - } - - if (!_node->projections().empty()) { - // return a projection - if (!_node->coveringIndexAttributePositions().empty()) { - // projections from an index value (covering index) - _documentProducer = [this](AqlItemBlock* res, VPackSlice slice, - size_t registerId, size_t& row, size_t fromRow) { - transaction::BuilderLeaser b(_trxPtr); - b->openObject(true); - - if (_allowCoveringIndexOptimization) { - // a potential call by a covering index iterator... - bool const isArray = slice.isArray(); - size_t i = 0; - VPackSlice found; - for (auto const& it : _node->projections()) { - if (isArray) { - // we will get a Slice with an array of index values. now we need - // to look up the array values from the correct positions to - // populate the result with the projection values this case will - // be triggered for indexes that can be set up on any number of - // attributes (hash/skiplist) - found = slice.at(_node->coveringIndexAttributePositions()[i]); - ++i; - } else { - // no array Slice... this case will be triggered for indexes that - // contain simple string values, such as the primary index or the - // edge index - found = slice; - } - if (found.isNone()) { - // attribute not found - b->add(it, VPackValue(VPackValueType::Null)); - } else { - if (_useRawDocumentPointers) { - b->add(VPackValue(it)); - b->addExternal(found.begin()); - } else { - b->add(it, found); - } - } - } - } else { - // projections from a "real" document - handleProjections(_node, _trxPtr, slice, *b.get(), _useRawDocumentPointers); - } - - b->close(); - - res->emplaceValue(row, static_cast(registerId), - b.get()); - - if (row != fromRow) { - // re-use already copied AQLValues - res->copyValuesFromRow(row, static_cast(registerId), fromRow); - } - ++row; - }; - return; - } - - // projections from a "real" document - _documentProducer = [this](AqlItemBlock* res, VPackSlice slice, - size_t registerId, size_t& row, size_t fromRow) { - transaction::BuilderLeaser b(_trxPtr); - b->openObject(true); - handleProjections(_node, _trxPtr, slice, *b.get(), _useRawDocumentPointers); - b->close(); - - res->emplaceValue(row, static_cast(registerId), b.get()); - - if (row != fromRow) { - // re-use already copied AQLValues - res->copyValuesFromRow(row, static_cast(registerId), fromRow); - } - ++row; - }; - return; - } - - // return the document as is - _documentProducer = [this](AqlItemBlock* res, VPackSlice slice, - size_t registerId, size_t& row, size_t fromRow) { - uint8_t const* vpack = slice.begin(); - if (_useRawDocumentPointers) { - res->emplaceValue(row, static_cast(registerId), - AqlValueHintDocumentNoCopy(vpack)); - } else { - res->emplaceValue(row, static_cast(registerId), - AqlValueHintCopy(vpack)); - } - if (row != fromRow) { - // re-use already copied AQLValues - res->copyValuesFromRow(row, static_cast(registerId), fromRow); - } - ++row; - }; -} diff --git a/arangod/Aql/DocumentProducingBlock.h b/arangod/Aql/DocumentProducingBlock.h deleted file mode 100644 index 9c8c0600a1..0000000000 --- a/arangod/Aql/DocumentProducingBlock.h +++ /dev/null @@ -1,77 +0,0 @@ -//////////////////////////////////////////////////////////////////////////////// -/// DISCLAIMER -/// -/// Copyright 2014-2016 ArangoDB GmbH, Cologne, Germany -/// Copyright 2004-2014 triAGENS GmbH, Cologne, Germany -/// -/// Licensed under the Apache License, Version 2.0 (the "License"); -/// you may not use this file except in compliance with the License. -/// You may obtain a copy of the License at -/// -/// http://www.apache.org/licenses/LICENSE-2.0 -/// -/// Unless required by applicable law or agreed to in writing, software -/// distributed under the License is distributed on an "AS IS" BASIS, -/// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -/// See the License for the specific language governing permissions and -/// limitations under the License. -/// -/// Copyright holder is ArangoDB GmbH, Cologne, Germany -/// -/// @author Jan Steemann -//////////////////////////////////////////////////////////////////////////////// - -#ifndef ARANGOD_AQL_DOCUMENT_PRODUCING_BLOCK_H -#define ARANGOD_AQL_DOCUMENT_PRODUCING_BLOCK_H 1 - -#include "Basics/Common.h" - -#include - -#include - -namespace arangodb { -namespace transaction { -class Methods; -} - -namespace aql { -class AqlItemBlock; -class DocumentProducingNode; -struct Variable; - -class DocumentProducingBlock { - public: - typedef std::function DocumentProducingFunction; - - DocumentProducingBlock(DocumentProducingNode const* node, transaction::Methods* trx); - virtual ~DocumentProducingBlock() = default; - - public: - inline bool produceResult() const { return _produceResult; } - void buildCallback(); - - private: - transaction::Methods* _trxPtr; - - DocumentProducingNode const* _node; - - /// @brief hether or not we want to build a result - bool const _produceResult; - - /// @brief whether or not we are allowed to pass documents via raw pointers - /// only (true for MMFiles, false for RocksDB) - bool const _useRawDocumentPointers; - - protected: - DocumentProducingFunction _documentProducer; - - /// @brief whether or not we are allowed to use the covering index - /// optimization in a callback - bool _allowCoveringIndexOptimization; -}; - -} // namespace aql -} // namespace arangodb - -#endif diff --git a/arangod/Aql/DocumentProducingNode.h b/arangod/Aql/DocumentProducingNode.h index a87d78a483..73a88f3394 100644 --- a/arangod/Aql/DocumentProducingNode.h +++ b/arangod/Aql/DocumentProducingNode.h @@ -67,10 +67,6 @@ class DocumentProducingNode { return _coveringIndexAttributePositions; } - void resetCoveringIndexAttributePositions() const { - _coveringIndexAttributePositions.clear(); - } - void toVelocyPack(arangodb::velocypack::Builder& builder) const; protected: diff --git a/arangod/Aql/ExecutionBlock.h b/arangod/Aql/ExecutionBlock.h index bc3b543310..07b700cc40 100644 --- a/arangod/Aql/ExecutionBlock.h +++ b/arangod/Aql/ExecutionBlock.h @@ -90,7 +90,7 @@ class ExecutionBlock { virtual std::pair initializeCursor(InputAqlItemRow const& input); /// @brief shutdown, will be called exactly once for the whole query - virtual std::pair shutdown(int); + virtual std::pair shutdown(int errorCode); /// @brief getSome, gets some more items, semantic is as follows: not /// more than atMost items may be delivered. The method tries to diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp index 41e9a317a7..0035dece45 100644 --- a/arangod/Aql/Functions.cpp +++ b/arangod/Aql/Functions.cpp @@ -3406,7 +3406,6 @@ AqlValue Functions::DateSubtract(ExpressionContext* expressionContext, // size == 3 unit / unit type // size == 2 iso duration - year_month_day ymd{floor(tp)}; if (parameters.size() == 3) { AqlValue const& durationUnit = extractFunctionParameterValue(parameters, 1); if (!durationUnit.isNumber()) { // unit must be number diff --git a/arangod/Aql/HashedCollectExecutor.h b/arangod/Aql/HashedCollectExecutor.h index f32be3b416..b1a18cd0be 100644 --- a/arangod/Aql/HashedCollectExecutor.h +++ b/arangod/Aql/HashedCollectExecutor.h @@ -74,14 +74,9 @@ class HashedCollectExecutorInfos : public ExecutorInfos { std::vector getAggregateTypes() const { return _aggregateTypes; } bool getCount() const noexcept { return _count; } transaction::Methods* getTransaction() const { return _trxPtr; } - RegisterId getInputRegister() const noexcept { return _inputRegister; } RegisterId getCollectRegister() const noexcept { return _collectRegister; } private: - // This is exactly the value in the parent member ExecutorInfo::_inRegs, - // respectively getInputRegisters(). - RegisterId _inputRegister; - /// @brief aggregate types std::vector _aggregateTypes; diff --git a/arangod/Aql/IdExecutor.h b/arangod/Aql/IdExecutor.h index 91f317787b..9949ccbe48 100644 --- a/arangod/Aql/IdExecutor.h +++ b/arangod/Aql/IdExecutor.h @@ -85,7 +85,6 @@ class IdExecutor { private: Fetcher& _fetcher; - std::unique_ptr _inputRegisterValues; }; } // namespace aql } // namespace arangodb diff --git a/arangod/Aql/IndexExecutor.cpp b/arangod/Aql/IndexExecutor.cpp index c68c65682f..ae16a7e81b 100644 --- a/arangod/Aql/IndexExecutor.cpp +++ b/arangod/Aql/IndexExecutor.cpp @@ -182,19 +182,19 @@ IndexExecutor::~IndexExecutor() = default; /// @brief order a cursor for the index at the specified position arangodb::OperationCursor* IndexExecutor::orderCursor(size_t currentIndex) { TRI_ASSERT(_infos.getIndexes().size() > currentIndex); - + OperationCursor* cursor = getCursor(currentIndex); if (cursor == nullptr) { // first create an empty cursor object if none is there yet - cursor = resetCursor(currentIndex, std::make_unique()); + cursor = resetCursor(currentIndex, std::make_unique()); } - + TRI_ASSERT(cursor != nullptr); - - IndexIterator* iterator = cursor->indexIterator(); - + + IndexIterator* iterator = cursor->indexIterator(); + AstNode const* conditionNode = nullptr; - if ((iterator == nullptr || !_infos.getNonConstExpressions().empty()) && + if ((iterator == nullptr || !_infos.getNonConstExpressions().empty()) && _infos.getCondition() != nullptr) { TRI_ASSERT(_infos.getIndexes().size() == _infos.getCondition()->numMembers()); TRI_ASSERT(_infos.getCondition()->numMembers() > currentIndex); @@ -207,10 +207,9 @@ arangodb::OperationCursor* IndexExecutor::orderCursor(size_t currentIndex) { resetCursor(currentIndex); } else if (iterator == nullptr || !iterator->canRearm()) { // inject a new index iterator into the existing cursor - cursor->rearm( - _infos.getTrxPtr()->indexScanForCondition( - _infos.getIndexes()[currentIndex], conditionNode, - _infos.getOutVariable(), _infos.getOptions())); + cursor->rearm(_infos.getTrxPtr()->indexScanForCondition(_infos.getIndexes()[currentIndex], conditionNode, + _infos.getOutVariable(), + _infos.getOptions())); } else { // try to rearm an existing iterator if (iterator->rearm(conditionNode, _infos.getOutVariable(), _infos.getOptions())) { @@ -218,10 +217,11 @@ arangodb::OperationCursor* IndexExecutor::orderCursor(size_t currentIndex) { resetCursor(currentIndex); } else { // iterator does not support the condition - cursor->rearm(std::make_unique(iterator->collection(), _infos.getTrxPtr())); + cursor->rearm(std::make_unique(iterator->collection(), + _infos.getTrxPtr())); } } - + return cursor; } @@ -391,8 +391,14 @@ bool IndexExecutor::advanceCursor() { while (!getCursor()->hasMore()) { if (!_infos.isAscending()) { decrCurrentIndex(); + if (_currentIndex == 0) { + setIsLastIndex(true); + } } else { incrCurrentIndex(); + if (_infos.getIndexes().size() - 1 == _currentIndex) { + setIsLastIndex(true); + } } if (getCurrentIndex() < _infos.getIndexes().size()) { diff --git a/arangod/Aql/IndexExecutor.h b/arangod/Aql/IndexExecutor.h index 6107d37823..54d6e9ebb2 100644 --- a/arangod/Aql/IndexExecutor.h +++ b/arangod/Aql/IndexExecutor.h @@ -76,8 +76,6 @@ class IndexExecutorInfos : public ExecutorInfos { std::vector const& getCoveringIndexAttributePositions() { return _coveringIndexAttributePositions; } - std::vector> getInVars() { return _inVars; } - std::vector> getInRegs() { return _inRegs; } bool getProduceResult() { return _produceResult; } bool getUseRawDocumentPointers() { return _useRawDocumentPointers; } std::vector const& getIndexes() { @@ -216,19 +214,20 @@ class IndexExecutor { inline arangodb::OperationCursor* getCursor(size_t pos) { return _cursors[pos].get(); } - std::vector>& getCursors() { - return _cursors; - } void setIndexesExhausted(bool flag) { _indexesExhausted = flag; } bool getIndexesExhausted() { return _indexesExhausted; } - void setLastIndex(bool flag) { _isLastIndex = flag; } bool isLastIndex() { return _isLastIndex; } + void setIsLastIndex(bool flag) { _isLastIndex = flag; } void setCurrentIndex(size_t pos) { _currentIndex = pos; } - void decrCurrentIndex() { _currentIndex--; } - void incrCurrentIndex() { _currentIndex++; } + void decrCurrentIndex() { + _currentIndex--; + } + void incrCurrentIndex() { + _currentIndex++; + } size_t getCurrentIndex() const noexcept { return _currentIndex; } private: diff --git a/arangod/Aql/LimitExecutor.h b/arangod/Aql/LimitExecutor.h index c732ff73c5..8517aa588f 100644 --- a/arangod/Aql/LimitExecutor.h +++ b/arangod/Aql/LimitExecutor.h @@ -56,7 +56,6 @@ class LimitExecutorInfos : public ExecutorInfos { LimitExecutorInfos(LimitExecutorInfos const&) = delete; ~LimitExecutorInfos() = default; - size_t getLimit() const noexcept { return _limit; }; size_t getOffset() const noexcept { return _offset; }; size_t getLimitPlusOffset() const noexcept { return _offset + _limit; }; bool isFullCountEnabled() const noexcept { return _fullCount; }; diff --git a/arangod/Aql/ModificationExecutorTraits.h b/arangod/Aql/ModificationExecutorTraits.h index 5093f3dae5..b2cb5650b1 100644 --- a/arangod/Aql/ModificationExecutorTraits.h +++ b/arangod/Aql/ModificationExecutorTraits.h @@ -54,7 +54,6 @@ struct ModificationBase { std::size_t _defaultBlockSize = ExecutionBlock::DefaultBatchSize(); velocypack::Builder _tmpBuilder; // default - bool _prepared = false; std::size_t _blockIndex = 0; // cursor to the current positon std::shared_ptr _block = nullptr; @@ -70,7 +69,6 @@ struct ModificationBase { // MUST not reset _block _justCopy = false; _last_not_skip = std::numeric_limits::max(); - _prepared = false; _blockIndex = 0; _tmpBuilder.clear(); diff --git a/arangod/Aql/Query.h b/arangod/Aql/Query.h index 8b0db7a7fa..bd206e6754 100644 --- a/arangod/Aql/Query.h +++ b/arangod/Aql/Query.h @@ -287,9 +287,6 @@ class Query { /// warnings. If there are none it will not modify the builder void addWarningsToVelocyPack(arangodb::velocypack::Builder&) const; - /// @brief get a description of the query's current state - std::string getStateString() const; - /// @brief look up a graph in the _graphs collection graph::Graph const* lookupGraphByName(std::string const& name); diff --git a/arangod/Aql/RestAqlHandler.cpp b/arangod/Aql/RestAqlHandler.cpp index ff199d7783..b43e8482ff 100644 --- a/arangod/Aql/RestAqlHandler.cpp +++ b/arangod/Aql/RestAqlHandler.cpp @@ -813,7 +813,7 @@ RestStatus RestAqlHandler::handleUseQuery(std::string const& operation, Query* q ExecutionState state; Result res; std::tie(state, res) = - query->engine()->shutdown(errorCode); // pass errorCode to shutdown + query->engine()->shutdown(errorCode); if (state == ExecutionState::WAITING) { return RestStatus::WAITING; } diff --git a/arangod/Aql/ScatterExecutor.h b/arangod/Aql/ScatterExecutor.h index bb8cf073f8..7ebded8f7d 100644 --- a/arangod/Aql/ScatterExecutor.h +++ b/arangod/Aql/ScatterExecutor.h @@ -83,8 +83,6 @@ class ExecutionBlockImpl : public BlockWithClients { ExecutorInfos const& infos() const { return _infos; } - Query const& getQuery() const { return _query; } - private: ExecutorInfos _infos; diff --git a/arangod/CMakeLists.txt b/arangod/CMakeLists.txt index 37f9bb3430..aa0edd84b0 100644 --- a/arangod/CMakeLists.txt +++ b/arangod/CMakeLists.txt @@ -201,7 +201,6 @@ SET(ARANGOD_SOURCES Aql/CountCollectExecutor.cpp Aql/DistinctCollectExecutor.cpp Aql/DistributeExecutor.cpp - Aql/DocumentProducingBlock.cpp Aql/DocumentProducingNode.cpp Aql/EngineInfoContainerCoordinator.cpp Aql/EngineInfoContainerDBServer.cpp