From 9cb08ded92c9eb9d505ef3f140dec20c894748cc Mon Sep 17 00:00:00 2001 From: Jan Date: Mon, 1 Jul 2019 16:35:28 +0200 Subject: [PATCH] make the comparison functions unambiguous (#9349) * make the comparison functions unambiguous * added @kaveh's suggestion --- .../include/velocypack/Collection.h | 2 +- .../velocypack/include/velocypack/Iterator.h | 4 +- .../velocypack/include/velocypack/Slice.h | 57 ++++++++++++------- 3rdParty/velocypack/src/Collection.cpp | 4 +- arangod/Agency/AgencyComm.cpp | 8 +-- arangod/Agency/FailedLeader.cpp | 2 +- arangod/Agency/Job.cpp | 12 ++-- arangod/Agency/Store.cpp | 39 ++++++------- arangod/Agency/Supervision.cpp | 2 +- arangod/Aql/AqlValue.h | 2 +- arangod/Aql/Functions.cpp | 3 +- arangod/Aql/OptimizerRules.cpp | 2 +- arangod/Cluster/AgencyCallback.cpp | 3 +- arangod/Cluster/ClusterInfo.cpp | 6 +- arangod/Cluster/ClusterMethods.cpp | 2 +- arangod/Cluster/Maintenance.cpp | 4 +- arangod/Graph/EdgeDocumentToken.h | 3 +- arangod/Graph/KShortestPathsFinder.h | 7 ++- arangod/Graph/PathEnumerator.cpp | 15 +++-- arangod/IResearch/IResearchLinkHelper.cpp | 2 +- arangod/IResearch/IResearchViewMeta.cpp | 8 ++- arangod/MMFiles/MMFilesEdgeIndex.h | 4 +- arangod/MMFiles/MMFilesPrimaryIndex.h | 4 +- arangod/Replication/TailingSyncer.cpp | 2 +- arangod/RestServer/BootstrapFeature.cpp | 2 +- arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp | 2 +- lib/Basics/VelocyPackHelper.h | 2 +- tests/Aql/AqlHelper.cpp | 3 +- tests/Aql/ShortestPathExecutorTest.cpp | 8 ++- tests/Mocks/StorageEngineMock.cpp | 4 +- 30 files changed, 120 insertions(+), 98 deletions(-) diff --git a/3rdParty/velocypack/include/velocypack/Collection.h b/3rdParty/velocypack/include/velocypack/Collection.h index 2cf3a5163d..2f2794dbfb 100644 --- a/3rdParty/velocypack/include/velocypack/Collection.h +++ b/3rdParty/velocypack/include/velocypack/Collection.h @@ -224,7 +224,7 @@ class Collection { struct IsEqualPredicate { IsEqualPredicate(Slice const& value) : value(value) {} bool operator()(Slice const& current, ValueLength) { - return value.equals(current); + return value.binaryEquals(current); } // compare value Slice const value; diff --git a/3rdParty/velocypack/include/velocypack/Iterator.h b/3rdParty/velocypack/include/velocypack/Iterator.h index a8682f1aa7..8ebc328e2b 100644 --- a/3rdParty/velocypack/include/velocypack/Iterator.h +++ b/3rdParty/velocypack/include/velocypack/Iterator.h @@ -50,7 +50,7 @@ class ArrayIterator { throw Exception(Exception::InvalidValueType, "Expecting Array slice"); } - _size = slice.arrayLength(head); + _size = slice.arrayLength(); if (_size > 0) { VELOCYPACK_ASSERT(head != 0x01); // no empty array allowed here @@ -191,7 +191,7 @@ class ObjectIterator { throw Exception(Exception::InvalidValueType, "Expecting Object slice"); } - _size = slice.objectLength(head); + _size = slice.objectLength(); if (_size > 0) { VELOCYPACK_ASSERT(head != 0x0a); // no empty object allowed here diff --git a/3rdParty/velocypack/include/velocypack/Slice.h b/3rdParty/velocypack/include/velocypack/Slice.h index bf1333a5c2..0146bf9e32 100644 --- a/3rdParty/velocypack/include/velocypack/Slice.h +++ b/3rdParty/velocypack/include/velocypack/Slice.h @@ -914,7 +914,16 @@ class Slice { } // check if two Slices are equal on the binary level - bool equals(Slice const& other) const { + // please note that for several values there are multiple possible representations, + // which differ on the binary level but will still resolve to the same logical + // values. For example, smallint(1) and int(1) are logically the same, but will + // resolve to either 0x31 or 0x28 0x01. + bool binaryEquals(Slice const& other) const { + if (start() == other.start()) { + // same underlying data, so the slices must be identical + return true; + } + if (head() != other.head()) { return false; } @@ -927,13 +936,19 @@ class Slice { return (memcmp(start(), other.start(), checkOverflow(size)) == 0); } - - bool operator==(Slice const& other) const { return equals(other); } - bool operator!=(Slice const& other) const { return !equals(other); } - - static bool equals(uint8_t const* left, uint8_t const* right) { - return Slice(left).equals(Slice(right)); + + static bool binaryEquals(uint8_t const* left, uint8_t const* right) { + return Slice(left).binaryEquals(Slice(right)); } + + // these operators are now deleted because they didn't do what people expected + // these operators checked for _binary_ equality of the velocypack slice with + // another. however, for several values there are multiple possible representations, + // which differ on the binary level but will still resolve to the same logical + // values. For example, smallint(1) and int(1) are logically the same, but will + // resolve to either 0x31 or 0x28 0x01. + bool operator==(Slice const& other) const = delete; + bool operator!=(Slice const& other) const = delete; std::string toHex() const; std::string toJson(Options const* options = &Options::Defaults) const; @@ -953,27 +968,28 @@ class Slice { private: // return the number of members for an Array // must only be called for Slices that have been validated to be of type Array - ValueLength arrayLength(uint8_t head) const { - VELOCYPACK_ASSERT(type(head) == ValueType::Array); + ValueLength arrayLength() const { + auto const h = head(); + VELOCYPACK_ASSERT(type(h) == ValueType::Array); - if (head == 0x01) { + if (h == 0x01) { // special case: empty! return 0; } - if (head == 0x13) { + if (h == 0x13) { // compact Array ValueLength end = readVariableValueLength(_start + 1); return readVariableValueLength(_start + end - 1); } - ValueLength const offsetSize = indexEntrySize(head); + ValueLength const offsetSize = indexEntrySize(h); VELOCYPACK_ASSERT(offsetSize > 0); // find number of items - if (head <= 0x05) { // No offset table or length, need to compute: - VELOCYPACK_ASSERT(head != 0x00 && head != 0x01); - ValueLength firstSubOffset = findDataOffset(head); + if (h <= 0x05) { // No offset table or length, need to compute: + VELOCYPACK_ASSERT(h != 0x00 && h != 0x01); + ValueLength firstSubOffset = findDataOffset(h); Slice first(_start + firstSubOffset); ValueLength s = first.byteSize(); if (VELOCYPACK_UNLIKELY(s == 0)) { @@ -991,21 +1007,22 @@ class Slice { // return the number of members for an Object // must only be called for Slices that have been validated to be of type Object - ValueLength objectLength(uint8_t head) const { - VELOCYPACK_ASSERT(type(head) == ValueType::Object); + ValueLength objectLength() const { + auto const h = head(); + VELOCYPACK_ASSERT(type(h) == ValueType::Object); - if (head == 0x0a) { + if (h == 0x0a) { // special case: empty! return 0; } - if (head == 0x14) { + if (h == 0x14) { // compact Object ValueLength end = readVariableValueLength(_start + 1); return readVariableValueLength(_start + end - 1); } - ValueLength const offsetSize = indexEntrySize(head); + ValueLength const offsetSize = indexEntrySize(h); VELOCYPACK_ASSERT(offsetSize > 0); if (offsetSize < 8) { diff --git a/3rdParty/velocypack/src/Collection.cpp b/3rdParty/velocypack/src/Collection.cpp index 17de56e1d6..3d5493a841 100644 --- a/3rdParty/velocypack/src/Collection.cpp +++ b/3rdParty/velocypack/src/Collection.cpp @@ -128,7 +128,7 @@ bool Collection::contains(Slice const& slice, Slice const& other) { ArrayIterator it(slice); while (it.valid()) { - if (it.value().equals(other)) { + if (it.value().binaryEquals(other)) { return true; } it.next(); @@ -142,7 +142,7 @@ ValueLength Collection::indexOf(Slice const& slice, Slice const& other) { ValueLength index = 0; while (it.valid()) { - if (it.value().equals(other)) { + if (it.value().binaryEquals(other)) { return index; } it.next(); diff --git a/arangod/Agency/AgencyComm.cpp b/arangod/Agency/AgencyComm.cpp index 268c4b1c24..a18d4e8ef7 100644 --- a/arangod/Agency/AgencyComm.cpp +++ b/arangod/Agency/AgencyComm.cpp @@ -1854,14 +1854,14 @@ bool AgencyComm::shouldInitializeStructure() { // Sanity if (result.slice().isArray() && result.slice().length() == 1) { - // No plan entry? Should initialise - if (result.slice()[0] == VPackSlice::emptyObjectSlice()) { + // No plan entry? Should initialize + if (result.slice()[0].isObject() && result.slice()[0].length() == 0) { LOG_TOPIC("98732", DEBUG, Logger::AGENCYCOMM) - << "agency initialisation should be performed"; + << "agency initialization should be performed"; return true; } else { LOG_TOPIC("abedb", DEBUG, Logger::AGENCYCOMM) - << "agency initialisation under way or done"; + << "agency initialization under way or done"; return false; } } else { diff --git a/arangod/Agency/FailedLeader.cpp b/arangod/Agency/FailedLeader.cpp index 9da6729be3..f56585492d 100644 --- a/arangod/Agency/FailedLeader.cpp +++ b/arangod/Agency/FailedLeader.cpp @@ -431,7 +431,7 @@ JOB_STATUS FailedLeader::status() { auto cur_slice = _snapshot.hasAsSlice(curColPrefix + sub + "/" + clone.shard + "/servers"); if (plan_slice.second && cur_slice.second && - plan_slice.first[0] != cur_slice.first[0]) { + basics::VelocyPackHelper::compare(plan_slice.first[0], cur_slice.first[0], false) != 0) { LOG_TOPIC("0d8ca", DEBUG, Logger::SUPERVISION) << "FailedLeader waiting for " << sub + "/" + shard; break; diff --git a/arangod/Agency/Job.cpp b/arangod/Agency/Job.cpp index 6ac70b44e6..72c5ebae0f 100644 --- a/arangod/Agency/Job.cpp +++ b/arangod/Agency/Job.cpp @@ -150,12 +150,10 @@ bool Job::finish(std::string const& server, std::string const& shard, } // -- operations - if (preconditions != Slice::emptyObjectSlice()) { // preconditions -- + if (preconditions.isObject() && preconditions.length() > 0) { // preconditions -- VPackObjectBuilder precguard(&finished); - if (preconditions.length() > 0) { - for (auto const& prec : VPackObjectIterator(preconditions)) { - finished.add(prec.key.copyString(), prec.value); - } + for (auto const& prec : VPackObjectIterator(preconditions)) { + finished.add(prec.key.copyString(), prec.value); } } // -- preconditions @@ -489,9 +487,9 @@ std::string Job::findNonblockedCommonHealthyInSyncFollower( // Which is in "GOO bool found = false; for (const auto& plannedServer : VPackArrayIterator(snap.hasAsArray(plannedShardPath).first)) { - if (plannedServer == server) { + if (plannedServer.isEqualString(server.stringRef())) { found = true; - continue; + break; } } diff --git a/arangod/Agency/Store.cpp b/arangod/Agency/Store.cpp index 0ee95c4d23..fc5b381277 100644 --- a/arangod/Agency/Store.cpp +++ b/arangod/Agency/Store.cpp @@ -444,18 +444,17 @@ check_ret_t Store::check(VPackSlice const& slice, CheckMode mode) const { } else if (oper == "in") { // in if (found) { if (node->slice().isArray()) { - bool _found = false; + bool found = false; for (auto const& i : VPackArrayIterator(node->slice())) { - if (i == op.value) { - _found = true; - continue; + if (basics::VelocyPackHelper::compare(i, op.value, false) == 0) { + found = true; + break; } } - if (_found) { + if (found) { continue; - } else { - ret.push_back(precond.key); - } + } + ret.push_back(precond.key); } } ret.push_back(precond.key); @@ -465,21 +464,19 @@ check_ret_t Store::check(VPackSlice const& slice, CheckMode mode) const { } else if (oper == "notin") { // in if (!found) { continue; - } else { - if (node->slice().isArray()) { - bool _found = false; - for (auto const& i : VPackArrayIterator(node->slice())) { - if (i == op.value) { - _found = true; - continue; - } - } - if (_found) { - ret.push_back(precond.key); - } else { - continue; + } + if (node->slice().isArray()) { + bool found = false; + for (auto const& i : VPackArrayIterator(node->slice())) { + if (basics::VelocyPackHelper::compare(i, op.value, false) == 0) { + found = true; + break; } } + if (!found) { + continue; + } + ret.push_back(precond.key); } ret.push_back(precond.key); if (mode == FIRST_FAIL) { diff --git a/arangod/Agency/Supervision.cpp b/arangod/Agency/Supervision.cpp index eee11d7c6d..34776eac55 100644 --- a/arangod/Agency/Supervision.cpp +++ b/arangod/Agency/Supervision.cpp @@ -1318,7 +1318,7 @@ void Supervision::readyOrphanedIndexCreations() { currentDBs(colPath + shname + "/indexes").slice(); for (auto const& curIndex : VPackArrayIterator(curIndexes)) { auto const& curId = curIndex.get("id"); - if (planId == curId) { + if (basics::VelocyPackHelper::compare(planId, curId, false) == 0) { ++nIndexes; } } diff --git a/arangod/Aql/AqlValue.h b/arangod/Aql/AqlValue.h index 489da38cfd..8f3eac420c 100644 --- a/arangod/Aql/AqlValue.h +++ b/arangod/Aql/AqlValue.h @@ -770,7 +770,7 @@ struct equal_to { if (type == arangodb::aql::AqlValue::VPACK_INLINE) { try { return arangodb::velocypack::Slice(&a._data.internal[0]) - .equals(arangodb::velocypack::Slice(&b._data.internal[0])); + .binaryEquals(arangodb::velocypack::Slice(&b._data.internal[0])); } catch (...) { TRI_ASSERT(false); } diff --git a/arangod/Aql/Functions.cpp b/arangod/Aql/Functions.cpp index a1471fd788..877f9acbcc 100644 --- a/arangod/Aql/Functions.cpp +++ b/arangod/Aql/Functions.cpp @@ -5961,7 +5961,8 @@ AqlValue Functions::Append(ExpressionContext* expressionContext, transaction::Me return AqlValue(AqlValueHintNull()); } - std::unordered_set added; + std::unordered_set added( + 11, basics::VelocyPackHelper::VPackHash(), basics::VelocyPackHelper::VPackEqual()); transaction::BuilderLeaser builder(trx); builder->openArray(); diff --git a/arangod/Aql/OptimizerRules.cpp b/arangod/Aql/OptimizerRules.cpp index c47b8c6c6a..8086358f51 100644 --- a/arangod/Aql/OptimizerRules.cpp +++ b/arangod/Aql/OptimizerRules.cpp @@ -1490,7 +1490,7 @@ class PropagateConstantAttributesHelper { return; } - if (!value->computeValue().equals(previous->computeValue())) { + if (!value->computeValue().binaryEquals(previous->computeValue())) { // different value found for an already tracked attribute. better not // use this attribute (*it2).second = nullptr; diff --git a/arangod/Cluster/AgencyCallback.cpp b/arangod/Cluster/AgencyCallback.cpp index f4e056deca..eb202e686e 100644 --- a/arangod/Cluster/AgencyCallback.cpp +++ b/arangod/Cluster/AgencyCallback.cpp @@ -33,6 +33,7 @@ #include "ApplicationFeatures/ApplicationServer.h" #include "Basics/ConditionLocker.h" #include "Basics/MutexLocker.h" +#include "Basics/VelocyPackHelper.h" #include "Logger/Logger.h" using namespace arangodb; @@ -90,7 +91,7 @@ void AgencyCallback::refetchAndUpdate(bool needToAcquireMutex, bool forceCheck) void AgencyCallback::checkValue(std::shared_ptr newData, bool forceCheck) { // Only called from refetchAndUpdate, we always have the mutex when // we get here! - if (!_lastData || !_lastData->slice().equals(newData->slice()) || forceCheck) { + if (!_lastData || arangodb::basics::VelocyPackHelper::compare(_lastData->slice(), newData->slice(), false) != 0 || forceCheck) { LOG_TOPIC("2bd14", DEBUG, Logger::CLUSTER) << "AgencyCallback: Got new value " << newData->slice().typeName() << " " << newData->toJson() << " forceCheck=" << forceCheck; diff --git a/arangod/Cluster/ClusterInfo.cpp b/arangod/Cluster/ClusterInfo.cpp index 9142805fc5..4295833bf5 100644 --- a/arangod/Cluster/ClusterInfo.cpp +++ b/arangod/Cluster/ClusterInfo.cpp @@ -3647,7 +3647,7 @@ void ClusterInfo::loadCurrentDBServers() { bool found = false; if (failedDBServers.isObject()) { for (auto const& failedServer : VPackObjectIterator(failedDBServers)) { - if (dbserver.key == failedServer.key) { + if (basics::VelocyPackHelper::compare(dbserver.key, failedServer.key, false) == 0) { found = true; break; } @@ -3660,7 +3660,7 @@ void ClusterInfo::loadCurrentDBServers() { if (cleanedDBServers.isArray()) { bool found = false; for (auto const& cleanedServer : VPackArrayIterator(cleanedDBServers)) { - if (dbserver.key == cleanedServer) { + if (basics::VelocyPackHelper::compare(dbserver.key, cleanedServer, false) == 0) { found = true; break; } @@ -3673,7 +3673,7 @@ void ClusterInfo::loadCurrentDBServers() { if (toBeCleanedDBServers.isArray()) { bool found = false; for (auto const& toBeCleanedServer : VPackArrayIterator(toBeCleanedDBServers)) { - if (dbserver.key == toBeCleanedServer) { + if (basics::VelocyPackHelper::compare(dbserver.key, toBeCleanedServer, false) == 0) { found = true; break; } diff --git a/arangod/Cluster/ClusterMethods.cpp b/arangod/Cluster/ClusterMethods.cpp index 0fea5827f5..b97b375b0f 100644 --- a/arangod/Cluster/ClusterMethods.cpp +++ b/arangod/Cluster/ClusterMethods.cpp @@ -314,7 +314,7 @@ static void mergeResultsAllShards(std::vector> con VPackSlice oneRes = it->slice(); TRI_ASSERT(oneRes.isArray()); oneRes = oneRes.at(currentIndex); - if (!oneRes.equals(notFound)) { + if (basics::VelocyPackHelper::compare(oneRes, notFound, false) != 0) { // This is the correct result // Use it resultBody->add(oneRes); diff --git a/arangod/Cluster/Maintenance.cpp b/arangod/Cluster/Maintenance.cpp index 6e79af1c26..db3f8092cb 100644 --- a/arangod/Cluster/Maintenance.cpp +++ b/arangod/Cluster/Maintenance.cpp @@ -85,7 +85,7 @@ static std::shared_ptr compareRelevantProps(VPackSlice const& firs VPackObjectBuilder b(result.get()); for (auto const& property : cmp) { auto const& planned = first.get(property); - if (planned != second.get(property)) { // Register any change + if (basics::VelocyPackHelper::compare(planned, second.get(property), false) != 0) { // Register any change result->add(property, planned); } } @@ -233,7 +233,7 @@ void handlePlanShard(VPackSlice const& cprops, VPackSlice const& ldb, } // If comparison has brought any updates - if (properties->slice() != VPackSlice::emptyObjectSlice() || + if (!properties->slice().isObject() || properties->slice().length() > 0 || leading != shouldBeLeading || !followersToDropString.empty()) { if (errors.shards.find(fullShardLabel) == errors.shards.end()) { actions.emplace_back(ActionDescription( diff --git a/arangod/Graph/EdgeDocumentToken.h b/arangod/Graph/EdgeDocumentToken.h index 35aba801e2..2c1283cb59 100644 --- a/arangod/Graph/EdgeDocumentToken.h +++ b/arangod/Graph/EdgeDocumentToken.h @@ -118,8 +118,7 @@ struct EdgeDocumentToken { #ifdef ARANGODB_ENABLE_MAINTAINER_MODE TRI_ASSERT(_type == TokenType::COORDINATOR); #endif - // FIXME: - return velocypack::Slice(_data.vpack) == velocypack::Slice(other._data.vpack); + return velocypack::Slice(_data.vpack).binaryEquals(velocypack::Slice(other._data.vpack)); } bool equalsLocal(EdgeDocumentToken const& other) const { diff --git a/arangod/Graph/KShortestPathsFinder.h b/arangod/Graph/KShortestPathsFinder.h index cd2616236d..77d21e3a39 100644 --- a/arangod/Graph/KShortestPathsFinder.h +++ b/arangod/Graph/KShortestPathsFinder.h @@ -88,6 +88,7 @@ class KShortestPathsFinder : public ShortestPathFinder { // Only append paths where the first vertex of p // is the same as the last vertex of this. TRI_ASSERT((_vertices.back().equals(p._vertices.front()))); + TRI_ASSERT(!_weights.empty()); double ew = _weights.back(); double pw = p._weights.at(a); @@ -98,7 +99,7 @@ class KShortestPathsFinder : public ShortestPathFinder { _weights.emplace_back(ew + (p._weights.at(a) - pw)); } _weight = _weights.back(); - }; + } // TODO: implement == for EdgeDocumentToken and VertexRef // so these things become less cluttery bool operator==(Path const& rhs) const { @@ -107,12 +108,12 @@ class KShortestPathsFinder : public ShortestPathFinder { return false; } for (size_t i = 0; i < _vertices.size(); ++i) { - if (!_vertices.at(i).equals(rhs._vertices.at(i))) { + if (!_vertices[i].equals(rhs._vertices[i])) { return false; } } for (size_t i = 0; i < _edges.size(); ++i) { - if (!_edges.at(i).equals(rhs._edges.at(i))) { + if (!_edges[i].equals(rhs._edges[i])) { return false; } } diff --git a/arangod/Graph/PathEnumerator.cpp b/arangod/Graph/PathEnumerator.cpp index 85f2a4a05e..e2ced71cfb 100644 --- a/arangod/Graph/PathEnumerator.cpp +++ b/arangod/Graph/PathEnumerator.cpp @@ -106,15 +106,18 @@ bool DepthFirstEnumerator::next() { } if (_opts->uniqueEdges == TraverserOptions::UniquenessLevel::PATH) { - ServerState::RoleEnum role = ServerState::instance()->getRole(); - for (auto const& it : _enumeratedPath.edges) { - // We might already have this edge on the path. - if (ServerState::isCoordinator(role)) { + if (ServerState::instance()->isCoordinator()) { + for (auto const& it : _enumeratedPath.edges) { + // We might already have this edge on the path. if (it.equalsCoordinator(eid)) { return; } - } else if (it.equalsLocal(eid)) { - return; + } + } else { + for (auto const& it : _enumeratedPath.edges) { + if (it.equalsLocal(eid)) { + return; + } } } } diff --git a/arangod/IResearch/IResearchLinkHelper.cpp b/arangod/IResearch/IResearchLinkHelper.cpp index f5ea30bd4a..e38bd95eb0 100644 --- a/arangod/IResearch/IResearchLinkHelper.cpp +++ b/arangod/IResearch/IResearchLinkHelper.cpp @@ -577,7 +577,7 @@ namespace iresearch { auto lhsViewSlice = lhs.get(StaticStrings::ViewIdField); auto rhsViewSlice = rhs.get(StaticStrings::ViewIdField); - if (!lhsViewSlice.equals(rhsViewSlice)) { + if (!lhsViewSlice.binaryEquals(rhsViewSlice)) { if (!lhsViewSlice.isString() || !rhsViewSlice.isString()) { return false; } diff --git a/arangod/IResearch/IResearchViewMeta.cpp b/arangod/IResearch/IResearchViewMeta.cpp index ce86f6915b..c13d45b2f7 100644 --- a/arangod/IResearch/IResearchViewMeta.cpp +++ b/arangod/IResearch/IResearchViewMeta.cpp @@ -25,6 +25,7 @@ #include "utils/locale_utils.hpp" #include "Basics/StringUtils.h" +#include "Basics/VelocyPackHelper.h" #include "IResearchCommon.h" #include "VelocyPackHelper.h" #include "VocBase/LogicalView.h" @@ -285,7 +286,7 @@ bool IResearchViewMeta::operator==(IResearchViewMeta const& other) const noexcep } try { - if (!_consolidationPolicy.properties().equals(other._consolidationPolicy.properties())) { + if (basics::VelocyPackHelper::compare(_consolidationPolicy.properties(), other._consolidationPolicy.properties(), false) != 0) { return false; // values do not match } } catch (...) { @@ -620,8 +621,9 @@ bool IResearchViewMeta::json(arangodb::velocypack::Builder& builder, arangodb::velocypack::Value(_consolidationIntervalMsec)); } - if ((!ignoreEqual || !_consolidationPolicy.properties().equals( - ignoreEqual->_consolidationPolicy.properties())) && + if ((!ignoreEqual || arangodb::basics::VelocyPackHelper::compare( + _consolidationPolicy.properties(), + ignoreEqual->_consolidationPolicy.properties(), false) != 0) && (!mask || mask->_consolidationPolicy)) { builder.add("consolidationPolicy", _consolidationPolicy.properties()); } diff --git a/arangod/MMFiles/MMFilesEdgeIndex.h b/arangod/MMFiles/MMFilesEdgeIndex.h index d7bae716fb..cf33d83dfd 100644 --- a/arangod/MMFiles/MMFilesEdgeIndex.h +++ b/arangod/MMFiles/MMFilesEdgeIndex.h @@ -74,7 +74,7 @@ struct MMFilesEdgeIndexHelper { try { VPackSlice tmp = right.slice(context); TRI_ASSERT(tmp.isString()); - return left->equals(tmp); + return left->binaryEquals(tmp); } catch (...) { return false; } @@ -98,7 +98,7 @@ struct MMFilesEdgeIndexHelper { TRI_ASSERT(lSlice.isString()); TRI_ASSERT(rSlice.isString()); - return lSlice.equals(rSlice); + return lSlice.binaryEquals(rSlice); } catch (...) { return false; } diff --git a/arangod/MMFiles/MMFilesPrimaryIndex.h b/arangod/MMFiles/MMFilesPrimaryIndex.h index 20c69ea3ce..5fc1fcdbcb 100644 --- a/arangod/MMFiles/MMFilesPrimaryIndex.h +++ b/arangod/MMFiles/MMFilesPrimaryIndex.h @@ -63,7 +63,7 @@ struct MMFilesPrimaryIndexHelper { try { VPackSlice tmp = right.slice(context); TRI_ASSERT(tmp.isString()); - return VPackSlice(key).equals(tmp); + return VPackSlice(key).binaryEquals(tmp); } catch (...) { return false; } @@ -89,7 +89,7 @@ struct MMFilesPrimaryIndexHelper { VPackSlice r = right.slice(context); TRI_ASSERT(l.isString()); TRI_ASSERT(r.isString()); - return l.equals(r); + return l.binaryEquals(r); } }; diff --git a/arangod/Replication/TailingSyncer.cpp b/arangod/Replication/TailingSyncer.cpp index de234efcb0..033dc46726 100644 --- a/arangod/Replication/TailingSyncer.cpp +++ b/arangod/Replication/TailingSyncer.cpp @@ -328,7 +328,7 @@ Result TailingSyncer::processDBMarker(TRI_replication_operation_e type, TRI_ERROR_REPLICATION_INVALID_RESPONSE, "create database marker did not contain data"); } - TRI_ASSERT(data.get("name") == nameSlice); + TRI_ASSERT(basics::VelocyPackHelper::compare(data.get("name"), nameSlice, false) == 0); TRI_vocbase_t* vocbase = DatabaseFeature::DATABASE->lookupDatabase(name); diff --git a/arangod/RestServer/BootstrapFeature.cpp b/arangod/RestServer/BootstrapFeature.cpp index 7301653785..d70d6783ae 100644 --- a/arangod/RestServer/BootstrapFeature.cpp +++ b/arangod/RestServer/BootstrapFeature.cpp @@ -250,7 +250,7 @@ void runActiveFailoverStart(std::string const& myId) { if (leader.isString() && leader.getStringLength() > 0) { ServerState::instance()->setFoxxmaster(leader.copyString()); - if (leader == myIdBuilder.slice()) { + if (basics::VelocyPackHelper::compare(leader, myIdBuilder.slice(), false) == 0) { LOG_TOPIC("95023", INFO, Logger::STARTUP) << "Became leader in active-failover setup"; } else { diff --git a/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp b/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp index a2b8fa9303..b66864bef3 100644 --- a/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp +++ b/arangod/RocksDBEngine/RocksDBPrimaryIndex.cpp @@ -625,7 +625,7 @@ Result RocksDBPrimaryIndex::update(transaction::Methods& trx, RocksDBMethods* mt Index::OperationMode mode) { Result res; VPackSlice keySlice = transaction::helpers::extractKeyFromDocument(oldDoc); - TRI_ASSERT(keySlice == oldDoc.get(StaticStrings::KeyString)); + TRI_ASSERT(keySlice.binaryEquals(oldDoc.get(StaticStrings::KeyString))); RocksDBKeyLeaser key(&trx); key->constructPrimaryIndexValue(_objectId, arangodb::velocypack::StringRef(keySlice)); diff --git a/lib/Basics/VelocyPackHelper.h b/lib/Basics/VelocyPackHelper.h index 3b0e292961..8dce5bf982 100644 --- a/lib/Basics/VelocyPackHelper.h +++ b/lib/Basics/VelocyPackHelper.h @@ -397,7 +397,7 @@ template <> struct equal_to { bool operator()(arangodb::basics::VPackHashedSlice const& lhs, arangodb::basics::VPackHashedSlice const& rhs) const { - return lhs.slice.equals(rhs.slice); + return lhs.slice.binaryEquals(rhs.slice); } }; diff --git a/tests/Aql/AqlHelper.cpp b/tests/Aql/AqlHelper.cpp index ba34f7cc34..79a7849845 100644 --- a/tests/Aql/AqlHelper.cpp +++ b/tests/Aql/AqlHelper.cpp @@ -23,6 +23,7 @@ #include "AqlHelper.h" #include "Aql/ExecutionStats.h" +#include "Basics/VelocyPackHelper.h" using namespace arangodb; using namespace arangodb::aql; @@ -90,7 +91,7 @@ bool arangodb::aql::operator==(arangodb::aql::AqlItemBlock const& left, AqlValue const& l = left.getValueReference(row, reg); AqlValue const& r = right.getValueReference(row, reg); // Doesn't work for docvecs or ranges - if (l.slice() != r.slice()) { + if (arangodb::basics::VelocyPackHelper::compare(l.slice(), r.slice(), false) != 0) { return false; } } diff --git a/tests/Aql/ShortestPathExecutorTest.cpp b/tests/Aql/ShortestPathExecutorTest.cpp index 98f4ffadac..6ab1740721 100644 --- a/tests/Aql/ShortestPathExecutorTest.cpp +++ b/tests/Aql/ShortestPathExecutorTest.cpp @@ -54,8 +54,10 @@ namespace aql { class TokenTranslator : public TraverserCache { public: - TokenTranslator(Query* query) : TraverserCache(query) {} - ~TokenTranslator(){}; + TokenTranslator(Query* query) + : TraverserCache(query), + _edges(11, arangodb::basics::VelocyPackHelper::VPackHash(), arangodb::basics::VelocyPackHelper::VPackEqual()) {} + ~TokenTranslator() {} arangodb::velocypack::StringRef makeVertex(std::string const& id) { VPackBuilder vertex; @@ -105,7 +107,7 @@ class TokenTranslator : public TraverserCache { private: std::vector>> _dataLake; std::unordered_map _vertices; - std::unordered_set _edges; + std::unordered_set _edges; }; class FakePathFinder : public ShortestPathFinder { diff --git a/tests/Mocks/StorageEngineMock.cpp b/tests/Mocks/StorageEngineMock.cpp index 5a79139ac6..51f096d211 100644 --- a/tests/Mocks/StorageEngineMock.cpp +++ b/tests/Mocks/StorageEngineMock.cpp @@ -832,7 +832,7 @@ arangodb::Result PhysicalCollectionMock::remove( arangodb::velocypack::Builder& doc = entry.first; - if (key == doc.slice().get(arangodb::StaticStrings::KeyString)) { + if (arangodb::basics::VelocyPackHelper::compare(key, doc.slice().get(arangodb::StaticStrings::KeyString), false) == 0) { entry.second = false; previous.setUnmanaged(doc.data()); TRI_ASSERT(previous.revisionId() == TRI_ExtractRevisionId(doc.slice())); @@ -896,7 +896,7 @@ arangodb::Result PhysicalCollectionMock::update( auto& doc = entry.first; - if (key == doc.slice().get(arangodb::StaticStrings::KeyString)) { + if (arangodb::basics::VelocyPackHelper::compare(key, doc.slice().get(arangodb::StaticStrings::KeyString), false) == 0) { if (!options.mergeObjects) { entry.second = false; previous.setUnmanaged(doc.data());