From f3ff880c85083b59a8dcec84581e58b358f9dc4c Mon Sep 17 00:00:00 2001 From: Kaveh Vahedipour Date: Wed, 27 Apr 2016 13:10:48 +0000 Subject: [PATCH 1/2] Node was not making correct == for rhs json objects --- arangod/Agency/Node.cpp | 23 +++++++++++++++++++++-- arangod/Agency/Node.h | 8 ++++++++ js/client/tests/agency/agency-test.js | 10 +++++++++- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/arangod/Agency/Node.cpp b/arangod/Agency/Node.cpp index 8b795e5859..f75cf61084 100644 --- a/arangod/Agency/Node.cpp +++ b/arangod/Agency/Node.cpp @@ -133,10 +133,14 @@ Node& Node::operator= (Node const& rhs) { _children = rhs._children; return *this; } - +#include // Comparison with slice bool Node::operator== (VPackSlice const& rhs) const { - return rhs.equals(slice()); + if (rhs.isObject()) { + return rhs.toJson() == toJson(); + } else { + return rhs.equals(slice()); + } } // Remove this node from store @@ -574,3 +578,18 @@ std::ostream& Node::print (std::ostream& o) const { return o; } +Node::Children& Node::children () { + return _children; +} + +Node::Children const& Node::children () const { + return _children; +} + +std::string Node::toJson() const { + Builder builder; + builder.openArray(); + toBuilder(builder); + builder.close(); + return builder.slice()[0].toJson(); +} diff --git a/arangod/Agency/Node.h b/arangod/Agency/Node.h index 49a5a5189f..ded28c8e1a 100644 --- a/arangod/Agency/Node.h +++ b/arangod/Agency/Node.h @@ -147,6 +147,12 @@ public: /// @brief Create Builder representing this store void toBuilder (Builder&) const; + /// @brief Access children + Children& children (); + + /// @brief Access children + Children const& children () const; + /// @brief Create slice from value Slice slice() const; @@ -165,6 +171,8 @@ public: Store& store(); Store const& store() const; + std::string toJson() const; + protected: /// @brief Add time to live entry diff --git a/js/client/tests/agency/agency-test.js b/js/client/tests/agency/agency-test.js index fd9f27640b..673ad52997 100644 --- a/js/client/tests/agency/agency-test.js +++ b/js/client/tests/agency/agency-test.js @@ -156,7 +156,15 @@ function agencyTestSuite () { // fail precond isArray res = writeAgency([[{"a":14},{"a":{"isArray":true}}]]); assertEqual(res.statusCode, 412); - assertEqual(res.bodyParsed, {"results":[0]}); + assertEqual(res.bodyParsed, {"results":[0]}); + // check object precondition + res = writeAgency([[{"/a/b/c":{"op":"set","new":12}}]]); + res = writeAgency([[{"/a/b/c":{"op":"set","new":13}},{"a":{"b":{"c":12}}}]]); + assertEqual(res.statusCode, 200); + res = writeAgency([[{"/a/b/c":{"op":"set","new":14}},{"/a":{"old":{"b":{"c":12}}}}]]); + assertEqual(res.statusCode, 412); + res = writeAgency([[{"/a/b/c":{"op":"set","new":14}},{"/a":{"old":{"b":{"c":13}}}}]]); + assertEqual(res.statusCode, 200); }, From 647c7b6584254c711af5c948d2d5b865e039c4e7 Mon Sep 17 00:00:00 2001 From: Michael Hackstein Date: Wed, 27 Apr 2016 15:23:40 +0200 Subject: [PATCH 2/2] Finished implementing baby api in Modification blocks. --- arangod/Aql/ModificationBlocks.cpp | 430 +++++++++++++++++++++-------- 1 file changed, 317 insertions(+), 113 deletions(-) diff --git a/arangod/Aql/ModificationBlocks.cpp b/arangod/Aql/ModificationBlocks.cpp index 6c2ecb77d1..3dc3e7ffcd 100644 --- a/arangod/Aql/ModificationBlocks.cpp +++ b/arangod/Aql/ModificationBlocks.cpp @@ -395,6 +395,7 @@ AqlItemBlock* InsertBlock::work(std::vector& blocks) { options.waitForSync = ep->_options.waitForSync; options.returnNew = producesOutput; + VPackBuilder babyBuilder; // loop over all blocks size_t dstRow = 0; for (auto it = blocks.begin(); it != blocks.end(); ++it) { @@ -402,34 +403,70 @@ AqlItemBlock* InsertBlock::work(std::vector& blocks) { size_t const n = res->size(); throwIfKilled(); // check if we were aborted + bool isMultiple = (n > 1); + if (!isMultiple) { + // loop over the complete block. Well it is one element only + for (size_t i = 0; i < n; ++i) { + AqlValue a = res->getValue(i, registerId); - // loop over the complete block - for (size_t i = 0; i < n; ++i) { - AqlValue a = res->getValue(i, registerId); + // only copy 1st row of registers inherited from previous frame(s) + inheritRegisters(res, result.get(), i, dstRow); - // only copy 1st row of registers inherited from previous frame(s) - inheritRegisters(res, result.get(), i, dstRow); + int errorCode = TRI_ERROR_NO_ERROR; - int errorCode = TRI_ERROR_NO_ERROR; + if (!a.isObject()) { + // value is no object + errorCode = TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID; + } else { + OperationResult opRes = _trx->insert(_collection->name, a.slice(), options); + errorCode = opRes.code; - if (!a.isObject()) { - // value is no object - errorCode = TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID; - } else { - OperationResult opRes = _trx->insert(_collection->name, a.slice(), options); - errorCode = opRes.code; + if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { + // return $NEW + result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); + } + } - if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { - // return $NEW - result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); + handleResult(errorCode, ep->_options.ignoreErrors); + ++dstRow; + } + // done with a block + } else { + babyBuilder.clear(); + babyBuilder.openArray(); + for (size_t i = 0; i < n; ++i) { + AqlValue a = res->getValue(i, registerId); + + // only copy 1st row of registers inherited from previous frame(s) + inheritRegisters(res, result.get(), i, dstRow); + // TODO This may be optimized with externals + babyBuilder.add(a.slice()); + ++dstRow; + } + babyBuilder.close(); + VPackSlice toSend = babyBuilder.slice(); + OperationResult opRes = + _trx->insert(_collection->name, toSend, options); + + if (producesOutput) { + // Reset dstRow + dstRow -= n; + VPackSlice resultList = opRes.slice(); + TRI_ASSERT(resultList.isArray()); + for (auto const& elm: VPackArrayIterator(resultList)) { + bool wasError = arangodb::basics::VelocyPackHelper::getBooleanValue( + elm, "error", false); + if (!wasError) { + // return $NEW + result->setValue(dstRow, _outRegNew, AqlValue(elm.get("new"))); + } + ++dstRow; } } - handleResult(errorCode, ep->_options.ignoreErrors); - ++dstRow; + handleBabyResult(opRes.countErrorCodes, toSend.length(), + ep->_options.ignoreErrors); } - // done with a block - // now free it already (*it) = nullptr; delete res; @@ -492,6 +529,11 @@ AqlItemBlock* UpdateBlock::work(std::vector& blocks) { throwIfKilled(); // check if we were aborted size_t const n = res->size(); + bool isMultiple = (n > 1); + if (isMultiple) { + object.clear(); + object.openArray(); + } // loop over the complete block for (size_t i = 0; i < n; ++i) { @@ -517,46 +559,82 @@ AqlItemBlock* UpdateBlock::work(std::vector& blocks) { } if (errorCode == TRI_ERROR_NO_ERROR) { - keyBuilder.clear(); - keyBuilder.openObject(); - keyBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); - keyBuilder.close(); - - VPackSlice toUpdate; - if (hasKeyVariable) { - object = VPackCollection::merge(a.slice(), keyBuilder.slice(), false, false); - toUpdate = object.slice(); + keyBuilder.clear(); + keyBuilder.openObject(); + keyBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); + keyBuilder.close(); + + VPackBuilder tmp = VPackCollection::merge( + a.slice(), keyBuilder.slice(), false, false); + if (isMultiple) { + object.add(tmp.slice()); + } else { + object = tmp; + } } else { // use original slice for updating - toUpdate = a.slice(); + object.add(a.slice()); } + } else { + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + } + } - // fetch old revision - OperationResult opRes = _trx->update(_collection->name, toUpdate, options); - errorCode = opRes.code; + if (isMultiple) { + object.close(); + } - if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { - if (ep->_outVariableOld != nullptr) { - // store $OLD - result->setValue(dstRow, _outRegOld, AqlValue(opRes.slice().get("old"))); - } - if (ep->_outVariableNew != nullptr) { - // store $NEW - result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); - } + VPackSlice toUpdate = object.slice(); + + // fetch old revision + OperationResult opRes = _trx->update(_collection->name, toUpdate, options); + if (!isMultiple) { + int errorCode = opRes.code; + + if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { + if (ep->_outVariableOld != nullptr) { + // store $OLD + result->setValue(dstRow, _outRegOld, AqlValue(opRes.slice().get("old"))); } - - if (errorCode == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND && _isDBServer && - ignoreDocumentNotFound) { - // Ignore document not found on the DBserver: - errorCode = TRI_ERROR_NO_ERROR; + if (ep->_outVariableNew != nullptr) { + // store $NEW + result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); } } + if (errorCode == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND && _isDBServer && + ignoreDocumentNotFound) { + // Ignore document not found on the DBserver: + errorCode = TRI_ERROR_NO_ERROR; + } + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); ++dstRow; + } else { + if (producesOutput) { + VPackSlice resultList = opRes.slice(); + TRI_ASSERT(resultList.isArray()); + for (auto const& elm : VPackArrayIterator(resultList)) { + bool wasError = arangodb::basics::VelocyPackHelper::getBooleanValue(elm, "error", false); + if (!wasError) { + if (ep->_outVariableOld != nullptr) { + // store $OLD + result->setValue(dstRow, _outRegOld, AqlValue(elm.get("old"))); + } + if (ep->_outVariableNew != nullptr) { + // store $NEW + result->setValue(dstRow, _outRegNew, AqlValue(elm.get("new"))); + } + } + dstRow++; + } + } else { + dstRow += toUpdate.length(); + } + handleBabyResult(opRes.countErrorCodes, toUpdate.length(), + ep->_options.ignoreErrors); } // done with a block @@ -612,10 +690,13 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { options.ignoreRevs = true; VPackBuilder keyBuilder; - VPackBuilder object; + VPackBuilder insertBuilder; + VPackBuilder updateBuilder; // loop over all blocks size_t dstRow = 0; + std::vector insRows; + std::vector upRows; for (auto it = blocks.begin(); it != blocks.end(); ++it) { auto* res = *it; @@ -623,7 +704,19 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { size_t const n = res->size(); + bool isMultiple = (n > 1); + if (isMultiple) { + insertBuilder.clear(); + updateBuilder.clear(); + insertBuilder.openArray(); + updateBuilder.openArray(); + } + insRows.clear(); + upRows.clear(); + + int errorCode; // loop over the complete block + // Prepare both builders for (size_t i = 0; i < n; ++i) { AqlValue const& a = res->getValueReference(i, docRegisterId); @@ -632,7 +725,7 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { std::string key; - int errorCode = TRI_ERROR_NO_ERROR; + errorCode = TRI_ERROR_NO_ERROR; if (a.isObject()) { // old document present => update case @@ -648,58 +741,128 @@ AqlItemBlock* UpsertBlock::work(std::vector& blocks) { keyBuilder.openObject(); keyBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); keyBuilder.close(); - - object = VPackCollection::merge(toUpdate, keyBuilder.slice(), false, false); - toUpdate = object.slice(); - - OperationResult opRes; - if (ep->_isReplace) { - // replace - opRes = _trx->replace(_collection->name, toUpdate, options); + if (isMultiple) { + VPackBuilder tmp = VPackCollection::merge(toUpdate, keyBuilder.slice(), false, false); + updateBuilder.add(tmp.slice()); + upRows.emplace_back(dstRow); } else { - // update - opRes = _trx->update(_collection->name, toUpdate, options); + updateBuilder = VPackCollection::merge(toUpdate, keyBuilder.slice(), false, false); } - errorCode = opRes.code; - if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { - // store $NEW - result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); - } } else { errorCode = TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID; } } - } else { // no document found => insert case AqlValue const& insertDoc = res->getValueReference(i, insertRegisterId); - - if (insertDoc.isObject()) { - VPackSlice toInsert = insertDoc.slice(); - LOG(INFO) << _isDBServer << " t " << !_usesDefaultSharding << " f " << toInsert.hasKey(TRI_VOC_ATTRIBUTE_KEY); - - if (_isDBServer && !_usesDefaultSharding && - toInsert.hasKey(TRI_VOC_ATTRIBUTE_KEY)) { - errorCode = TRI_ERROR_CLUSTER_MUST_NOT_SPECIFY_KEY; - } else { - - OperationResult opRes = _trx->insert(_collection->name, toInsert, options); - errorCode = opRes.code; - - if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { - result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); - } - } + VPackSlice toInsert = insertDoc.slice(); + if (toInsert.isObject()) { + insertBuilder.add(toInsert); + insRows.emplace_back(dstRow); } else { errorCode = TRI_ERROR_ARANGO_DOCUMENT_TYPE_INVALID; } } - handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + if (errorCode != TRI_ERROR_NO_ERROR) { + // Handle the error here, it won't be send to server + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + } ++dstRow; } - // done with a block + // done with collecting a block + + if (isMultiple) { + insertBuilder.close(); + updateBuilder.close(); + } + + VPackSlice toInsert = insertBuilder.slice(); + VPackSlice toUpdate = updateBuilder.slice(); + + if (!toInsert.isNone()) { + if (isMultiple) { + TRI_ASSERT(toInsert.isArray()); + if (toInsert.length() != 0) { + OperationResult opRes = _trx->insert(_collection->name, toInsert, options); + if (producesOutput) { + VPackSlice resultList = opRes.slice(); + TRI_ASSERT(resultList.isArray()); + size_t i = 0; + for (auto const& elm : VPackArrayIterator(resultList)) { + bool wasError = + arangodb::basics::VelocyPackHelper::getBooleanValue( + elm, "error", false); + if (!wasError) { + // return $NEW + result->setValue(insRows[i], _outRegNew, AqlValue(elm.get("new"))); + } + ++i; + } + } + handleBabyResult(opRes.countErrorCodes, toInsert.length(), + ep->_options.ignoreErrors); + } + } else { + OperationResult opRes = _trx->insert(_collection->name, toInsert, options); + errorCode = opRes.code; + + if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { + result->setValue(dstRow - 1, _outRegNew, AqlValue(opRes.slice().get("new"))); + } + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + } + } + + if (!toUpdate.isNone()) { + if (isMultiple) { + TRI_ASSERT(toUpdate.isArray()); + if (toUpdate.length() != 0) { + OperationResult opRes; + if (ep->_isReplace) { + // replace + opRes = _trx->replace(_collection->name, toUpdate, options); + } else { + // update + opRes = _trx->update(_collection->name, toUpdate, options); + } + if (producesOutput) { + VPackSlice resultList = opRes.slice(); + TRI_ASSERT(resultList.isArray()); + size_t i = 0; + for (auto const& elm : VPackArrayIterator(resultList)) { + bool wasError = + arangodb::basics::VelocyPackHelper::getBooleanValue( + elm, "error", false); + if (!wasError) { + // return $NEW + result->setValue(upRows[i], _outRegNew, AqlValue(elm.get("new"))); + } + ++i; + } + } + handleBabyResult(opRes.countErrorCodes, toUpdate.length(), + ep->_options.ignoreErrors); + } + } else { + OperationResult opRes; + if (ep->_isReplace) { + // replace + opRes = _trx->replace(_collection->name, toUpdate, options); + } else { + // update + opRes = _trx->update(_collection->name, toUpdate, options); + } + errorCode = opRes.code; + + if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { + // store $NEW + result->setValue(dstRow - 1, _outRegNew, AqlValue(opRes.slice().get("new"))); + } + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + } + } // now free it already (*it) = nullptr; @@ -763,6 +926,11 @@ AqlItemBlock* ReplaceBlock::work(std::vector& blocks) { throwIfKilled(); // check if we were aborted size_t const n = res->size(); + bool isMultiple = (n > 1); + if (isMultiple) { + object.clear(); + object.openArray(); + } // loop over the complete block for (size_t i = 0; i < n; ++i) { @@ -788,47 +956,83 @@ AqlItemBlock* ReplaceBlock::work(std::vector& blocks) { } if (errorCode == TRI_ERROR_NO_ERROR) { - keyBuilder.clear(); - keyBuilder.openObject(); - keyBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); - keyBuilder.close(); - - VPackSlice toUpdate; if (hasKeyVariable) { - object = VPackCollection::merge(a.slice(), keyBuilder.slice(), false, false); - toUpdate = object.slice(); - } - else { - // use original slice for updating - toUpdate = a.slice(); - } - - // fetch old revision - OperationResult opRes = _trx->replace(_collection->name, toUpdate, options); - errorCode = opRes.code; - - if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { - if (ep->_outVariableOld != nullptr) { - // store $OLD - result->setValue(dstRow, _outRegOld, AqlValue(opRes.slice().get("old"))); - } - if (ep->_outVariableNew != nullptr) { - // store $NEW - result->setValue(dstRow, _outRegNew, AqlValue(opRes.slice().get("new"))); + keyBuilder.clear(); + keyBuilder.openObject(); + keyBuilder.add(TRI_VOC_ATTRIBUTE_KEY, VPackValue(key)); + keyBuilder.close(); + VPackBuilder tmp = VPackCollection::merge( + a.slice(), keyBuilder.slice(), false, false); + if (isMultiple) { + object.add(tmp.slice()); + } else { + object = tmp; } + } else { + // Use the original slice for updateing + object.add(a.slice()); } + } else { + handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); + } + } - if (errorCode == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND && _isDBServer && - ignoreDocumentNotFound) { - // Ignore document not found on the DBserver: - errorCode = TRI_ERROR_NO_ERROR; + if (isMultiple) { + object.close(); + } + + VPackSlice toUpdate = object.slice(); + // fetch old revision + OperationResult opRes = _trx->replace(_collection->name, toUpdate, options); + if (!isMultiple) { + int errorCode = opRes.code; + + if (producesOutput && errorCode == TRI_ERROR_NO_ERROR) { + if (ep->_outVariableOld != nullptr) { + // store $OLD + result->setValue(dstRow, _outRegOld, + AqlValue(opRes.slice().get("old"))); + } + if (ep->_outVariableNew != nullptr) { + // store $NEW + result->setValue(dstRow, _outRegNew, + AqlValue(opRes.slice().get("new"))); } } + if (errorCode == TRI_ERROR_ARANGO_DOCUMENT_NOT_FOUND && _isDBServer && + ignoreDocumentNotFound) { + // Ignore document not found on the DBserver: + errorCode = TRI_ERROR_NO_ERROR; + } handleResult(errorCode, ep->_options.ignoreErrors, &errorMessage); ++dstRow; + } else { + if (producesOutput) { + VPackSlice resultList = opRes.slice(); + TRI_ASSERT(resultList.isArray()); + for (auto const& elm : VPackArrayIterator(resultList)) { + bool wasError = arangodb::basics::VelocyPackHelper::getBooleanValue(elm, "error", false); + if (!wasError) { + if (ep->_outVariableOld != nullptr) { + // store $OLD + result->setValue(dstRow, _outRegOld, AqlValue(elm.get("old"))); + } + if (ep->_outVariableNew != nullptr) { + // store $NEW + result->setValue(dstRow, _outRegNew, AqlValue(elm.get("new"))); + } + } + dstRow++; + } + } else { + dstRow += toUpdate.length(); + } + handleBabyResult(opRes.countErrorCodes, toUpdate.length(), + ep->_options.ignoreErrors); } + // done with a block // now free it already