diff --git a/src/distributed/produce_rpc_server.cpp b/src/distributed/produce_rpc_server.cpp index 1dc2a9d9e..b88db8071 100644 --- a/src/distributed/produce_rpc_server.cpp +++ b/src/distributed/produce_rpc_server.cpp @@ -74,7 +74,7 @@ ProduceRpcServer::OngoingProduce::PullOneFromCursor() { if (cursor_->Pull(frame_, context_)) { results.reserve(pull_symbols_.size()); for (const auto &symbol : pull_symbols_) { - results.emplace_back(std::move(frame_[symbol])); + results.push_back(frame_[symbol]); } } else { cursor_state_ = PullState::CURSOR_EXHAUSTED; diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 68e596d7d..06bbf20c2 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -3165,18 +3165,10 @@ class CartesianCursor : public Cursor { } bool Pull(Frame &frame, Context &context) override { - auto copy_frame = [&frame]() { - std::vector result; - for (auto &elem : frame.elems()) { - result.emplace_back(std::move(elem)); - } - return result; - }; - if (!cartesian_pull_initialized_) { // Pull all left_op frames. while (left_op_cursor_->Pull(frame, context)) { - left_op_frames_.emplace_back(copy_frame()); + left_op_frames_.emplace_back(frame.elems()); } // We're setting the iterator to 'end' here so it pulls the right @@ -3201,7 +3193,7 @@ class CartesianCursor : public Cursor { // Advance right_op_cursor_. if (!right_op_cursor_->Pull(frame, context)) return false; - right_op_frame_ = copy_frame(); + right_op_frame_ = frame.elems(); left_op_frames_it_ = left_op_frames_.begin(); } else { // Make sure right_op_cursor last pulled results are on frame. diff --git a/src/query/typed_value.cpp b/src/query/typed_value.cpp index 8d7e90f5e..a1a87a789 100644 --- a/src/query/typed_value.cpp +++ b/src/query/typed_value.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include "glog/logging.h" @@ -50,6 +51,51 @@ TypedValue::TypedValue(const PropertyValue &value) { LOG(FATAL) << "Unsupported type"; } +TypedValue::TypedValue(PropertyValue &&value) { + switch (value.type()) { + case PropertyValue::Type::Null: + type_ = Type::Null; + break; + case PropertyValue::Type::Bool: + type_ = Type::Bool; + bool_v = value.Value(); + break; + case PropertyValue::Type::Int: + type_ = Type::Int; + int_v = value.Value(); + break; + case PropertyValue::Type::Double: + type_ = Type::Double; + double_v = value.Value(); + break; + case PropertyValue::Type::String: + type_ = Type::String; + // TODO: std::move() when PropertyValue is fixed + new (&string_v) std::string(value.Value()); + break; + case PropertyValue::Type::List: { + // TODO: std::move() when PropertyValue is fixed + type_ = Type::List; + auto vec = value.Value>(); + new (&list_v) + std::vector(std::make_move_iterator(vec.begin()), + std::make_move_iterator(vec.end())); + break; + } + case PropertyValue::Type::Map: { + // TODO: std::move() when PropertyValue is fixed + type_ = Type::Map; + auto map = value.Value>(); + new (&map_v) std::map( + std::make_move_iterator(map.begin()), + std::make_move_iterator(map.end())); + break; + } + } + + value = PropertyValue::Null; +} + TypedValue::TypedValue(const TypedValue &other) : type_(other.type_) { switch (other.type_) { case TypedValue::Type::Null: @@ -85,6 +131,42 @@ TypedValue::TypedValue(const TypedValue &other) : type_(other.type_) { LOG(FATAL) << "Unsupported TypedValue::Type"; } +TypedValue::TypedValue(TypedValue &&other) : type_(other.type_) { + switch (other.type_) { + case TypedValue::Type::Null: + break; + case TypedValue::Type::Bool: + this->bool_v = other.bool_v; + break; + case Type::Int: + this->int_v = other.int_v; + break; + case Type::Double: + this->double_v = other.double_v; + break; + case TypedValue::Type::String: + new (&string_v) std::string(std::move(other.string_v)); + break; + case Type::List: + new (&list_v) std::vector(std::move(other.list_v)); + break; + case Type::Map: + new (&map_v) std::map(std::move(other.map_v)); + break; + case Type::Vertex: + new (&vertex_v) VertexAccessor(std::move(other.vertex_v)); + break; + case Type::Edge: + new (&edge_v) EdgeAccessor(std::move(other.edge_v)); + break; + case Type::Path: + new (&path_v) Path(std::move(other.path_v)); + break; + } + + other = TypedValue::Null; +} + TypedValue::operator PropertyValue() const { switch (type_) { case TypedValue::Type::Null: @@ -227,35 +309,57 @@ std::ostream &operator<<(std::ostream &os, const TypedValue &value) { LOG(FATAL) << "Unsupported PropertyValue::Type"; } -#define DEFINE_TYPED_VALUE_ASSIGNMENT(type_param, typed_value_type, member) \ - TypedValue &TypedValue::operator=(const type_param &other) { \ - if (this->type_ == TypedValue::Type::typed_value_type) { \ - this->member = other; \ - } else { \ - *this = TypedValue(other); \ - } \ - \ - return *this; \ +#define DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(type_param, typed_value_type, \ + member) \ + TypedValue &TypedValue::operator=(type_param other) { \ + if (this->type_ == TypedValue::Type::typed_value_type) { \ + this->member = other; \ + } else { \ + *this = TypedValue(other); \ + } \ + \ + return *this; \ } -DEFINE_TYPED_VALUE_ASSIGNMENT(char *const, String, string_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(int, Int, int_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(bool, Bool, bool_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(int64_t, Int, int_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(double, Double, double_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(std::string, String, string_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(std::vector, List, list_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(TypedValue::value_map_t, Map, map_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(VertexAccessor, Vertex, vertex_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(EdgeAccessor, Edge, edge_v) -DEFINE_TYPED_VALUE_ASSIGNMENT(Path, Path, path_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const char *, String, string_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(int, Int, int_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(bool, Bool, bool_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(int64_t, Int, int_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(double, Double, double_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const std::string &, String, string_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const std::vector &, List, + list_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const TypedValue::value_map_t &, Map, map_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const VertexAccessor &, Vertex, vertex_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const EdgeAccessor &, Edge, edge_v) +DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const Path &, Path, path_v) -#undef DEFINE_TYPED_VALUE_ASSIGNMENT +#undef DEFINE_TYPED_VALUE_COPY_ASSIGNMENT + +#define DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(type_param, typed_value_type, \ + member) \ + TypedValue &TypedValue::operator=(type_param &&other) { \ + if (this->type_ == TypedValue::Type::typed_value_type) { \ + this->member = std::move(other); \ + } else { \ + *this = TypedValue(std::move(other)); \ + } \ + \ + return *this; \ + } + +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(std::string, String, string_v) +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(std::vector, List, list_v) +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(TypedValue::value_map_t, Map, map_v) +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(VertexAccessor, Vertex, vertex_v) +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(EdgeAccessor, Edge, edge_v) +DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(Path, Path, path_v) + +#undef DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT TypedValue &TypedValue::operator=(const TypedValue &other) { if (this != &other) { - this->~TypedValue(); - // set the type of this + DestroyValue(); type_ = other.type_; switch (other.type_) { @@ -294,44 +398,92 @@ TypedValue &TypedValue::operator=(const TypedValue &other) { return *this; } +TypedValue &TypedValue::operator=(TypedValue &&other) { + if (this != &other) { + DestroyValue(); + type_ = other.type_; + + switch (other.type_) { + case TypedValue::Type::Null: + break; + case TypedValue::Type::Bool: + this->bool_v = other.bool_v; + break; + case TypedValue::Type::Int: + this->int_v = other.int_v; + break; + case TypedValue::Type::Double: + this->double_v = other.double_v; + break; + case TypedValue::Type::String: + new (&string_v) std::string(std::move(other.string_v)); + break; + case TypedValue::Type::List: + new (&list_v) std::vector(std::move(other.list_v)); + break; + case TypedValue::Type::Map: + new (&map_v) std::map(std::move(other.map_v)); + break; + case TypedValue::Type::Vertex: + new (&vertex_v) VertexAccessor(std::move(other.vertex_v)); + break; + case TypedValue::Type::Edge: + new (&edge_v) EdgeAccessor(std::move(other.edge_v)); + break; + case TypedValue::Type::Path: + new (&path_v) Path(std::move(other.path_v)); + break; + } + + other = TypedValue::Null; + } + + return *this; +} + const TypedValue TypedValue::Null = TypedValue(); -TypedValue::~TypedValue() { +void TypedValue::DestroyValue() { switch (type_) { // destructor for primitive types does nothing - case Type::Null: - case Type::Bool: - case Type::Int: - case Type::Double: - return; + case Type::Null: + case Type::Bool: + case Type::Int: + case Type::Double: + break; // we need to call destructors for non primitive types since we used // placement new - case Type::String: - // Clang fails to compile ~std::string. It seems it is a bug in some - // versions of clang. using namespace std statement solves the issue. - using namespace std; - string_v.~string(); - return; - case Type::List: - using namespace std; - list_v.~vector(); - return; - case Type::Map: - using namespace std; - map_v.~map(); - return; - case Type::Vertex: - vertex_v.~VertexAccessor(); - return; - case Type::Edge: - edge_v.~EdgeAccessor(); - return; - case Type::Path: - path_v.~Path(); - return; + case Type::String: + // Clang fails to compile ~std::string. It seems it is a bug in some + // versions of clang. using namespace std statement solves the issue. + using namespace std; + string_v.~string(); + break; + case Type::List: + using namespace std; + list_v.~vector(); + break; + case Type::Map: + using namespace std; + map_v.~map(); + break; + case Type::Vertex: + vertex_v.~VertexAccessor(); + break; + case Type::Edge: + edge_v.~EdgeAccessor(); + break; + case Type::Path: + path_v.~Path(); + break; } - LOG(FATAL) << "Unsupported TypedValue::Type"; + + type_ = TypedValue::Type::Null; +} + +TypedValue::~TypedValue() { + DestroyValue(); } /** @@ -651,7 +803,7 @@ bool TypedValue::BoolEqual::operator()(const TypedValue &lhs, default: LOG(FATAL) << "Equality between two TypedValues resulted in something other " - "then Null or bool"; + "than Null or bool"; } } diff --git a/src/query/typed_value.hpp b/src/query/typed_value.hpp index ee8f85f4d..5f1f44b84 100644 --- a/src/query/typed_value.hpp +++ b/src/query/typed_value.hpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include "query/path.hpp" @@ -55,6 +56,7 @@ class TypedValue * in the floating point domain (TypedValue operator== behaves the same). * */ using unordered_set = std::unordered_set; + using value_map_t = std::map; /** Private default constructor, makes Null */ TypedValue() : type_(Type::Null) {} @@ -77,6 +79,9 @@ class TypedValue // single static reference to Null, used whenever Null should be returned static const TypedValue Null; + TypedValue(const TypedValue &other); + TypedValue(TypedValue &&other); + // constructors for primitive types TypedValue(bool value) : type_(Type::Bool) { bool_v = value; } TypedValue(int value) : type_(Type::Int) { int_v = value; } @@ -86,7 +91,7 @@ class TypedValue // conversion function to PropertyValue explicit operator PropertyValue() const; - /// constructors for non-primitive types + // copy constructors for non-primitive types TypedValue(const std::string &value) : type_(Type::String) { new (&string_v) std::string(value); } @@ -109,33 +114,50 @@ class TypedValue TypedValue(const Path &path) : type_(Type::Path) { new (&path_v) Path(path); } TypedValue(const PropertyValue &value); -/** - * There are all sorts of explicit assignments here because this way we avoid - * destructor and constructor of TypedValue for creating intermediary values, - * and can fill the typed value storage directly if it has the same underlying - * type. - */ -#define DECLARE_TYPED_VALUE_ASSIGNMENT(type_param) \ - TypedValue &operator=(const type_param &other); + // move constructors for non-primitive types + TypedValue(std::string &&value) : type_(Type::String) { + new (&string_v) std::string(std::move(value)); + } + TypedValue(std::vector &&value) : type_(Type::List) { + new (&list_v) std::vector(std::move(value)); + } + TypedValue(std::map &&value) : type_(Type::Map) { + new (&map_v) std::map(std::move(value)); + } + TypedValue(VertexAccessor &&vertex) : type_(Type::Vertex) { + new (&vertex_v) VertexAccessor(std::move(vertex)); + } + TypedValue(EdgeAccessor &&edge) : type_(Type::Edge) { + new (&edge_v) EdgeAccessor(std::move(edge)); + } + TypedValue(Path &&path) : type_(Type::Path) { + new (&path_v) Path(std::move(path)); + } + TypedValue(PropertyValue &&value); - using value_map_t = std::map; - // Don't delete char * const assignment because char* strings will be assigned - // using boolean assignment (not good). - DECLARE_TYPED_VALUE_ASSIGNMENT(char *const) - DECLARE_TYPED_VALUE_ASSIGNMENT(int) - DECLARE_TYPED_VALUE_ASSIGNMENT(bool) - DECLARE_TYPED_VALUE_ASSIGNMENT(int64_t) - DECLARE_TYPED_VALUE_ASSIGNMENT(double) - DECLARE_TYPED_VALUE_ASSIGNMENT(std::string) - DECLARE_TYPED_VALUE_ASSIGNMENT(std::vector) - DECLARE_TYPED_VALUE_ASSIGNMENT(TypedValue::value_map_t) - DECLARE_TYPED_VALUE_ASSIGNMENT(VertexAccessor) - DECLARE_TYPED_VALUE_ASSIGNMENT(EdgeAccessor) - DECLARE_TYPED_VALUE_ASSIGNMENT(Path) - DECLARE_TYPED_VALUE_ASSIGNMENT(TypedValue) -#undef DECLARE_TYPED_VALUE_ASSIGNMENT + // copy assignment operators + TypedValue &operator=(const char *); + TypedValue &operator=(int); + TypedValue &operator=(bool); + TypedValue &operator=(int64_t); + TypedValue &operator=(double); + TypedValue &operator=(const std::string &); + TypedValue &operator=(const std::vector &); + TypedValue &operator=(const TypedValue::value_map_t &); + TypedValue &operator=(const VertexAccessor &); + TypedValue &operator=(const EdgeAccessor &); + TypedValue &operator=(const Path &); + TypedValue &operator=(const TypedValue &); + + // move assignment operators + TypedValue &operator=(std::string &&); + TypedValue &operator=(std::vector &&); + TypedValue &operator=(TypedValue::value_map_t &&); + TypedValue &operator=(VertexAccessor &&); + TypedValue &operator=(EdgeAccessor &&); + TypedValue &operator=(Path &&); + TypedValue &operator=(TypedValue &&); - TypedValue(const TypedValue &other); ~TypedValue(); Type type() const { return type_; } @@ -189,6 +211,8 @@ class TypedValue friend std::ostream &operator<<(std::ostream &stream, const TypedValue &prop); private: + void DestroyValue(); + // storage for the value of the property union { bool bool_v; diff --git a/src/storage/common/property_value.hpp b/src/storage/common/property_value.hpp index b91b54638..51fefc08c 100644 --- a/src/storage/common/property_value.hpp +++ b/src/storage/common/property_value.hpp @@ -10,8 +10,8 @@ #include "utils/total_ordering.hpp" /** - * Encapsulation of a value and it's type encapsulated in a class that has no - * compiled-time info about that type. + * Encapsulation of a value and its type in a class that has no compile-time + * info about the type. * * Values can be of a number of predefined types that are enumerated in * PropertyValue::Type. Each such type corresponds to exactly one C++ type.