tursom/memgraph
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address review comments

Browse Source
This commit is contained in:
jbajic 2022-12-15 11:25:37 +01:00
parent 817433d342
commit a0e0791aa1
6 changed files with 36 additions and 83 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/storage/v3/delta.hpp
View File

@ -11,6 +11,7 @@
#pragma once
#include <cstdint>
#include <memory>
#include "storage/v3/edge_ref.hpp"
#include "storage/v3/id_types.hpp"
@ -129,7 +130,7 @@ inline bool operator==(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer
inline bool operator!=(const PreviousPtr::Pointer &a, const PreviousPtr::Pointer &b) { return !(a == b); }
struct Delta {
enum class Action {
enum class Action : uint8_t {
// Used for both Vertex and Edge
DELETE_OBJECT,
RECREATE_OBJECT,

51
src/storage/v3/indices.cpp
View File

@ -12,8 +12,6 @@
#include "indices.hpp"
#include <algorithm>
#include <cstddef>
#include <cstdint>
#include <functional>
#include <limits>
@ -342,7 +340,7 @@ LabelIndex::Iterable::Iterator &LabelIndex::Iterable::Iterator::operator++() {
}
void LabelIndex::Iterable::Iterator::AdvanceUntilValid() {
for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) {
for (; index_iterator_ != self_->index_container_->end(); ++index_iterator_) {
if (index_iterator_->vertex == current_vertex_) {
continue;
}
@ -355,9 +353,9 @@ void LabelIndex::Iterable::Iterator::AdvanceUntilValid() {
}
}
LabelIndex::Iterable::Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction,
LabelIndex::Iterable::Iterable(LabelIndexContainer &index_container, LabelId label, View view, Transaction *transaction,
Indices *indices, Config::Items config, const VertexValidator &vertex_validator)
: index_accessor_(&index_accessor),
: index_container_(&index_container),
label_(label),
view_(view),
transaction_(transaction),
@ -482,7 +480,7 @@ LabelPropertyIndex::Iterable::Iterator &LabelPropertyIndex::Iterable::Iterator::
}
void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() {
for (; index_iterator_ != self_->index_accessor_->end(); ++index_iterator_) {
for (; index_iterator_ != self_->index_container_->end(); ++index_iterator_) {
if (index_iterator_->vertex == current_vertex_) {
continue;
}
@ -497,11 +495,11 @@ void LabelPropertyIndex::Iterable::Iterator::AdvanceUntilValid() {
}
if (self_->upper_bound_) {
if (self_->upper_bound_->value() < index_iterator_->value) {
index_iterator_ = self_->index_accessor_->end();
index_iterator_ = self_->index_container_->end();
break;
}
if (!self_->upper_bound_->IsInclusive() && index_iterator_->value == self_->upper_bound_->value()) {
index_iterator_ = self_->index_accessor_->end();
index_iterator_ = self_->index_container_->end();
break;
}
}
@ -528,12 +526,12 @@ const PropertyValue kSmallestMap = PropertyValue(std::map<std::string, PropertyV
const PropertyValue kSmallestTemporalData =
PropertyValue(TemporalData{static_cast<TemporalType>(0), std::numeric_limits<int64_t>::min()});
LabelPropertyIndex::Iterable::Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property,
LabelPropertyIndex::Iterable::Iterable(LabelPropertyIndexContainer &index_container, LabelId label, PropertyId property,
const std::optional<utils::Bound<PropertyValue>> &lower_bound,
const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view,
Transaction *transaction, Indices *indices, Config::Items config,
const VertexValidator &vertex_validator)
: index_accessor_(&index_accessor),
: index_container_(&index_container),
label_(label),
property_(property),
lower_bound_(lower_bound),
@ -641,29 +639,24 @@ LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::begin() {
// If the bounds are set and don't have comparable types we don't yield any
// items from the index.
if (!bounds_valid_) {
return {this, index_accessor_->end()};
return {this, index_container_->end()};
}
auto index_iterator = index_accessor_->begin();
if (lower_bound_) {
index_iterator = std::ranges::find_if(*index_accessor_, [lower_bound = lower_bound_->value()](const auto &pair) {
return pair.value >= lower_bound;
});
return {this, std::ranges::lower_bound(*index_container_, lower_bound_->value(), std::less{}, &Entry::value)};
}
return {this, index_iterator};
return {this, index_container_->begin()};
}
LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_accessor_->end()}; }
LabelPropertyIndex::Iterable::Iterator LabelPropertyIndex::Iterable::end() { return {this, index_container_->end()}; }
int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property, const PropertyValue &value) const {
auto it = index_.find({label, property});
MG_ASSERT(it != index_.end(), "Index for label {} and property {} doesn't exist", label.AsUint(), property.AsUint());
if (!value.IsNull()) {
return static_cast<int64_t>(
std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; }));
}
// TODO Do check this
MG_ASSERT(!value.IsNull(), "Null is not supported!");
auto start_it = std::ranges::lower_bound(it->second, value, std::less{}, &Entry::value);
return static_cast<int64_t>(
std::ranges::count_if(it->second, [&value](const auto &elem) { return elem.value == value; }));
std::ranges::count_if(start_it, it->second.end(), [&value](const auto &elem) { return elem.value == value; }));
}
int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property,
@ -675,11 +668,9 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property,
[&index = it->second](const auto value, const auto def) {
if (value) {
if (value->IsInclusive()) {
return std::lower_bound(index.begin(), index.end(), value->value(),
[](const Entry &elem, const PropertyValue &val) { return elem.value < val; });
return std::ranges::lower_bound(index, value->value(), std::less{}, &Entry::value);
}
return std::upper_bound(index.begin(), index.end(), value->value(),
[](const PropertyValue &val, const Entry &elem) { return val < elem.value; });
return std::ranges::upper_bound(index, value->value(), std::less{}, &Entry::value);
}
return def;
},
@ -688,11 +679,9 @@ int64_t LabelPropertyIndex::VertexCount(LabelId label, PropertyId property,
[&index = it->second](const auto value, const auto def) {
if (value) {
if (value->IsInclusive()) {
return std::upper_bound(index.begin(), index.end(), value->value(),
[](const PropertyValue &val, const Entry &elem) { return val < elem.value; });
return std::ranges::upper_bound(index, value->value(), std::less{}, &Entry::value);
}
return std::lower_bound(index.begin(), index.end(), value->value(),
[](const Entry &elem, const PropertyValue &val) { return elem.value < val; });
return std::ranges::lower_bound(index, value->value(), std::less{}, &Entry::value);
}
return def;
},

12
src/storage/v3/indices.hpp
View File

@ -63,7 +63,7 @@ class LabelIndex {
class Iterable {
public:
Iterable(LabelIndexContainer &index_accessor, LabelId label, View view, Transaction *transaction, Indices *indices,
Iterable(LabelIndexContainer &index_container, LabelId label, View view, Transaction *transaction, Indices *indices,
Config::Items config, const VertexValidator &vertex_validator);
class Iterator {
@ -86,11 +86,11 @@ class LabelIndex {
Vertex *current_vertex_;
};
Iterator begin() { return {this, index_accessor_->begin()}; }
Iterator end() { return {this, index_accessor_->end()}; }
Iterator begin() { return {this, index_container_->begin()}; }
Iterator end() { return {this, index_container_->end()}; }
private:
LabelIndexContainer *index_accessor_;
LabelIndexContainer *index_container_;
LabelId label_;
View view_;
Transaction *transaction_;
@ -159,7 +159,7 @@ class LabelPropertyIndex {
class Iterable {
public:
Iterable(LabelPropertyIndexContainer &index_accessor, LabelId label, PropertyId property,
Iterable(LabelPropertyIndexContainer &index_container, LabelId label, PropertyId property,
const std::optional<utils::Bound<PropertyValue>> &lower_bound,
const std::optional<utils::Bound<PropertyValue>> &upper_bound, View view, Transaction *transaction,
Indices *indices, Config::Items config, const VertexValidator &vertex_validator);
@ -188,7 +188,7 @@ class LabelPropertyIndex {
Iterator end();
private:
LabelPropertyIndexContainer *index_accessor_;
LabelPropertyIndexContainer *index_container_;
LabelId label_;
PropertyId property_;
std::optional<utils::Bound<PropertyValue>> lower_bound_;

21
src/storage/v3/mvcc.hpp
View File

@ -12,6 +12,7 @@
#pragma once
#include <type_traits>
#include "storage/v3/edge.hpp"
#include "storage/v3/property_value.hpp"
#include "storage/v3/transaction.hpp"
@ -21,6 +22,10 @@
namespace memgraph::storage::v3 {
inline VertexData *GetDeltaHolder(Vertex *vertex) { return &vertex->second; }
inline Edge *GetDeltaHolder(Edge *edge) { return edge; }
/// This function iterates through the undo buffers from an object (starting
/// from the supplied delta) and determines what deltas should be applied to get
/// the currently visible version of the object. When the function finds a delta
@ -83,13 +88,7 @@ inline void ApplyDeltasForRead(Transaction *transaction, const Delta *delta, Vie
template <typename TObj>
requires utils::SameAsAnyOf<TObj, Edge, Vertex>
inline bool PrepareForWrite(Transaction *transaction, TObj *object) {
auto *delta_holder = std::invoke([object]() -> auto * {
if constexpr (std::is_same_v<TObj, Vertex>) {
return &object->second;
} else {
return object;
}
});
auto *delta_holder = GetDeltaHolder(object);
if (delta_holder->delta == nullptr) return true;
const auto &delta_commit_info = *delta_holder->delta->commit_info;
@ -121,13 +120,7 @@ requires utils::SameAsAnyOf<TObj, Edge, Vertex>
inline void CreateAndLinkDelta(Transaction *transaction, TObj *object, Args &&...args) {
auto delta = &transaction->deltas.emplace_back(std::forward<Args>(args)..., transaction->commit_info.get(),
transaction->command_id);
auto *delta_holder = std::invoke([object]() -> auto * {
if constexpr (std::is_same_v<TObj, Vertex>) {
return &object->second;
} else {
return object;
}
});
auto *delta_holder = GetDeltaHolder(object);
// The operations are written in such order so that both `next` and `prev`
// chains are valid at all times. The chains must be valid at all times

30
src/storage/v3/property_value.hpp
View File

@ -330,36 +330,6 @@ inline bool operator<(const PropertyValue &first, const PropertyValue &second) {
}
}
inline bool operator>=(const PropertyValue &first, const PropertyValue &second) {
if (!PropertyValue::AreComparableTypes(first.type(), second.type())) return first.type() >= second.type();
switch (first.type()) {
case PropertyValue::Type::Null:
return false;
case PropertyValue::Type::Bool:
return first.ValueBool() >= second.ValueBool();
case PropertyValue::Type::Int:
if (second.type() == PropertyValue::Type::Double) {
return static_cast<double>(first.ValueInt()) >= second.ValueDouble();
} else {
return first.ValueInt() >= second.ValueInt();
}
case PropertyValue::Type::Double:
if (second.type() == PropertyValue::Type::Double) {
return first.ValueDouble() >= second.ValueDouble();
} else {
return first.ValueDouble() >= static_cast<double>(second.ValueInt());
}
case PropertyValue::Type::String:
return first.ValueString() >= second.ValueString();
case PropertyValue::Type::List:
return first.ValueList() >= second.ValueList();
case PropertyValue::Type::Map:
return first.ValueMap() >= second.ValueMap();
case PropertyValue::Type::TemporalData:
return first.ValueTemporalData() >= second.ValueTemporalData();
}
}
inline PropertyValue::PropertyValue(const PropertyValue &other) : type_(other.type_) {
switch (other.type_) {
case Type::Null:

2
src/storage/v3/shard.cpp
View File

@ -1017,7 +1017,7 @@ void Shard::CollectGarbage(const io::Time current_time) {
RemoveObsoleteEntries(&indices_, clean_up_before_timestamp);
}
for (const auto &vertex : deleted_vertices_) {
for (const auto *vertex : deleted_vertices_) {
MG_ASSERT(vertices_.erase(*vertex), "Invalid database state!");
}
deleted_vertices_.clear();