From a282542666db13910b897ea2b6b1fdec39a09dc2 Mon Sep 17 00:00:00 2001 From: Gareth Andrew Lloyd Date: Tue, 12 Mar 2024 00:26:11 +0000 Subject: [PATCH] Optimise ORDER BY, RANGE, UNWIND (#1781) * Optimise frame change * Optimise distinct + orderby memory usage - dispose collections as earlier as possible - move values rather than copy * Better perf, ORDER BY * Optimise RANGE and UNWIND * ConstraintVerificationInfo only if at least one constraint * Optimise TypeValue * Clang-tidy fix --- src/query/common.cpp | 60 +------- src/query/common.hpp | 128 ++++++++++++++---- src/query/frame_change.hpp | 7 +- .../interpret/awesome_memgraph_functions.cpp | 6 + src/query/plan/operator.cpp | 73 ++++++---- src/query/plan/pretty_print.cpp | 2 +- src/query/typed_value.cpp | 40 +++--- src/query/typed_value.hpp | 71 +++++----- .../replication_handler.cpp | 2 +- src/storage/v2/constraints/constraints.hpp | 3 +- .../v2/constraints/existence_constraints.hpp | 4 +- .../v2/constraints/unique_constraints.hpp | 4 +- src/storage/v2/disk/storage.cpp | 3 +- src/storage/v2/disk/unique_constraints.cpp | 3 +- src/storage/v2/disk/unique_constraints.hpp | 3 +- src/storage/v2/inmemory/storage.cpp | 17 ++- .../v2/inmemory/unique_constraints.cpp | 1 + .../v2/inmemory/unique_constraints.hpp | 3 + src/storage/v2/transaction.hpp | 6 +- src/storage/v2/vertex_accessor.cpp | 36 +++-- tests/unit/storage_rocks.cpp | 2 - tests/unit/storage_v2_wal_file.cpp | 2 +- 22 files changed, 270 insertions(+), 206 deletions(-) diff --git a/src/query/common.cpp b/src/query/common.cpp index 3c75ed5ec..94a8d8cdf 100644 --- a/src/query/common.cpp +++ b/src/query/common.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -13,64 +13,6 @@ namespace memgraph::query { -namespace impl { - -bool TypedValueCompare(const TypedValue &a, const TypedValue &b) { - // in ordering null comes after everything else - // at the same time Null is not less that null - // first deal with Null < Whatever case - if (a.IsNull()) return false; - // now deal with NotNull < Null case - if (b.IsNull()) return true; - - // comparisons are from this point legal only between values of - // the same type, or int+float combinations - if ((a.type() != b.type() && !(a.IsNumeric() && b.IsNumeric()))) - throw QueryRuntimeException("Can't compare value of type {} to value of type {}.", a.type(), b.type()); - - switch (a.type()) { - case TypedValue::Type::Bool: - return !a.ValueBool() && b.ValueBool(); - case TypedValue::Type::Int: - if (b.type() == TypedValue::Type::Double) - return a.ValueInt() < b.ValueDouble(); - else - return a.ValueInt() < b.ValueInt(); - case TypedValue::Type::Double: - if (b.type() == TypedValue::Type::Int) - return a.ValueDouble() < b.ValueInt(); - else - return a.ValueDouble() < b.ValueDouble(); - case TypedValue::Type::String: - // NOLINTNEXTLINE(modernize-use-nullptr) - return a.ValueString() < b.ValueString(); - case TypedValue::Type::Date: - // NOLINTNEXTLINE(modernize-use-nullptr) - return a.ValueDate() < b.ValueDate(); - case TypedValue::Type::LocalTime: - // NOLINTNEXTLINE(modernize-use-nullptr) - return a.ValueLocalTime() < b.ValueLocalTime(); - case TypedValue::Type::LocalDateTime: - // NOLINTNEXTLINE(modernize-use-nullptr) - return a.ValueLocalDateTime() < b.ValueLocalDateTime(); - case TypedValue::Type::Duration: - // NOLINTNEXTLINE(modernize-use-nullptr) - return a.ValueDuration() < b.ValueDuration(); - case TypedValue::Type::List: - case TypedValue::Type::Map: - case TypedValue::Type::Vertex: - case TypedValue::Type::Edge: - case TypedValue::Type::Path: - case TypedValue::Type::Graph: - case TypedValue::Type::Function: - throw QueryRuntimeException("Comparison is not defined for values of type {}.", a.type()); - case TypedValue::Type::Null: - LOG_FATAL("Invalid type"); - } -} - -} // namespace impl - int64_t QueryTimestamp() { return std::chrono::duration_cast(std::chrono::system_clock::now().time_since_epoch()) .count(); diff --git a/src/query/common.hpp b/src/query/common.hpp index 36ba07791..9f4e01cc0 100644 --- a/src/query/common.hpp +++ b/src/query/common.hpp @@ -23,6 +23,7 @@ #include "query/frontend/ast/ast.hpp" #include "query/frontend/semantic/symbol.hpp" #include "query/typed_value.hpp" +#include "range/v3/all.hpp" #include "storage/v2/id_types.hpp" #include "storage/v2/property_value.hpp" #include "storage/v2/result.hpp" @@ -31,9 +32,91 @@ namespace memgraph::query { -namespace impl { -bool TypedValueCompare(const TypedValue &a, const TypedValue &b); -} // namespace impl +namespace { +std::partial_ordering TypedValueCompare(TypedValue const &a, TypedValue const &b) { + // First assume typical same type comparisons + if (a.type() == b.type()) { + switch (a.type()) { + case TypedValue::Type::Bool: + return a.UnsafeValueBool() <=> b.UnsafeValueBool(); + case TypedValue::Type::Int: + return a.UnsafeValueInt() <=> b.UnsafeValueInt(); + case TypedValue::Type::Double: + return a.UnsafeValueDouble() <=> b.UnsafeValueDouble(); + case TypedValue::Type::String: + return a.UnsafeValueString() <=> b.UnsafeValueString(); + case TypedValue::Type::Date: + return a.UnsafeValueDate() <=> b.UnsafeValueDate(); + case TypedValue::Type::LocalTime: + return a.UnsafeValueLocalTime() <=> b.UnsafeValueLocalTime(); + case TypedValue::Type::LocalDateTime: + return a.UnsafeValueLocalDateTime() <=> b.UnsafeValueLocalDateTime(); + case TypedValue::Type::Duration: + return a.UnsafeValueDuration() <=> b.UnsafeValueDuration(); + case TypedValue::Type::Null: + return std::partial_ordering::equivalent; + case TypedValue::Type::List: + case TypedValue::Type::Map: + case TypedValue::Type::Vertex: + case TypedValue::Type::Edge: + case TypedValue::Type::Path: + case TypedValue::Type::Graph: + case TypedValue::Type::Function: + throw QueryRuntimeException("Comparison is not defined for values of type {}.", a.type()); + } + } else { + // from this point legal only between values of + // int+float combinations or against null + + // in ordering null comes after everything else + // at the same time Null is not less that null + // first deal with Null < Whatever case + if (a.IsNull()) return std::partial_ordering::greater; + // now deal with NotNull < Null case + if (b.IsNull()) return std::partial_ordering::less; + + if (!(a.IsNumeric() && b.IsNumeric())) [[unlikely]] + throw QueryRuntimeException("Can't compare value of type {} to value of type {}.", a.type(), b.type()); + + switch (a.type()) { + case TypedValue::Type::Int: + return a.UnsafeValueInt() <=> b.ValueDouble(); + case TypedValue::Type::Double: + return a.UnsafeValueDouble() <=> b.ValueInt(); + case TypedValue::Type::Bool: + case TypedValue::Type::Null: + case TypedValue::Type::String: + case TypedValue::Type::List: + case TypedValue::Type::Map: + case TypedValue::Type::Vertex: + case TypedValue::Type::Edge: + case TypedValue::Type::Path: + case TypedValue::Type::Date: + case TypedValue::Type::LocalTime: + case TypedValue::Type::LocalDateTime: + case TypedValue::Type::Duration: + case TypedValue::Type::Graph: + case TypedValue::Type::Function: + LOG_FATAL("Invalid type"); + } + } +} + +} // namespace + +struct OrderedTypedValueCompare { + OrderedTypedValueCompare(Ordering ordering) : ordering_{ordering}, ascending{ordering == Ordering::ASC} {} + + auto operator()(const TypedValue &lhs, const TypedValue &rhs) const -> std::partial_ordering { + return ascending ? TypedValueCompare(lhs, rhs) : TypedValueCompare(rhs, lhs); + } + + auto ordering() const { return ordering_; } + + private: + Ordering ordering_; + bool ascending = true; +}; /// Custom Comparator type for comparing vectors of TypedValues. /// @@ -43,32 +126,27 @@ bool TypedValueCompare(const TypedValue &a, const TypedValue &b); class TypedValueVectorCompare final { public: TypedValueVectorCompare() = default; - explicit TypedValueVectorCompare(const std::vector &ordering) : ordering_(ordering) {} + explicit TypedValueVectorCompare(std::vector orderings) + : orderings_{std::move(orderings)} {} - template - bool operator()(const std::vector &c1, const std::vector &c2) const { - // ordering is invalid if there are more elements in the collections - // then there are in the ordering_ vector - MG_ASSERT(c1.size() <= ordering_.size() && c2.size() <= ordering_.size(), - "Collections contain more elements then there are orderings"); + const auto &orderings() const { return orderings_; } - auto c1_it = c1.begin(); - auto c2_it = c2.begin(); - auto ordering_it = ordering_.begin(); - for (; c1_it != c1.end() && c2_it != c2.end(); c1_it++, c2_it++, ordering_it++) { - if (impl::TypedValueCompare(*c1_it, *c2_it)) return *ordering_it == Ordering::ASC; - if (impl::TypedValueCompare(*c2_it, *c1_it)) return *ordering_it == Ordering::DESC; - } - - // at least one collection is exhausted - // c1 is less then c2 iff c1 reached the end but c2 didn't - return (c1_it == c1.end()) && (c2_it != c2.end()); + auto lex_cmp() const { + return [orderings = &orderings_](const std::vector &lhs, + const std::vector &rhs) { + auto rng = ranges::views::zip(*orderings, lhs, rhs); + for (auto const &[cmp, l, r] : rng) { + auto res = cmp(l, r); + if (res == std::partial_ordering::less) return true; + if (res == std::partial_ordering::greater) return false; + } + DMG_ASSERT(orderings->size() == lhs.size() && lhs.size() == rhs.size()); + return false; + }; } - // TODO: Remove this, member is public - const auto &ordering() const { return ordering_; } - - std::vector ordering_; + private: + std::vector orderings_; }; /// Raise QueryRuntimeException if the value for symbol isn't of expected type. diff --git a/src/query/frame_change.hpp b/src/query/frame_change.hpp index 7baf1fe41..f51185722 100644 --- a/src/query/frame_change.hpp +++ b/src/query/frame_change.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -126,10 +126,11 @@ class FrameChangeCollector { } bool ResetTrackingValue(const std::string &key) { - if (!tracked_values_.contains(utils::pmr::string(key, utils::NewDeleteResource()))) { + auto const it = tracked_values_.find(utils::pmr::string(key, utils::NewDeleteResource())); + if (it == tracked_values_.cend()) { return false; } - tracked_values_.erase(utils::pmr::string(key, utils::NewDeleteResource())); + tracked_values_.erase(it); AddTrackingKey(key); return true; } diff --git a/src/query/interpret/awesome_memgraph_functions.cpp b/src/query/interpret/awesome_memgraph_functions.cpp index 6be8c4837..a9381f92a 100644 --- a/src/query/interpret/awesome_memgraph_functions.cpp +++ b/src/query/interpret/awesome_memgraph_functions.cpp @@ -761,13 +761,19 @@ TypedValue Range(const TypedValue *args, int64_t nargs, const FunctionContext &c int64_t step = nargs == 3 ? args[2].ValueInt() : 1; TypedValue::TVector list(ctx.memory); if (lbound <= rbound && step > 0) { + int64_t n = ((rbound - lbound + 1) + (step - 1)) / step; + list.reserve(n); for (auto i = lbound; i <= rbound; i += step) { list.emplace_back(i); } + MG_ASSERT(list.size() == n); } else if (lbound >= rbound && step < 0) { + int64_t n = ((lbound - rbound + 1) + (-step - 1)) / -step; + list.reserve(n); for (auto i = lbound; i >= rbound; i += step) { list.emplace_back(i); } + MG_ASSERT(list.size() == n); } return TypedValue(std::move(list)); } diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 7cd506050..bf2194641 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -47,6 +47,7 @@ #include "query/procedure/mg_procedure_impl.hpp" #include "query/procedure/module.hpp" #include "query/typed_value.hpp" +#include "range/v3/all.hpp" #include "storage/v2/property_value.hpp" #include "storage/v2/view.hpp" #include "utils/algorithm.hpp" @@ -4147,14 +4148,14 @@ OrderBy::OrderBy(const std::shared_ptr &input, const std::vecto const std::vector &output_symbols) : input_(input), output_symbols_(output_symbols) { // split the order_by vector into two vectors of orderings and expressions - std::vector ordering; + std::vector ordering; ordering.reserve(order_by.size()); order_by_.reserve(order_by.size()); for (const auto &ordering_expression_pair : order_by) { ordering.emplace_back(ordering_expression_pair.ordering); order_by_.emplace_back(ordering_expression_pair.expression); } - compare_ = TypedValueVectorCompare(ordering); + compare_ = TypedValueVectorCompare(std::move(ordering)); } ACCEPT_WITH_INPUT(OrderBy) @@ -4175,29 +4176,43 @@ class OrderByCursor : public Cursor { OOMExceptionEnabler oom_exception; SCOPED_PROFILE_OP_BY_REF(self_); - if (!did_pull_all_) { + if (!did_pull_all_) [[unlikely]] { ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, storage::View::OLD); - auto *mem = cache_.get_allocator().GetMemoryResource(); + auto *pull_mem = context.evaluation_context.memory; + auto *query_mem = cache_.get_allocator().GetMemoryResource(); + + utils::pmr::vector> order_by(pull_mem); // Not cached, pull memory + utils::pmr::vector> output(query_mem); // Cached, query memory + while (input_cursor_->Pull(frame, context)) { // collect the order_by elements - utils::pmr::vector order_by(mem); - order_by.reserve(self_.order_by_.size()); - for (auto expression_ptr : self_.order_by_) { - order_by.emplace_back(expression_ptr->Accept(evaluator)); + utils::pmr::vector order_by_elem(pull_mem); + order_by_elem.reserve(self_.order_by_.size()); + for (auto const &expression_ptr : self_.order_by_) { + order_by_elem.emplace_back(expression_ptr->Accept(evaluator)); } + order_by.emplace_back(std::move(order_by_elem)); // collect the output elements - utils::pmr::vector output(mem); - output.reserve(self_.output_symbols_.size()); - for (const Symbol &output_sym : self_.output_symbols_) output.emplace_back(frame[output_sym]); - - cache_.push_back(Element{std::move(order_by), std::move(output)}); + utils::pmr::vector output_elem(query_mem); + output_elem.reserve(self_.output_symbols_.size()); + for (const Symbol &output_sym : self_.output_symbols_) { + output_elem.emplace_back(frame[output_sym]); + } + output.emplace_back(std::move(output_elem)); } - std::sort(cache_.begin(), cache_.end(), [this](const auto &pair1, const auto &pair2) { - return self_.compare_(pair1.order_by, pair2.order_by); - }); + // sorting with range zip + // we compare on just the projection of the 1st range (order_by) + // this will also permute the 2nd range (output) + ranges::sort( + ranges::views::zip(order_by, output), self_.compare_.lex_cmp(), + [](auto const &value) -> auto const & { return std::get<0>(value); }); + + // no longer need the order_by terms + order_by.clear(); + cache_ = std::move(output); did_pull_all_ = true; cache_it_ = cache_.begin(); @@ -4208,15 +4223,15 @@ class OrderByCursor : public Cursor { AbortCheck(context); // place the output values on the frame - DMG_ASSERT(self_.output_symbols_.size() == cache_it_->remember.size(), + DMG_ASSERT(self_.output_symbols_.size() == cache_it_->size(), "Number of values does not match the number of output symbols " "in OrderBy"); auto output_sym_it = self_.output_symbols_.begin(); - for (const TypedValue &output : cache_it_->remember) { - if (context.frame_change_collector && context.frame_change_collector->IsKeyTracked(output_sym_it->name())) { + for (TypedValue &output : *cache_it_) { + if (context.frame_change_collector) { context.frame_change_collector->ResetTrackingValue(output_sym_it->name()); } - frame[*output_sym_it++] = output; + frame[*output_sym_it++] = std::move(output); } cache_it_++; return true; @@ -4231,17 +4246,12 @@ class OrderByCursor : public Cursor { } private: - struct Element { - utils::pmr::vector order_by; - utils::pmr::vector remember; - }; - const OrderBy &self_; const UniqueCursorPtr input_cursor_; bool did_pull_all_{false}; // a cache of elements pulled from the input - // the cache is filled and sorted (only on first elem) on first Pull - utils::pmr::vector cache_; + // the cache is filled and sorted on first Pull + utils::pmr::vector> cache_; // iterator over the cache_, maintains state between Pulls decltype(cache_.begin()) cache_it_ = cache_.begin(); }; @@ -4445,6 +4455,7 @@ class UnwindCursor : public Cursor { if (input_value.type() != TypedValue::Type::List) throw QueryRuntimeException("Argument of UNWIND must be a list, but '{}' was provided.", input_value.type()); // Copy the evaluted input_value_list to our vector. + // eval memory != query memory input_value_ = input_value.ValueList(); input_value_it_ = input_value_.begin(); } @@ -4452,7 +4463,7 @@ class UnwindCursor : public Cursor { // if we reached the end of our list of values goto back to top if (input_value_it_ == input_value_.end()) continue; - frame[self_.output_symbol_] = *input_value_it_++; + frame[self_.output_symbol_] = std::move(*input_value_it_++); if (context.frame_change_collector && context.frame_change_collector->IsKeyTracked(self_.output_symbol_.name_)) { context.frame_change_collector->ResetTrackingValue(self_.output_symbol_.name_); } @@ -4493,7 +4504,11 @@ class DistinctCursor : public Cursor { SCOPED_PROFILE_OP("Distinct"); while (true) { - if (!input_cursor_->Pull(frame, context)) return false; + if (!input_cursor_->Pull(frame, context)) { + // Nothing left to pull, we can dispose of seen_rows now + seen_rows_.clear(); + return false; + } utils::pmr::vector row(seen_rows_.get_allocator().GetMemoryResource()); row.reserve(self_.value_symbols_.size()); diff --git a/src/query/plan/pretty_print.cpp b/src/query/plan/pretty_print.cpp index eeb0c15b5..2e6fad9d0 100644 --- a/src/query/plan/pretty_print.cpp +++ b/src/query/plan/pretty_print.cpp @@ -769,7 +769,7 @@ bool PlanToJsonVisitor::PreVisit(OrderBy &op) { for (auto i = 0; i < op.order_by_.size(); ++i) { json json; - json["ordering"] = ToString(op.compare_.ordering_[i]); + json["ordering"] = ToString(op.compare_.orderings()[i].ordering()); json["expression"] = ToJson(op.order_by_[i]); self["order_by"].push_back(json); } diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index 86d25f01b..059e1b1ba 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -321,6 +321,20 @@ TypedValue::operator storage::PropertyValue() const { throw TypedValueException("Unsupported conversion from TypedValue to PropertyValue"); } +// NOLINTNEXTLINE(cppcoreguidelines-macro-usage) +#define DEFINE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(type_param, type_enum, field) \ + type_param &TypedValue::Value##type_enum() { \ + if (type_ != Type::type_enum) [[unlikely]] \ + throw TypedValueException("TypedValue is of type '{}', not '{}'", type_, Type::type_enum); \ + return field; \ + } \ + type_param TypedValue::Value##type_enum() const { \ + if (type_ != Type::type_enum) [[unlikely]] \ + throw TypedValueException("TypedValue is of type '{}', not '{}'", type_, Type::type_enum); \ + return field; \ + } \ + bool TypedValue::Is##type_enum() const { return type_ == Type::type_enum; } + #define DEFINE_VALUE_AND_TYPE_GETTERS(type_param, type_enum, field) \ type_param &TypedValue::Value##type_enum() { \ if (type_ != Type::type_enum) [[unlikely]] \ @@ -334,9 +348,9 @@ TypedValue::operator storage::PropertyValue() const { } \ bool TypedValue::Is##type_enum() const { return type_ == Type::type_enum; } -DEFINE_VALUE_AND_TYPE_GETTERS(bool, Bool, bool_v) -DEFINE_VALUE_AND_TYPE_GETTERS(int64_t, Int, int_v) -DEFINE_VALUE_AND_TYPE_GETTERS(double, Double, double_v) +DEFINE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(bool, Bool, bool_v) +DEFINE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(int64_t, Int, int_v) +DEFINE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(double, Double, double_v) DEFINE_VALUE_AND_TYPE_GETTERS(TypedValue::TString, String, string_v) DEFINE_VALUE_AND_TYPE_GETTERS(TypedValue::TVector, List, list_v) DEFINE_VALUE_AND_TYPE_GETTERS(TypedValue::TMap, Map, map_v) @@ -348,24 +362,10 @@ DEFINE_VALUE_AND_TYPE_GETTERS(utils::LocalTime, LocalTime, local_time_v) DEFINE_VALUE_AND_TYPE_GETTERS(utils::LocalDateTime, LocalDateTime, local_date_time_v) DEFINE_VALUE_AND_TYPE_GETTERS(utils::Duration, Duration, duration_v) DEFINE_VALUE_AND_TYPE_GETTERS(std::function, Function, function_v) - -Graph &TypedValue::ValueGraph() { - if (type_ != Type::Graph) { - throw TypedValueException("TypedValue is of type '{}', not '{}'", type_, Type::Graph); - } - return *graph_v; -} - -const Graph &TypedValue::ValueGraph() const { - if (type_ != Type::Graph) { - throw TypedValueException("TypedValue is of type '{}', not '{}'", type_, Type::Graph); - } - return *graph_v; -} - -bool TypedValue::IsGraph() const { return type_ == Type::Graph; } +DEFINE_VALUE_AND_TYPE_GETTERS(Graph, Graph, *graph_v) #undef DEFINE_VALUE_AND_TYPE_GETTERS +#undef DEFINE_VALUE_AND_TYPE_GETTERS_PRIMITIVE bool TypedValue::ContainsDeleted() const { switch (type_) { @@ -399,8 +399,6 @@ bool TypedValue::ContainsDeleted() const { return false; } -bool TypedValue::IsNull() const { return type_ == Type::Null; } - bool TypedValue::IsNumeric() const { return IsInt() || IsDouble(); } bool TypedValue::IsPropertyValue() const { diff --git a/src/query/typed_value.hpp b/src/query/typed_value.hpp index a1353869a..9b9346a1c 100644 --- a/src/query/typed_value.hpp +++ b/src/query/typed_value.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -475,50 +475,51 @@ class TypedValue { Type type() const { return type_; } - // TODO consider adding getters for primitives by value (and not by ref) +#define DECLARE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(type_param, type_enum, field) \ + /** Gets the value of type field. Throws if value is not field*/ \ + type_param &Value##type_enum(); \ + /** Gets the value of type field. Throws if value is not field*/ \ + type_param Value##type_enum() const; \ + /** Checks if it's the value is of the given type */ \ + bool Is##type_enum() const; \ + /** Get the value of the type field. Unchecked */ \ + type_param UnsafeValue##type_enum() const { return field; } -#define DECLARE_VALUE_AND_TYPE_GETTERS(type_param, field) \ - /** Gets the value of type field. Throws if value is not field*/ \ - type_param &Value##field(); \ - /** Gets the value of type field. Throws if value is not field*/ \ - const type_param &Value##field() const; \ - /** Checks if it's the value is of the given type */ \ - bool Is##field() const; +#define DECLARE_VALUE_AND_TYPE_GETTERS(type_param, type_enum, field) \ + /** Gets the value of type field. Throws if value is not field*/ \ + type_param &Value##type_enum(); \ + /** Gets the value of type field. Throws if value is not field*/ \ + const type_param &Value##type_enum() const; \ + /** Checks if it's the value is of the given type */ \ + bool Is##type_enum() const; \ + /** Get the value of the type field. Unchecked */ \ + type_param const &UnsafeValue##type_enum() const { return field; } - DECLARE_VALUE_AND_TYPE_GETTERS(bool, Bool) - DECLARE_VALUE_AND_TYPE_GETTERS(int64_t, Int) - DECLARE_VALUE_AND_TYPE_GETTERS(double, Double) - DECLARE_VALUE_AND_TYPE_GETTERS(TString, String) + DECLARE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(bool, Bool, bool_v) + DECLARE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(int64_t, Int, int_v) + DECLARE_VALUE_AND_TYPE_GETTERS_PRIMITIVE(double, Double, double_v) + DECLARE_VALUE_AND_TYPE_GETTERS(TString, String, string_v) - /** - * Get the list value. - * @throw TypedValueException if stored value is not a list. - */ - TVector &ValueList(); + DECLARE_VALUE_AND_TYPE_GETTERS(TVector, List, list_v) + DECLARE_VALUE_AND_TYPE_GETTERS(TMap, Map, map_v) + DECLARE_VALUE_AND_TYPE_GETTERS(VertexAccessor, Vertex, vertex_v) + DECLARE_VALUE_AND_TYPE_GETTERS(EdgeAccessor, Edge, edge_v) + DECLARE_VALUE_AND_TYPE_GETTERS(Path, Path, path_v) - const TVector &ValueList() const; - - /** Check if the stored value is a list value */ - bool IsList() const; - - DECLARE_VALUE_AND_TYPE_GETTERS(TMap, Map) - DECLARE_VALUE_AND_TYPE_GETTERS(VertexAccessor, Vertex) - DECLARE_VALUE_AND_TYPE_GETTERS(EdgeAccessor, Edge) - DECLARE_VALUE_AND_TYPE_GETTERS(Path, Path) - - DECLARE_VALUE_AND_TYPE_GETTERS(utils::Date, Date) - DECLARE_VALUE_AND_TYPE_GETTERS(utils::LocalTime, LocalTime) - DECLARE_VALUE_AND_TYPE_GETTERS(utils::LocalDateTime, LocalDateTime) - DECLARE_VALUE_AND_TYPE_GETTERS(utils::Duration, Duration) - DECLARE_VALUE_AND_TYPE_GETTERS(Graph, Graph) - DECLARE_VALUE_AND_TYPE_GETTERS(std::function, Function) + DECLARE_VALUE_AND_TYPE_GETTERS(utils::Date, Date, date_v) + DECLARE_VALUE_AND_TYPE_GETTERS(utils::LocalTime, LocalTime, local_time_v) + DECLARE_VALUE_AND_TYPE_GETTERS(utils::LocalDateTime, LocalDateTime, local_date_time_v) + DECLARE_VALUE_AND_TYPE_GETTERS(utils::Duration, Duration, duration_v) + DECLARE_VALUE_AND_TYPE_GETTERS(Graph, Graph, *graph_v) + DECLARE_VALUE_AND_TYPE_GETTERS(std::function, Function, function_v) #undef DECLARE_VALUE_AND_TYPE_GETTERS +#undef DECLARE_VALUE_AND_TYPE_GETTERS_PRIMITIVE bool ContainsDeleted() const; /** Checks if value is a TypedValue::Null. */ - bool IsNull() const; + bool IsNull() const { return type_ == Type::Null; } /** Convenience function for checking if this TypedValue is either * an integer or double */ diff --git a/src/replication_handler/replication_handler.cpp b/src/replication_handler/replication_handler.cpp index 34ccdfc99..4ae4c796e 100644 --- a/src/replication_handler/replication_handler.cpp +++ b/src/replication_handler/replication_handler.cpp @@ -310,7 +310,7 @@ auto ReplicationHandler::ShowReplicas() const -> utils::BasicResultstorage_mode_ != storage::StorageMode::IN_MEMORY_TRANSACTIONAL) return; if (!full_info && storage->name() == dbms::kDefaultDB) return; - auto ok = + [[maybe_unused]] auto ok = storage->repl_storage_state_.WithClient(replica.name_, [&](storage::ReplicationStorageClient &client) { auto ts_info = client.GetTimestampInfo(storage); auto state = client.State(); diff --git a/src/storage/v2/constraints/constraints.hpp b/src/storage/v2/constraints/constraints.hpp index 1f5ef999e..0d2a49875 100644 --- a/src/storage/v2/constraints/constraints.hpp +++ b/src/storage/v2/constraints/constraints.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -33,6 +33,7 @@ struct Constraints { std::unique_ptr existence_constraints_; std::unique_ptr unique_constraints_; + bool empty() const { return existence_constraints_->empty() && unique_constraints_->empty(); } }; } // namespace memgraph::storage diff --git a/src/storage/v2/constraints/existence_constraints.hpp b/src/storage/v2/constraints/existence_constraints.hpp index c3b68828a..a043a9f5b 100644 --- a/src/storage/v2/constraints/existence_constraints.hpp +++ b/src/storage/v2/constraints/existence_constraints.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -40,6 +40,8 @@ class ExistenceConstraints { const LabelId &label, const PropertyId &property); }; + bool empty() const { return constraints_.empty(); } + [[nodiscard]] static std::optional ValidateVertexOnConstraint(const Vertex &vertex, const LabelId &label, const PropertyId &property); diff --git a/src/storage/v2/constraints/unique_constraints.hpp b/src/storage/v2/constraints/unique_constraints.hpp index b9ec04bfc..fcdcf1739 100644 --- a/src/storage/v2/constraints/unique_constraints.hpp +++ b/src/storage/v2/constraints/unique_constraints.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -60,6 +60,8 @@ class UniqueConstraints { virtual void Clear() = 0; + virtual bool empty() const = 0; + protected: static DeletionStatus CheckPropertiesBeforeDeletion(const std::set &properties) { if (properties.empty()) { diff --git a/src/storage/v2/disk/storage.cpp b/src/storage/v2/disk/storage.cpp index 21ae7755e..21fa5ecc7 100644 --- a/src/storage/v2/disk/storage.cpp +++ b/src/storage/v2/disk/storage.cpp @@ -2049,7 +2049,8 @@ Transaction DiskStorage::CreateTransaction(IsolationLevel isolation_level, Stora edge_import_mode_active = edge_import_status_ == EdgeImportMode::ACTIVE; } - return {transaction_id, start_timestamp, isolation_level, storage_mode, edge_import_mode_active}; + return {transaction_id, start_timestamp, isolation_level, + storage_mode, edge_import_mode_active, !constraints_.empty()}; } uint64_t DiskStorage::CommitTimestamp(const std::optional desired_commit_timestamp) { diff --git a/src/storage/v2/disk/unique_constraints.cpp b/src/storage/v2/disk/unique_constraints.cpp index 3c17530c2..04a0c265a 100644 --- a/src/storage/v2/disk/unique_constraints.cpp +++ b/src/storage/v2/disk/unique_constraints.cpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -347,5 +347,6 @@ void DiskUniqueConstraints::LoadUniqueConstraints(const std::vector constraints_.emplace(label, properties); } } +bool DiskUniqueConstraints::empty() const { return constraints_.empty(); } } // namespace memgraph::storage diff --git a/src/storage/v2/disk/unique_constraints.hpp b/src/storage/v2/disk/unique_constraints.hpp index 0cc5a9586..4e3450ef1 100644 --- a/src/storage/v2/disk/unique_constraints.hpp +++ b/src/storage/v2/disk/unique_constraints.hpp @@ -1,4 +1,4 @@ -// Copyright 2023 Memgraph Ltd. +// Copyright 2024 Memgraph Ltd. // // Use of this software is governed by the Business Source License // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source @@ -59,6 +59,7 @@ class DiskUniqueConstraints : public UniqueConstraints { RocksDBStorage *GetRocksDBStorage() const; void LoadUniqueConstraints(const std::vector &keys); + bool empty() const override; private: utils::Synchronized>>>>> diff --git a/src/storage/v2/inmemory/storage.cpp b/src/storage/v2/inmemory/storage.cpp index 1ea909450..dab56750b 100644 --- a/src/storage/v2/inmemory/storage.cpp +++ b/src/storage/v2/inmemory/storage.cpp @@ -779,9 +779,10 @@ utils::BasicResult InMemoryStorage::InMemoryAcce // This is usually done by the MVCC, but it does not handle the metadata deltas transaction_.EnsureCommitTimestampExists(); - if (transaction_.constraint_verification_info.NeedsExistenceConstraintVerification()) { + if (transaction_.constraint_verification_info && + transaction_.constraint_verification_info->NeedsExistenceConstraintVerification()) { const auto vertices_to_update = - transaction_.constraint_verification_info.GetVerticesForExistenceConstraintChecking(); + transaction_.constraint_verification_info->GetVerticesForExistenceConstraintChecking(); for (auto const *vertex : vertices_to_update) { // No need to take any locks here because we modified this vertex and no // one else can touch it until we commit. @@ -808,12 +809,13 @@ utils::BasicResult InMemoryStorage::InMemoryAcce static_cast(storage_->constraints_.unique_constraints_.get()); commit_timestamp_.emplace(mem_storage->CommitTimestamp(reparg.desired_commit_timestamp)); - if (transaction_.constraint_verification_info.NeedsUniqueConstraintVerification()) { + if (transaction_.constraint_verification_info && + transaction_.constraint_verification_info->NeedsUniqueConstraintVerification()) { // Before committing and validating vertices against unique constraints, // we have to update unique constraints with the vertices that are going // to be validated/committed. const auto vertices_to_update = - transaction_.constraint_verification_info.GetVerticesForUniqueConstraintChecking(); + transaction_.constraint_verification_info->GetVerticesForUniqueConstraintChecking(); for (auto const *vertex : vertices_to_update) { mem_unique_constraints->UpdateBeforeCommit(vertex, transaction_); @@ -994,10 +996,11 @@ void InMemoryStorage::InMemoryAccessor::Abort() { // note: this check also saves on unnecessary contention on `engine_lock_` if (!transaction_.deltas.empty()) { // CONSTRAINTS - if (transaction_.constraint_verification_info.NeedsUniqueConstraintVerification()) { + if (transaction_.constraint_verification_info && + transaction_.constraint_verification_info->NeedsUniqueConstraintVerification()) { // Need to remove elements from constraints before handling of the deltas, so the elements match the correct // values - auto vertices_to_check = transaction_.constraint_verification_info.GetVerticesForUniqueConstraintChecking(); + auto vertices_to_check = transaction_.constraint_verification_info->GetVerticesForUniqueConstraintChecking(); auto vertices_to_check_v = std::vector{vertices_to_check.begin(), vertices_to_check.end()}; storage_->constraints_.AbortEntries(vertices_to_check_v, transaction_.start_timestamp); } @@ -1449,7 +1452,7 @@ Transaction InMemoryStorage::CreateTransaction( start_timestamp = timestamp_; } } - return {transaction_id, start_timestamp, isolation_level, storage_mode, false}; + return {transaction_id, start_timestamp, isolation_level, storage_mode, false, !constraints_.empty()}; } void InMemoryStorage::SetStorageMode(StorageMode new_storage_mode) { diff --git a/src/storage/v2/inmemory/unique_constraints.cpp b/src/storage/v2/inmemory/unique_constraints.cpp index e08965eab..dd47a3f68 100644 --- a/src/storage/v2/inmemory/unique_constraints.cpp +++ b/src/storage/v2/inmemory/unique_constraints.cpp @@ -522,5 +522,6 @@ void InMemoryUniqueConstraints::Clear() { constraints_.clear(); constraints_by_label_.clear(); } +bool InMemoryUniqueConstraints::empty() const { return constraints_.empty() && constraints_by_label_.empty(); } } // namespace memgraph::storage diff --git a/src/storage/v2/inmemory/unique_constraints.hpp b/src/storage/v2/inmemory/unique_constraints.hpp index 27fae1b30..40ea0a19e 100644 --- a/src/storage/v2/inmemory/unique_constraints.hpp +++ b/src/storage/v2/inmemory/unique_constraints.hpp @@ -41,6 +41,9 @@ struct FixedCapacityArray { using PropertyIdArray = FixedCapacityArray; class InMemoryUniqueConstraints : public UniqueConstraints { + public: + bool empty() const override; + private: struct Entry { std::vector values; diff --git a/src/storage/v2/transaction.hpp b/src/storage/v2/transaction.hpp index 9f973cbf0..ff1699626 100644 --- a/src/storage/v2/transaction.hpp +++ b/src/storage/v2/transaction.hpp @@ -41,7 +41,7 @@ const uint64_t kTransactionInitialId = 1ULL << 63U; struct Transaction { Transaction(uint64_t transaction_id, uint64_t start_timestamp, IsolationLevel isolation_level, - StorageMode storage_mode, bool edge_import_mode_active) + StorageMode storage_mode, bool edge_import_mode_active, bool has_constraints) : transaction_id(transaction_id), start_timestamp(start_timestamp), command_id(0), @@ -50,6 +50,8 @@ struct Transaction { isolation_level(isolation_level), storage_mode(storage_mode), edge_import_mode_active(edge_import_mode_active), + constraint_verification_info{(has_constraints) ? std::optional{std::in_place} + : std::nullopt}, vertices_{(storage_mode == StorageMode::ON_DISK_TRANSACTIONAL) ? std::optional>{std::in_place} : std::nullopt}, @@ -99,7 +101,7 @@ struct Transaction { // Used to speedup getting info about a vertex when there is a long delta // chain involved in rebuilding that info. mutable VertexInfoCache manyDeltasCache{}; - mutable ConstraintVerificationInfo constraint_verification_info{}; + mutable std::optional constraint_verification_info{}; // Store modified edges GID mapped to changed Delta and serialized edge key // Only for disk storage diff --git a/src/storage/v2/vertex_accessor.cpp b/src/storage/v2/vertex_accessor.cpp index ef0a6ab3e..7d78070a8 100644 --- a/src/storage/v2/vertex_accessor.cpp +++ b/src/storage/v2/vertex_accessor.cpp @@ -120,7 +120,7 @@ Result VertexAccessor::AddLabel(LabelId label) { /// TODO: some by pointers, some by reference => not good, make it better storage_->constraints_.unique_constraints_->UpdateOnAddLabel(label, *vertex_, transaction_->start_timestamp); - transaction_->constraint_verification_info.AddedLabel(vertex_); + if (transaction_->constraint_verification_info) transaction_->constraint_verification_info->AddedLabel(vertex_); storage_->indices_.UpdateOnAddLabel(label, vertex_, *transaction_); transaction_->manyDeltasCache.Invalidate(vertex_, label); @@ -276,10 +276,12 @@ Result VertexAccessor::SetProperty(PropertyId property, const Pro }}; std::invoke(atomic_memory_block); - if (!value.IsNull()) { - transaction_->constraint_verification_info.AddedProperty(vertex_); - } else { - transaction_->constraint_verification_info.RemovedProperty(vertex_); + if (transaction_->constraint_verification_info) { + if (!value.IsNull()) { + transaction_->constraint_verification_info->AddedProperty(vertex_); + } else { + transaction_->constraint_verification_info->RemovedProperty(vertex_); + } } storage_->indices_.UpdateOnSetProperty(property, value, vertex_, *transaction_); transaction_->manyDeltasCache.Invalidate(vertex_, property); @@ -309,10 +311,12 @@ Result VertexAccessor::InitProperties(const std::mapindices_.UpdateOnSetProperty(property, value, vertex, *transaction); transaction->manyDeltasCache.Invalidate(vertex, property); - if (!value.IsNull()) { - transaction->constraint_verification_info.AddedProperty(vertex); - } else { - transaction->constraint_verification_info.RemovedProperty(vertex); + if (transaction->constraint_verification_info) { + if (!value.IsNull()) { + transaction->constraint_verification_info->AddedProperty(vertex); + } else { + transaction->constraint_verification_info->RemovedProperty(vertex); + } } } result = true; @@ -347,10 +351,12 @@ Result>> Vertex storage->indices_.UpdateOnSetProperty(id, new_value, vertex, *transaction); CreateAndLinkDelta(transaction, vertex, Delta::SetPropertyTag(), id, std::move(old_value)); transaction->manyDeltasCache.Invalidate(vertex, id); - if (!new_value.IsNull()) { - transaction->constraint_verification_info.AddedProperty(vertex); - } else { - transaction->constraint_verification_info.RemovedProperty(vertex); + if (transaction->constraint_verification_info) { + if (!new_value.IsNull()) { + transaction->constraint_verification_info->AddedProperty(vertex); + } else { + transaction->constraint_verification_info->RemovedProperty(vertex); + } } } }}; @@ -380,9 +386,11 @@ Result> VertexAccessor::ClearProperties() { for (const auto &[property, value] : *properties) { CreateAndLinkDelta(transaction, vertex, Delta::SetPropertyTag(), property, value); storage->indices_.UpdateOnSetProperty(property, PropertyValue(), vertex, *transaction); - transaction->constraint_verification_info.RemovedProperty(vertex); transaction->manyDeltasCache.Invalidate(vertex, property); } + if (transaction->constraint_verification_info) { + transaction->constraint_verification_info->RemovedProperty(vertex); + } vertex->properties.ClearProperties(); }}; std::invoke(atomic_memory_block); diff --git a/tests/unit/storage_rocks.cpp b/tests/unit/storage_rocks.cpp index 5cdaf4691..539cf3e0a 100644 --- a/tests/unit/storage_rocks.cpp +++ b/tests/unit/storage_rocks.cpp @@ -17,8 +17,6 @@ #include #include "disk_test_utils.hpp" -#include "query/common.hpp" -#include "query/db_accessor.hpp" #include "storage/v2/delta.hpp" #include "storage/v2/disk/storage.hpp" #include "storage/v2/id_types.hpp" diff --git a/tests/unit/storage_v2_wal_file.cpp b/tests/unit/storage_v2_wal_file.cpp index dcb7d3326..4094090f5 100644 --- a/tests/unit/storage_v2_wal_file.cpp +++ b/tests/unit/storage_v2_wal_file.cpp @@ -74,7 +74,7 @@ class DeltaGenerator final { explicit Transaction(DeltaGenerator *gen) : gen_(gen), transaction_(gen->transaction_id_++, gen->timestamp_++, memgraph::storage::IsolationLevel::SNAPSHOT_ISOLATION, - gen->storage_mode_, false) {} + gen->storage_mode_, false, false) {} public: memgraph::storage::Vertex *CreateVertex() {