Hawkin
/
arangodb
mirror of https://gitee.com/bigwinds/arangodb
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Bug fix/fixes asan uninitialized by line (#8588) 

* Fixed maybe-uninitialized warnings by removing unnecessary boost:optionals

* Fixed use after free

* Update arangod/Aql/SingleRemoteModificationExecutor.cpp

* Fixed another autocast number=>bool thanks c !

* Fixed wrong usage of almost identically named variables
This commit is contained in:
Michael Hackstein 2019-03-27 20:54:36 +01:00 committed by GitHub
parent 6e5ce95dad
commit a12a28b094
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 140 additions and 182 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
arangod/Aql/ClusterNodes.cpp
View File

@ -27,8 +27,8 @@
#include "Aql/Ast.h" #include "Aql/Ast.h"
#include "Aql/ClusterBlocks.h" #include "Aql/ClusterBlocks.h"
#include "Aql/Collection.h" #include "Aql/Collection.h"
#include "Aql/ExecutionBlockImpl.h"
#include "Aql/DistributeExecutor.h" #include "Aql/DistributeExecutor.h"
#include "Aql/ExecutionBlockImpl.h"
#include "Aql/ExecutionPlan.h" #include "Aql/ExecutionPlan.h"
#include "Aql/ExecutorInfos.h" #include "Aql/ExecutorInfos.h"
#include "Aql/GraphNode.h" #include "Aql/GraphNode.h"
@ -37,9 +37,9 @@
#include "Aql/ModificationNodes.h" #include "Aql/ModificationNodes.h"
#include "Aql/Query.h" #include "Aql/Query.h"
#include "Aql/RemoteExecutor.h" #include "Aql/RemoteExecutor.h"
#include "Aql/SortingGatherExecutor.h"
#include "Aql/ScatterExecutor.h" #include "Aql/ScatterExecutor.h"
#include "Aql/SingleRemoteModificationExecutor.h" #include "Aql/SingleRemoteModificationExecutor.h"
#include "Aql/SortingGatherExecutor.h"
#include "Transaction/Methods.h" #include "Transaction/Methods.h"
@ -127,7 +127,8 @@ std::unique_ptr<ExecutionBlock> RemoteNode::createBlock(
// TODO This is only for me to find out about the current behaviour. Should // TODO This is only for me to find out about the current behaviour. Should
// probably be either removed, or made into an assert. // probably be either removed, or made into an assert.
if (!regsToClear.empty()) { if (!regsToClear.empty()) {
LOG_TOPIC("4fd06", WARN, Logger::AQL) << "RemoteBlock has registers to clear. " LOG_TOPIC("4fd06", WARN, Logger::AQL)
<< "RemoteBlock has registers to clear. "
<< "Shouldn't this be done before network?"; << "Shouldn't this be done before network?";
} }
#endif #endif
@ -387,7 +388,8 @@ GatherNode::GatherNode(ExecutionPlan* plan, arangodb::velocypack::Slice const& b
auto const sortModeSlice = base.get("sortmode"); auto const sortModeSlice = base.get("sortmode");
if (!toSortMode(VelocyPackHelper::getStringRef(sortModeSlice, ""), _sortmode)) { if (!toSortMode(VelocyPackHelper::getStringRef(sortModeSlice, ""), _sortmode)) {
LOG_TOPIC("2c6f3", ERR, Logger::AQL) << "invalid sort mode detected while " LOG_TOPIC("2c6f3", ERR, Logger::AQL)
<< "invalid sort mode detected while "
"creating 'GatherNode' from vpack"; "creating 'GatherNode' from vpack";
} }
} }
@ -501,23 +503,20 @@ std::unique_ptr<ExecutionBlock> SingleRemoteOperationNode::createBlock(
TRI_ASSERT(previousNode != nullptr); TRI_ASSERT(previousNode != nullptr);
auto in = variableToRegisterOptionalId(_inVariable); RegisterId in = variableToRegisterOptionalId(_inVariable);
auto out = variableToRegisterOptionalId(_outVariable); RegisterId out = variableToRegisterOptionalId(_outVariable);
auto outputNew = variableToRegisterOptionalId(_outVariableNew); RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
auto outputOld = variableToRegisterOptionalId(_outVariableOld); RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
SingleRemoteModificationInfos infos( SingleRemoteModificationInfos infos(
in /*input1*/, boost::none /*input1*/, boost::none /*input1*/, outputNew, in, outputNew, outputOld, out,
outputOld, out /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegsToClear(), getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegsToClear(),
calcRegsToKeep(), _plan->getAst()->query()->trx(), std::move(options), calcRegsToKeep(), _plan->getAst()->query()->trx(), std::move(options),
_collection, ProducesResults(false /*producesResults()*/), _collection, ConsultAqlWriteFilter(_options.consultAqlWriteFilter),
ConsultAqlWriteFilter(_options.consultAqlWriteFilter), IgnoreErrors(_options.ignoreErrors),
IgnoreErrors(_options.ignoreErrors), DoCount(true /*countStats()*/),
IsReplace(false) /*(needed by upsert)*/,
IgnoreDocumentNotFound(_options.ignoreDocumentNotFound), _key, IgnoreDocumentNotFound(_options.ignoreDocumentNotFound), _key,
this->hasParent(), this->_replaceIndexNode); this->hasParent(), this->_replaceIndexNode);

5
arangod/Aql/ExecutionNode.h
View File

@ -65,7 +65,6 @@
#include "VocBase/voc-types.h" #include "VocBase/voc-types.h"
#include "VocBase/vocbase.h" #include "VocBase/vocbase.h"
#include <boost/optional.hpp>
#include <type_traits> #include <type_traits>
namespace arangodb { namespace arangodb {
@ -557,11 +556,11 @@ class ExecutionNode {
RegisterId variableToRegisterId(Variable const*) const; RegisterId variableToRegisterId(Variable const*) const;
boost::optional<RegisterId> variableToRegisterOptionalId(Variable const* var) const { RegisterId variableToRegisterOptionalId(Variable const* var) const {
if (var) { if (var) {
return variableToRegisterId(var); return variableToRegisterId(var);
} }
return boost::none; return ExecutionNode::MaxRegisterId;
} }
protected: protected:

13
arangod/Aql/ModificationExecutor.cpp
View File

@ -20,7 +20,6 @@
/// @author Jan Christoph Uhde /// @author Jan Christoph Uhde
//////////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////////
#include <boost/optional.hpp>
#include "Aql/AqlValue.h" #include "Aql/AqlValue.h"
#include "Aql/Collection.h" #include "Aql/Collection.h"
#include "Aql/OutputAqlItemRow.h" #include "Aql/OutputAqlItemRow.h"
@ -36,10 +35,14 @@ using namespace arangodb::aql;
namespace arangodb { namespace arangodb {
namespace aql { namespace aql {
std::string toString(AllRowsFetcher&) { return "AllRowsFetcher"; } std::string toString(AllRowsFetcher&) { return "AllRowsFetcher"; }
std::string toString(SingleBlockFetcher<true>&) { return "SingleBlockFetcher<true>"; } std::string toString(SingleBlockFetcher<true>&) {
std::string toString(SingleBlockFetcher<false>&) { return "SingleBlockFetcher<false>"; } return "SingleBlockFetcher<true>";
} }
std::string toString(SingleBlockFetcher<false>&) {
return "SingleBlockFetcher<false>";
} }
} // namespace aql
} // namespace arangodb
template <typename FetcherType> template <typename FetcherType>
ModificationExecutorBase<FetcherType>::ModificationExecutorBase(Fetcher& fetcher, Infos& infos) ModificationExecutorBase<FetcherType>::ModificationExecutorBase(Fetcher& fetcher, Infos& infos)
@ -62,8 +65,8 @@ ModificationExecutor<Modifier, FetcherType>::produceRow(OutputAqlItemRow& output
ModificationExecutor::Stats stats; ModificationExecutor::Stats stats;
// TODO - fix / improve prefetching if possible // TODO - fix / improve prefetching if possible
while (!this->_prepared && while (!this->_prepared && (this->_fetcher.upstreamState() !=
(this->_fetcher.upstreamState() != ExecutionState::DONE /*|| this->_fetcher._prefetched */)) { ExecutionState::DONE /*|| this->_fetcher._prefetched */)) {
std::shared_ptr<AqlItemBlockShell> block; std::shared_ptr<AqlItemBlockShell> block;
std::tie(state, block) = this->_fetcher.fetchBlockForModificationExecutor( std::tie(state, block) = this->_fetcher.fetchBlockForModificationExecutor(

41
arangod/Aql/ModificationExecutor.h
View File

@ -35,8 +35,6 @@
#include "velocypack/Slice.h" #include "velocypack/Slice.h"
#include "velocypack/velocypack-aliases.h" #include "velocypack/velocypack-aliases.h"
#include <boost/optional.hpp>
namespace arangodb { namespace arangodb {
namespace transaction { namespace transaction {
class Methods; class Methods;
@ -87,13 +85,11 @@ inline OperationOptions convertOptions(ModificationOptions const& in,
return out; return out;
} }
using wrap = std::reference_wrapper<boost::optional<RegisterId>>; inline std::shared_ptr<std::unordered_set<RegisterId>> makeSet(std::initializer_list<RegisterId> regList) {
inline std::shared_ptr<std::unordered_set<RegisterId>> makeSet(std::initializer_list<wrap> reg_wrap) {
auto rv = make_shared_unordered_set(); auto rv = make_shared_unordered_set();
for (auto wrap : reg_wrap) { for (RegisterId regId : regList) {
auto const& opt = wrap.get(); if (regId < ExecutionNode::MaxRegisterId) {
if (opt.has_value()) { rv->insert(regId);
rv->insert(opt.get());
} }
} }
return rv; return rv;
@ -127,20 +123,17 @@ struct IgnoreDocumentNotFound : BoolWrapper {
struct ModificationExecutorInfos : public ExecutorInfos { struct ModificationExecutorInfos : public ExecutorInfos {
ModificationExecutorInfos( ModificationExecutorInfos(
boost::optional<RegisterId> input1RegisterId, boost::optional<RegisterId> input2RegisterId, RegisterId input1RegisterId, RegisterId input2RegisterId, RegisterId input3RegisterId,
boost::optional<RegisterId> input3RegisterId, boost::optional<RegisterId> outputNewRegisterId, RegisterId outputNewRegisterId, RegisterId outputOldRegisterId,
boost::optional<RegisterId> outputOldRegisterId, RegisterId outputRegisterId, RegisterId nrInputRegisters,
boost::optional<RegisterId> outputRegisterId, RegisterId nrInputRegisters,
RegisterId nrOutputRegisters, std::unordered_set<RegisterId> registersToClear, RegisterId nrOutputRegisters, std::unordered_set<RegisterId> registersToClear,
std::unordered_set<RegisterId> registersToKeep, transaction::Methods* trx, std::unordered_set<RegisterId> registersToKeep, transaction::Methods* trx,
OperationOptions options, aql::Collection const* aqlCollection, OperationOptions options, aql::Collection const* aqlCollection,
ProducesResults producesResults, ConsultAqlWriteFilter consultAqlWriteFilter, ProducesResults producesResults, ConsultAqlWriteFilter consultAqlWriteFilter,
IgnoreErrors ignoreErrors, DoCount doCount, /*bool returnInheritedResults,*/ IgnoreErrors ignoreErrors, DoCount doCount, IsReplace isReplace,
IsReplace isReplace, IgnoreDocumentNotFound ignoreDocumentNotFound) IgnoreDocumentNotFound ignoreDocumentNotFound)
: ExecutorInfos(makeSet({wrap(input1RegisterId), wrap(input2RegisterId), : ExecutorInfos(makeSet({input1RegisterId, input2RegisterId, input3RegisterId}) /*input registers*/,
wrap(input3RegisterId)}) /*input registers*/, makeSet({outputOldRegisterId, outputNewRegisterId, outputRegisterId}) /*output registers*/,
makeSet({wrap(outputOldRegisterId), wrap(outputNewRegisterId),
wrap(outputRegisterId)}) /*output registers*/,
nrInputRegisters, nrOutputRegisters, nrInputRegisters, nrOutputRegisters,
std::move(registersToClear), std::move(registersToKeep)), std::move(registersToClear), std::move(registersToKeep)),
_trx(trx), _trx(trx),
@ -177,15 +170,15 @@ struct ModificationExecutorInfos : public ExecutorInfos {
IgnoreDocumentNotFound _ignoreDocumentNotFound; // needed for update replace IgnoreDocumentNotFound _ignoreDocumentNotFound; // needed for update replace
// insert (singleinput) - upsert (inDoc) - update replace (inDoc) // insert (singleinput) - upsert (inDoc) - update replace (inDoc)
boost::optional<RegisterId> _input1RegisterId; RegisterId _input1RegisterId;
// upsert (insertVar) -- update replace (keyVar) // upsert (insertVar) -- update replace (keyVar)
boost::optional<RegisterId> _input2RegisterId; RegisterId _input2RegisterId;
// upsert (updateVar) // upsert (updateVar)
boost::optional<RegisterId> _input3RegisterId; RegisterId _input3RegisterId;
boost::optional<RegisterId> _outputNewRegisterId; RegisterId _outputNewRegisterId;
boost::optional<RegisterId> _outputOldRegisterId; RegisterId _outputOldRegisterId;
boost::optional<RegisterId> _outputRegisterId; // single remote RegisterId _outputRegisterId; // single remote
}; };
template <typename FetcherType> template <typename FetcherType>

31
arangod/Aql/ModificationExecutorTraits.cpp
View File

@ -188,7 +188,7 @@ bool Insert::doModifications(ModificationExecutorInfos& info, ModificationStats&
reset(); reset();
_tmpBuilder.openArray(); _tmpBuilder.openArray();
const RegisterId& inReg = info._input1RegisterId.value(); const RegisterId& inReg = info._input1RegisterId;
TRI_ASSERT(_block); TRI_ASSERT(_block);
_block->forRowInBlock([this, inReg, &info](InputAqlItemRow&& row) { _block->forRowInBlock([this, inReg, &info](InputAqlItemRow&& row) {
auto const& inVal = row.getValue(inReg); auto const& inVal = row.getValue(inReg);
@ -279,7 +279,7 @@ bool Insert::doOutput(ModificationExecutorInfos& info, OutputAqlItemRow& output)
AqlValue value(elm.get("new")); AqlValue value(elm.get("new"));
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
// store $NEW // store $NEW
output.moveValueInto(info._outputNewRegisterId.value(), input, guard); output.moveValueInto(info._outputNewRegisterId, input, guard);
} }
if (options.returnOld) { if (options.returnOld) {
// store $OLD // store $OLD
@ -287,11 +287,11 @@ bool Insert::doOutput(ModificationExecutorInfos& info, OutputAqlItemRow& output)
if (slice.isNone()) { if (slice.isNone()) {
AqlValue value(VPackSlice::nullSlice()); AqlValue value(VPackSlice::nullSlice());
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(info._outputOldRegisterId.value(), input, guard); output.moveValueInto(info._outputOldRegisterId, input, guard);
} else { } else {
AqlValue value(slice); AqlValue value(slice);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(info._outputOldRegisterId.value(), input, guard); output.moveValueInto(info._outputOldRegisterId, input, guard);
} }
} }
} // !wasError - end } // !wasError - end
@ -319,7 +319,7 @@ bool Remove::doModifications(ModificationExecutorInfos& info, ModificationStats&
std::string key; std::string key;
std::string rev; std::string rev;
const RegisterId& inReg = info._input1RegisterId.value(); const RegisterId& inReg = info._input1RegisterId;
TRI_ASSERT(_block); TRI_ASSERT(_block);
_block->forRowInBlock([this, &stats, &errorCode, &key, &rev, trx, inReg, _block->forRowInBlock([this, &stats, &errorCode, &key, &rev, trx, inReg,
&info](InputAqlItemRow&& row) { &info](InputAqlItemRow&& row) {
@ -433,7 +433,7 @@ bool Remove::doOutput(ModificationExecutorInfos& info, OutputAqlItemRow& output)
AqlValue value(slice); AqlValue value(slice);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(info._outputOldRegisterId.value(), input, guard); output.moveValueInto(info._outputOldRegisterId, input, guard);
} }
} }
++_operationResultIterator; ++_operationResultIterator;
@ -461,9 +461,9 @@ bool Upsert::doModifications(ModificationExecutorInfos& info, ModificationStats&
std::string errorMessage; std::string errorMessage;
std::string key; std::string key;
auto* trx = info._trx; auto* trx = info._trx;
const RegisterId& inDocReg = info._input1RegisterId.value(); const RegisterId& inDocReg = info._input1RegisterId;
const RegisterId& insertReg = info._input2RegisterId.value(); const RegisterId& insertReg = info._input2RegisterId;
const RegisterId& updateReg = info._input3RegisterId.value(); const RegisterId& updateReg = info._input3RegisterId;
_block->forRowInBlock([this, &stats, &errorCode, &errorMessage, &key, trx, inDocReg, _block->forRowInBlock([this, &stats, &errorCode, &errorMessage, &key, trx, inDocReg,
insertReg, updateReg, &info](InputAqlItemRow&& row) { insertReg, updateReg, &info](InputAqlItemRow&& row) {
@ -623,7 +623,7 @@ bool Upsert::doOutput(ModificationExecutorInfos& info, OutputAqlItemRow& output)
AqlValue value(elm.get("new")); AqlValue value(elm.get("new"));
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
// store $NEW // store $NEW
output.moveValueInto(info._outputNewRegisterId.value(), input, guard); output.moveValueInto(info._outputNewRegisterId, input, guard);
} }
} }
@ -658,10 +658,9 @@ bool UpdateReplace<ModType>::doModifications(ModificationExecutorInfos& info,
std::string key; std::string key;
std::string rev; std::string rev;
auto* trx = info._trx; auto* trx = info._trx;
const RegisterId& inDocReg = info._input1RegisterId.value(); const RegisterId& inDocReg = info._input1RegisterId;
const bool hasKeyVariable = info._input2RegisterId.has_value(); const RegisterId& keyReg = info._input2RegisterId;
const RegisterId& keyReg = hasKeyVariable ? info._input2RegisterId.value() const bool hasKeyVariable = keyReg != ExecutionNode::MaxRegisterId;
: 42 /*invalid but unused*/;
_block->forRowInBlock([this, &options, &stats, &errorCode, &errorMessage, _block->forRowInBlock([this, &options, &stats, &errorCode, &errorMessage,
&key, &rev, trx, inDocReg, keyReg, hasKeyVariable, &key, &rev, trx, inDocReg, keyReg, hasKeyVariable,
@ -797,14 +796,14 @@ bool UpdateReplace<ModType>::doOutput(ModificationExecutorInfos& info,
AqlValue value(elm.get("new")); AqlValue value(elm.get("new"));
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
// store $NEW // store $NEW
output.moveValueInto(info._outputNewRegisterId.value(), input, guard); output.moveValueInto(info._outputNewRegisterId, input, guard);
} }
if (options.returnOld) { if (options.returnOld) {
auto slice = elm.get("old"); auto slice = elm.get("old");
AqlValue value(slice); AqlValue value(slice);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
// store $OLD // store $OLD
output.moveValueInto(info._outputOldRegisterId.value(), input, guard); output.moveValueInto(info._outputOldRegisterId, input, guard);
} }
} }
++_operationResultIterator; ++_operationResultIterator;

79
arangod/Aql/ModificationNodes.cpp
View File

@ -115,21 +115,14 @@ std::unique_ptr<ExecutionBlock> RemoveNode::createBlock(
TRI_ASSERT(previousNode != nullptr); TRI_ASSERT(previousNode != nullptr);
RegisterId inDocRegister = variableToRegisterId(_inVariable); RegisterId inDocRegister = variableToRegisterId(_inVariable);
RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
boost::optional<RegisterId> outputNew; RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
if (_outVariableNew) {
outputNew = variableToRegisterId(_outVariableNew);
}
boost::optional<RegisterId> outputOld;
if (_outVariableOld) {
outputOld = variableToRegisterId(_outVariableOld);
}
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
ModificationExecutorInfos infos( ModificationExecutorInfos infos(
inDocRegister, boost::none, boost::none, outputNew, outputOld, boost::none /*output*/, inDocRegister, ExecutionNode::MaxRegisterId, ExecutionNode::MaxRegisterId,
outputNew, outputOld, ExecutionNode::MaxRegisterId /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/,
getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(), getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(),
@ -194,20 +187,14 @@ std::unique_ptr<ExecutionBlock> InsertNode::createBlock(
RegisterId inputRegister = variableToRegisterId(_inVariable); RegisterId inputRegister = variableToRegisterId(_inVariable);
boost::optional<RegisterId> outputNew; RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
if (_outVariableNew) { RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
outputNew = variableToRegisterId(_outVariableNew);
}
boost::optional<RegisterId> outputOld;
if (_outVariableOld) {
outputOld = variableToRegisterId(_outVariableOld);
}
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
ModificationExecutorInfos infos( ModificationExecutorInfos infos(
inputRegister, boost::none, boost::none, outputNew, outputOld, boost::none /*output*/, inputRegister, ExecutionNode::MaxRegisterId, ExecutionNode::MaxRegisterId,
outputNew, outputOld, ExecutionNode::MaxRegisterId /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/,
getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(), getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(),
@ -290,25 +277,15 @@ std::unique_ptr<ExecutionBlock> UpdateNode::createBlock(
RegisterId inDocRegister = variableToRegisterId(_inDocVariable); RegisterId inDocRegister = variableToRegisterId(_inDocVariable);
boost::optional<RegisterId> inKeyRegister; RegisterId inKeyRegister = variableToRegisterOptionalId(_inKeyVariable);
if (_inKeyVariable) { RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
inKeyRegister = variableToRegisterId(_inKeyVariable); RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
}
boost::optional<RegisterId> outputNew;
if (_outVariableNew) {
outputNew = variableToRegisterId(_outVariableNew);
}
boost::optional<RegisterId> outputOld;
if (_outVariableOld) {
outputOld = variableToRegisterId(_outVariableOld);
}
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
ModificationExecutorInfos infos( ModificationExecutorInfos infos(
inDocRegister, inKeyRegister, boost::none, outputNew, outputOld, boost::none /*output*/, inDocRegister, inKeyRegister, ExecutionNode::MaxRegisterId, outputNew,
outputOld, ExecutionNode::MaxRegisterId /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/,
getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(), getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(),
@ -372,25 +349,17 @@ std::unique_ptr<ExecutionBlock> ReplaceNode::createBlock(
RegisterId inDocRegister = variableToRegisterId(_inDocVariable); RegisterId inDocRegister = variableToRegisterId(_inDocVariable);
boost::optional<RegisterId> inKeyRegister; RegisterId inKeyRegister = variableToRegisterOptionalId(_inKeyVariable);
if (_inKeyVariable) {
inKeyRegister = variableToRegisterId(_inKeyVariable);
}
boost::optional<RegisterId> outputNew; RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
if (_outVariableNew) {
outputNew = variableToRegisterId(_outVariableNew);
}
boost::optional<RegisterId> outputOld; RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
if (_outVariableOld) {
outputOld = variableToRegisterId(_outVariableOld);
}
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
ModificationExecutorInfos infos( ModificationExecutorInfos infos(
inDocRegister, inKeyRegister, boost::none, outputNew, outputOld, boost::none /*output*/, inDocRegister, inKeyRegister, ExecutionNode::MaxRegisterId, outputNew,
outputOld, ExecutionNode::MaxRegisterId /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/,
getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(), getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(),
@ -475,20 +444,14 @@ std::unique_ptr<ExecutionBlock> UpsertNode::createBlock(
RegisterId insert = variableToRegisterId(_insertVariable); RegisterId insert = variableToRegisterId(_insertVariable);
RegisterId update = variableToRegisterId(_updateVariable); RegisterId update = variableToRegisterId(_updateVariable);
boost::optional<RegisterId> outputNew; RegisterId outputNew = variableToRegisterOptionalId(_outVariableNew);
if (_outVariableNew) {
outputNew = variableToRegisterId(_outVariableNew);
}
boost::optional<RegisterId> outputOld; RegisterId outputOld = variableToRegisterOptionalId(_outVariableOld);
if (_outVariableOld) {
outputOld = variableToRegisterId(_outVariableOld);
}
OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld); OperationOptions options = convertOptions(_options, _outVariableNew, _outVariableOld);
ModificationExecutorInfos infos( ModificationExecutorInfos infos(
inDoc, insert, update, outputNew, outputOld, boost::none /*output*/, inDoc, insert, update, outputNew, outputOld, ExecutionNode::MaxRegisterId /*output*/,
getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/, getRegisterPlan()->nrRegs[previousNode->getDepth()] /*nr input regs*/,
getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/, getRegisterPlan()->nrRegs[getDepth()] /*nr output regs*/,
getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(), getRegsToClear(), calcRegsToKeep(), _plan->getAst()->query()->trx(),

57
arangod/Aql/SingleRemoteModificationExecutor.cpp
View File

@ -27,12 +27,12 @@
#include "Aql/OutputAqlItemRow.h" #include "Aql/OutputAqlItemRow.h"
#include "Aql/SingleRowFetcher.h" #include "Aql/SingleRowFetcher.h"
#include "Basics/Common.h" #include "Basics/Common.h"
#include "ModificationExecutorTraits.h" #include "Basics/StaticStrings.h"
#include "VocBase/LogicalCollection.h"
#include "Cluster/ClusterComm.h" #include "Cluster/ClusterComm.h"
#include "Cluster/ClusterInfo.h" #include "Cluster/ClusterInfo.h"
#include "Cluster/ServerState.h" #include "Cluster/ServerState.h"
#include "Basics/StaticStrings.h" #include "ModificationExecutorTraits.h"
#include "VocBase/LogicalCollection.h"
#include <algorithm> #include <algorithm>
@ -63,7 +63,8 @@ std::unique_ptr<VPackBuilder> merge(VPackSlice document, std::string const& key,
} // namespace } // namespace
template <typename Modifier> template <typename Modifier>
SingleRemoteModificationExecutor<Modifier>::SingleRemoteModificationExecutor(Fetcher& fetcher, Infos& info) SingleRemoteModificationExecutor<Modifier>::SingleRemoteModificationExecutor(Fetcher& fetcher,
Infos& info)
: _info(info), _fetcher(fetcher), _upstreamState(ExecutionState::HASMORE) { : _info(info), _fetcher(fetcher), _upstreamState(ExecutionState::HASMORE) {
TRI_ASSERT(arangodb::ServerState::instance()->isCoordinator()); TRI_ASSERT(arangodb::ServerState::instance()->isCoordinator());
}; };
@ -92,12 +93,11 @@ SingleRemoteModificationExecutor<Modifier>::produceRow(OutputAqlItemRow& output)
} }
template <typename Modifier> template <typename Modifier>
bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOperation(InputAqlItemRow& input, bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOperation(
OutputAqlItemRow& output, InputAqlItemRow& input, OutputAqlItemRow& output, Stats& stats) {
Stats& stats) {
_info._options.silent = false; _info._options.silent = false;
_info._options.returnOld = _info._options.returnOld || _info._outputRegisterId; _info._options.returnOld = _info._options.returnOld ||
_info._outputRegisterId != ExecutionNode::MaxRegisterId;
const bool isIndex = std::is_same<Modifier, IndexTag>::value; const bool isIndex = std::is_same<Modifier, IndexTag>::value;
const bool isInsert = std::is_same<Modifier, Insert>::value; const bool isInsert = std::is_same<Modifier, Insert>::value;
@ -109,24 +109,19 @@ bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOpera
OperationOptions& options = _info._options; OperationOptions& options = _info._options;
if (_info._key.empty() && !_info._input1RegisterId.has_value()) { if (_info._key.empty() && _info._input1RegisterId == ExecutionNode::MaxRegisterId) {
THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND, THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND,
"missing document reference"); "missing document reference");
} }
VPackBuilder inBuilder; VPackBuilder inBuilder;
VPackSlice inSlice = VPackSlice::emptyObjectSlice(); VPackSlice inSlice = VPackSlice::emptyObjectSlice();
if (_info._input1RegisterId.has_value()) { // IF NOT REMOVE OR SELECT if (_info._input1RegisterId != ExecutionNode::MaxRegisterId) { // IF NOT REMOVE OR SELECT
// if (_buffer.size() < 1) { AqlValue const& inDocument = input.getValue(_info._input1RegisterId);
// THROW_ARANGO_EXCEPTION_MESSAGE(TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND,
// "missing document reference in Register");
//}
AqlValue const& inDocument = input.getValue(_info._input1RegisterId.value());
inBuilder.add(inDocument.slice()); inBuilder.add(inDocument.slice());
inSlice = inBuilder.slice(); inSlice = inBuilder.slice();
} }
std::unique_ptr<VPackBuilder> mergedBuilder = nullptr; std::unique_ptr<VPackBuilder> mergedBuilder = nullptr;
if (!_info._key.empty()) { if (!_info._key.empty()) {
mergedBuilder = merge(inSlice, _info._key, 0); mergedBuilder = merge(inSlice, _info._key, 0);
@ -148,7 +143,7 @@ bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOpera
result = _info._trx->remove(_info._aqlCollection->name(), inSlice, _info._options); result = _info._trx->remove(_info._aqlCollection->name(), inSlice, _info._options);
possibleWrites = 1; possibleWrites = 1;
} else if (isReplace) { } else if (isReplace) {
if (_info._replaceIndex && !_info._input1RegisterId.has_value()) { if (_info._replaceIndex && _info._input1RegisterId == ExecutionNode::MaxRegisterId) {
// we have a FOR .. IN FILTER doc._key == ... REPLACE - no WITH. // we have a FOR .. IN FILTER doc._key == ... REPLACE - no WITH.
// in this case replace needs to behave as if it was UPDATE. // in this case replace needs to behave as if it was UPDATE.
result = _info._trx->update(_info._aqlCollection->name(), inSlice, _info._options); result = _info._trx->update(_info._aqlCollection->name(), inSlice, _info._options);
@ -182,7 +177,9 @@ bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOpera
stats.addWritesExecuted(possibleWrites); stats.addWritesExecuted(possibleWrites);
stats.incrScannedIndex(); stats.incrScannedIndex();
if (!(_info._outputRegisterId.has_value() || _info._outputOldRegisterId.has_value() || _info._outputNewRegisterId.has_value())) { if (!(_info._outputRegisterId != ExecutionNode::MaxRegisterId ||
_info._outputOldRegisterId != ExecutionNode::MaxRegisterId ||
_info._outputNewRegisterId != ExecutionNode::MaxRegisterId)) {
if (_info._hasParent) { if (_info._hasParent) {
output.copyRow(input); output.copyRow(input);
} }
@ -203,41 +200,43 @@ bool SingleRemoteModificationExecutor<Modifier>::doSingleRemoteModificationOpera
VPackSlice oldDocument = VPackSlice::nullSlice(); VPackSlice oldDocument = VPackSlice::nullSlice();
VPackSlice newDocument = VPackSlice::nullSlice(); VPackSlice newDocument = VPackSlice::nullSlice();
if (outDocument.isObject()) { if (outDocument.isObject()) {
if (_info._outputNewRegisterId.has_value() && outDocument.hasKey("new")) { if (_info._outputNewRegisterId != ExecutionNode::MaxRegisterId &&
outDocument.hasKey("new")) {
newDocument = outDocument.get("new"); newDocument = outDocument.get("new");
} }
if (outDocument.hasKey("old")) { if (outDocument.hasKey("old")) {
outDocument = outDocument.get("old"); outDocument = outDocument.get("old");
if (_info._outputOldRegisterId.has_value()) { if (_info._outputOldRegisterId != ExecutionNode::MaxRegisterId) {
oldDocument = outDocument; oldDocument = outDocument;
} }
} }
} }
TRI_ASSERT(_info._outputRegisterId || _info._outputOldRegisterId.has_value() || _info._outputNewRegisterId.has_value()); TRI_ASSERT(_info._outputRegisterId != ExecutionNode::MaxRegisterId ||
_info._outputOldRegisterId != ExecutionNode::MaxRegisterId ||
_info._outputNewRegisterId != ExecutionNode::MaxRegisterId);
// place documents as in the out variable slots of the result // place documents as in the out variable slots of the result
if (_info._outputRegisterId.has_value()) { if (_info._outputRegisterId != ExecutionNode::MaxRegisterId) {
AqlValue value(outDocument); AqlValue value(outDocument);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(_info._outputRegisterId.value(), input, guard); output.moveValueInto(_info._outputRegisterId, input, guard);
} }
if (_info._outputOldRegisterId.has_value()) { if (_info._outputOldRegisterId != ExecutionNode::MaxRegisterId) {
TRI_ASSERT(options.returnOld); TRI_ASSERT(options.returnOld);
AqlValue value(oldDocument); AqlValue value(oldDocument);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(_info._outputOldRegisterId.value(), input, guard); output.moveValueInto(_info._outputOldRegisterId, input, guard);
} }
if (_info._outputNewRegisterId.has_value()) { if (_info._outputNewRegisterId != ExecutionNode::MaxRegisterId) {
TRI_ASSERT(options.returnNew); TRI_ASSERT(options.returnNew);
AqlValue value(newDocument); AqlValue value(newDocument);
AqlValueGuard guard(value, true); AqlValueGuard guard(value, true);
output.moveValueInto(_info._outputNewRegisterId.value(), input, guard); output.moveValueInto(_info._outputNewRegisterId, input, guard);
} }
TRI_IF_FAILURE("SingleRemoteModificationOperationBlock::moreDocuments") { TRI_IF_FAILURE("SingleRemoteModificationOperationBlock::moreDocuments") {
THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG); THROW_ARANGO_EXCEPTION(TRI_ERROR_DEBUG);
} }

35
arangod/Aql/SingleRemoteModificationExecutor.h
View File

@ -30,31 +30,34 @@ namespace aql {
struct SingleRemoteModificationInfos : ModificationExecutorInfos { struct SingleRemoteModificationInfos : ModificationExecutorInfos {
SingleRemoteModificationInfos( SingleRemoteModificationInfos(
boost::optional<RegisterId> input1RegisterId, boost::optional<RegisterId> input2RegisterId, RegisterId inputRegister, RegisterId outputNewRegisterId,
boost::optional<RegisterId> input3RegisterId, boost::optional<RegisterId> outputNewRegisterId, RegisterId outputOldRegisterId, RegisterId outputRegisterId,
boost::optional<RegisterId> outputOldRegisterId, RegisterId nrInputRegisters, RegisterId nrOutputRegisters,
boost::optional<RegisterId> outputRegisterId, RegisterId nrInputRegisters, std::unordered_set<RegisterId> const& registersToClear,
RegisterId nrOutputRegisters, std::unordered_set<RegisterId> registersToClear, std::unordered_set<RegisterId>&& registersToKeep, transaction::Methods* trx,
std::unordered_set<RegisterId> registersToKeep, transaction::Methods* trx,
OperationOptions options, aql::Collection const* aqlCollection, OperationOptions options, aql::Collection const* aqlCollection,
ProducesResults producesResults, ConsultAqlWriteFilter consultAqlWriteFilter, ConsultAqlWriteFilter consultAqlWriteFilter, IgnoreErrors ignoreErrors,
IgnoreErrors ignoreErrors, DoCount doCount, IsReplace isReplace,
IgnoreDocumentNotFound ignoreDocumentNotFound, // end of base class params IgnoreDocumentNotFound ignoreDocumentNotFound, // end of base class params
std::string key, bool hasParent, bool replaceIndex) std::string key, bool hasParent, bool replaceIndex)
: ModificationExecutorInfos( : ModificationExecutorInfos(inputRegister, ExecutionNode::MaxRegisterId,
std::move(input1RegisterId), std::move(input2RegisterId), ExecutionNode::MaxRegisterId, outputNewRegisterId,
std::move(input3RegisterId), std::move(outputNewRegisterId), outputOldRegisterId, outputRegisterId,
std::move(outputOldRegisterId), std::move(outputRegisterId), nrInputRegisters, nrOutputRegisters,
nrInputRegisters, std::move(nrOutputRegisters), registersToClear, std::move(registersToKeep),
std::move(registersToClear), std::move(registersToKeep), trx, trx, std::move(options), aqlCollection,
std::move(options), aqlCollection, producesResults, consultAqlWriteFilter, ProducesResults(false), consultAqlWriteFilter,
ignoreErrors, doCount, isReplace, ignoreDocumentNotFound), ignoreErrors, DoCount(true), IsReplace(false),
ignoreDocumentNotFound),
_key(std::move(key)), _key(std::move(key)),
_hasParent(hasParent), _hasParent(hasParent),
_replaceIndex(replaceIndex) {} _replaceIndex(replaceIndex) {}
std::string _key; std::string _key;
bool _hasParent; // node->hasParent(); bool _hasParent; // node->hasParent();
bool _replaceIndex; bool _replaceIndex;
constexpr static double const defaultTimeOut = 3600.0; constexpr static double const defaultTimeOut = 3600.0;
}; };

12
tests/Aql/MultiDependencySingleRowFetcherTest.cpp
View File

@ -49,12 +49,11 @@ namespace aql {
SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") { SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
ResourceMonitor monitor; ResourceMonitor monitor;
ExecutionState state; ExecutionState state;
InputAqlItemRow row{CreateInvalidInputRowHint{}};
GIVEN("there are no blocks upstream, single dependency") { GIVEN("there are no blocks upstream, single dependency") {
VPackBuilder input; VPackBuilder input;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 0, 1}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 0, 1};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
WHEN("the producer does not wait") { WHEN("the producer does not wait") {
blockFetcherMock.getDependencyMock(0).shouldReturn(ExecutionState::DONE, nullptr); blockFetcherMock.getDependencyMock(0).shouldReturn(ExecutionState::DONE, nullptr);
@ -102,8 +101,8 @@ SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
GIVEN("A single upstream block with a single row, single dependency") { GIVEN("A single upstream block with a single row, single dependency") {
VPackBuilder input; VPackBuilder input;
ResourceMonitor monitor;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, 1}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, 1};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
std::unique_ptr<AqlItemBlock> block = buildBlock<1>(&monitor, {{42}}); std::unique_ptr<AqlItemBlock> block = buildBlock<1>(&monitor, {{42}});
@ -224,8 +223,8 @@ SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
// specification should be compared with the actual output. // specification should be compared with the actual output.
GIVEN("there are multiple blocks upstream, single dependency") { GIVEN("there are multiple blocks upstream, single dependency") {
ResourceMonitor monitor;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, 1}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, 1};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
// three 1-column matrices with 3, 2 and 1 rows, respectively // three 1-column matrices with 3, 2 and 1 rows, respectively
std::unique_ptr<AqlItemBlock> block1 = std::unique_ptr<AqlItemBlock> block1 =
@ -358,6 +357,7 @@ SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
VPackBuilder input; VPackBuilder input;
size_t numDeps = 3; size_t numDeps = 3;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 0, numDeps}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 0, numDeps};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
WHEN("the producers do not wait") { WHEN("the producers do not wait") {
for (size_t i = 0; i < numDeps; ++i) { for (size_t i = 0; i < numDeps; ++i) {
@ -416,9 +416,9 @@ SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
GIVEN("A single upstream block with a single row, multi dependency") { GIVEN("A single upstream block with a single row, multi dependency") {
VPackBuilder input; VPackBuilder input;
ResourceMonitor monitor;
size_t numDeps = 3; size_t numDeps = 3;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, numDeps}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, numDeps};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
std::unique_ptr<AqlItemBlock> blockDep1 = buildBlock<1>(&monitor, {{42}}); std::unique_ptr<AqlItemBlock> blockDep1 = buildBlock<1>(&monitor, {{42}});
std::unique_ptr<AqlItemBlock> blockDep2 = buildBlock<1>(&monitor, {{23}}); std::unique_ptr<AqlItemBlock> blockDep2 = buildBlock<1>(&monitor, {{23}});
@ -605,9 +605,9 @@ SCENARIO("MultiDependencySingleRowFetcher", "[AQL][EXECUTOR][FETCHER]") {
// specification should be compared with the actual output. // specification should be compared with the actual output.
GIVEN("there are multiple blocks upstream, multiple dependencies") { GIVEN("there are multiple blocks upstream, multiple dependencies") {
ResourceMonitor monitor;
size_t numDeps = 3; size_t numDeps = 3;
MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, numDeps}; MultiBlockFetcherMock<false> blockFetcherMock{monitor, 1, numDeps};
InputAqlItemRow row{CreateInvalidInputRowHint{}};
// three 1-column matrices with 3, 2 and 1 rows, respectively // three 1-column matrices with 3, 2 and 1 rows, respectively
std::unique_ptr<AqlItemBlock> block1Dep1 = std::unique_ptr<AqlItemBlock> block1Dep1 =