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

Use Allocator for string in TypedValue

Browse Source 
Reviewers: mtomic, llugovic, mferencevic

Reviewed By: mtomic, llugovic

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D2104
This commit is contained in:
Teon Banek 2019-05-30 12:30:36 +02:00
parent 16555eeb9a
commit 6bb72d14a1
9 changed files with 177 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/glue/communication.cpp
View File

@ -54,7 +54,7 @@ Value ToBoltValue(const query::TypedValue &value) {
case query::TypedValue::Type::Double:
return Value(value.ValueDouble());
case query::TypedValue::Type::String:
return Value(value.ValueString());
return Value(std::string(value.ValueString()));
case query::TypedValue::Type::List: {
std::vector<Value> values;
values.reserve(value.ValueList().size());

2
src/query/distributed/serialization.cpp
View File

@ -25,7 +25,7 @@ void Save(const query::TypedValue &value, slk::Builder *builder,
return;
case query::TypedValue::Type::String:
slk::Save(static_cast<uint8_t>(4), builder);
slk::Save(value.ValueString(), builder);
slk::Save(std::string(value.ValueString()), builder);
return;
case query::TypedValue::Type::List: {
slk::Save(static_cast<uint8_t>(5), builder);

123
src/query/interpret/awesome_memgraph_functions.cpp
View File

@ -1,5 +1,6 @@
#include "query/interpret/awesome_memgraph_functions.hpp"
#include <algorithm>
#include <cctype>
#include <cmath>
#include <cstdlib>
@ -583,7 +584,8 @@ TypedValue Rand(TypedValue *, int64_t nargs, const EvaluationContext &,
return rand_dist_(pseudo_rand_gen_);
}
template <bool (*Predicate)(const std::string &s1, const std::string &s2)>
template <bool (*Predicate)(const TypedValue::TString &s1,
const TypedValue::TString &s2)>
TypedValue StringMatchOperator(TypedValue *args, int64_t nargs,
const EvaluationContext &,
database::GraphDbAccessor *) {
@ -609,21 +611,24 @@ TypedValue StringMatchOperator(TypedValue *args, int64_t nargs,
}
// Check if s1 starts with s2.
bool StartsWithPredicate(const std::string &s1, const std::string &s2) {
bool StartsWithPredicate(const TypedValue::TString &s1,
const TypedValue::TString &s2) {
if (s1.size() < s2.size()) return false;
return std::equal(s2.begin(), s2.end(), s1.begin());
}
auto StartsWith = StringMatchOperator<StartsWithPredicate>;
// Check if s1 ends with s2.
bool EndsWithPredicate(const std::string &s1, const std::string &s2) {
bool EndsWithPredicate(const TypedValue::TString &s1,
const TypedValue::TString &s2) {
if (s1.size() < s2.size()) return false;
return std::equal(s2.rbegin(), s2.rend(), s1.rbegin());
}
auto EndsWith = StringMatchOperator<EndsWithPredicate>;
// Check if s1 contains s2.
bool ContainsPredicate(const std::string &s1, const std::string &s2) {
bool ContainsPredicate(const TypedValue::TString &s1,
const TypedValue::TString &s2) {
if (s1.size() < s2.size()) return false;
return s1.find(s2) != std::string::npos;
}
@ -642,7 +647,10 @@ TypedValue Assert(TypedValue *args, int64_t nargs, const EvaluationContext &,
"Second argument of 'assert' must be a string.");
if (!args[0].ValueBool()) {
std::string message("Assertion failed");
if (nargs == 2) message += ": " + args[1].ValueString();
if (nargs == 2) {
message += ": ";
message += args[1].ValueString();
}
message += ".";
throw QueryRuntimeException(message);
}
@ -760,7 +768,10 @@ TypedValue Left(TypedValue *args, int64_t nargs, const EvaluationContext &,
"Second argument of 'left' must be a non-negative integer.");
case TypedValue::Type::String:
if (args[1].IsInt() && args[1].ValueInt() >= 0) {
return args[0].ValueString().substr(0, args[1].ValueInt());
auto *memory = args[0].GetMemoryResource();
return TypedValue(
utils::Substr(args[0].ValueString(), 0, args[1].ValueInt()),
memory);
}
throw QueryRuntimeException(
"Second argument of 'left' must be a non-negative integer.");
@ -784,8 +795,12 @@ TypedValue Right(TypedValue *args, int64_t nargs, const EvaluationContext &,
case TypedValue::Type::String: {
const auto &str = args[0].ValueString();
if (args[1].IsInt() && args[1].ValueInt() >= 0) {
int len = args[1].ValueInt();
return len <= str.size() ? str.substr(str.size() - len, len) : str;
auto len = args[1].ValueInt();
auto *memory = args[0].GetMemoryResource();
return len <= str.size()
? TypedValue(utils::Substr(str, str.size() - len, len),
memory)
: TypedValue(str, memory);
}
throw QueryRuntimeException(
"Second argument of 'right' must be a non-negative integer.");
@ -796,30 +811,69 @@ TypedValue Right(TypedValue *args, int64_t nargs, const EvaluationContext &,
}
}
#define WRAP_STRING_FUNCTION(name, lowercased_name, function) \
TypedValue name(TypedValue *args, int64_t nargs, const EvaluationContext &, \
database::GraphDbAccessor *) { \
if (nargs != 1) { \
throw QueryRuntimeException("'" #lowercased_name \
"' requires exactly one argument."); \
} \
switch (args[0].type()) { \
case TypedValue::Type::Null: \
return TypedValue::Null; \
case TypedValue::Type::String: \
return std::string(function(args[0].ValueString())); \
default: \
throw QueryRuntimeException("'" #lowercased_name \
"' argument should be a string."); \
} \
TypedValue CallStringFunction(
TypedValue *args, int64_t nargs, const std::string &name,
std::function<TypedValue::TString(const TypedValue::TString &)> fun) {
if (nargs != 1) {
throw QueryRuntimeException("'" + name +
"' requires exactly one argument.");
}
switch (args[0].type()) {
case TypedValue::Type::Null:
return TypedValue(args[0].GetMemoryResource());
case TypedValue::Type::String:
return fun(args[0].ValueString());
default:
throw QueryRuntimeException("'" + name +
"' argument should be a string.");
}
}
WRAP_STRING_FUNCTION(LTrim, lTrim, utils::LTrim);
WRAP_STRING_FUNCTION(RTrim, rTrim, utils::RTrim);
WRAP_STRING_FUNCTION(Trim, trim, utils::Trim);
WRAP_STRING_FUNCTION(Reverse, reverse, utils::Reversed);
WRAP_STRING_FUNCTION(ToLower, toLower, utils::ToLowerCase);
WRAP_STRING_FUNCTION(ToUpper, toUpper, utils::ToUpperCase);
TypedValue LTrim(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "lTrim", [](const auto &str) {
return TypedValue::TString(utils::LTrim(str), str.get_allocator());
});
}
TypedValue RTrim(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "rTrim", [](const auto &str) {
return TypedValue::TString(utils::RTrim(str), str.get_allocator());
});
}
TypedValue Trim(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "trim", [](const auto &str) {
return TypedValue::TString(utils::Trim(str), str.get_allocator());
});
}
TypedValue Reverse(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "reverse", [](const auto &str) {
return utils::Reversed(str, str.get_allocator());
});
}
TypedValue ToLower(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "toLower", [](const auto &str) {
TypedValue::TString res(str.get_allocator());
utils::ToLowerCase(&res, str);
return res;
});
}
TypedValue ToUpper(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *) {
return CallStringFunction(args, nargs, "toUpper", [](const auto &str) {
TypedValue::TString res(str.get_allocator());
utils::ToUpperCase(&res, str);
return res;
});
}
TypedValue Replace(TypedValue *args, int64_t nargs, const EvaluationContext &,
database::GraphDbAccessor *dba) {
@ -890,12 +944,13 @@ TypedValue Substring(TypedValue *args, int64_t nargs, const EvaluationContext &,
return TypedValue::Null;
}
const auto &str = args[0].ValueString();
int start = args[1].ValueInt();
auto start = args[1].ValueInt();
auto *memory = args[0].GetMemoryResource();
if (nargs == 2) {
return start < str.size() ? str.substr(start) : "";
return TypedValue(utils::Substr(str, start), memory);
}
int len = args[2].ValueInt();
return start < str.size() ? str.substr(start, len) : "";
auto len = args[2].ValueInt();
return TypedValue(utils::Substr(str, start, len), memory);
}
#if MG_SINGLE_NODE

6
src/query/interpret/eval.hpp
View File

@ -184,7 +184,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
throw QueryRuntimeException("Expected a string as a map index, got {}.",
index.type());
const auto &map = lhs.Value<std::map<std::string, TypedValue>>();
auto found = map.find(index.ValueString());
auto found = map.find(std::string(index.ValueString()));
if (found == map.end()) return TypedValue::Null;
return found->second;
}
@ -194,7 +194,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
throw QueryRuntimeException(
"Expected a string as a property name, got {}.", index.type());
return lhs.Value<VertexAccessor>().PropsAt(
dba_->Property(index.ValueString()));
dba_->Property(std::string(index.ValueString())));
}
if (lhs.IsEdge()) {
@ -202,7 +202,7 @@ class ExpressionEvaluator : public ExpressionVisitor<TypedValue> {
throw QueryRuntimeException(
"Expected a string as a property name, got {}.", index.type());
return lhs.Value<EdgeAccessor>().PropsAt(
dba_->Property(index.ValueString()));
dba_->Property(std::string(index.ValueString())));
}
// lhs is Null

14
src/query/interpreter.cpp
View File

@ -114,9 +114,10 @@ Callback HandleAuthQuery(AuthQuery *auth_query, auth::Auth *auth,
std::lock_guard<std::mutex> lock(auth->WithLock());
auto user = auth->AddUser(
username, password.IsString()
? std::make_optional(password.ValueString())
: std::nullopt);
username,
password.IsString()
? std::make_optional(std::string(password.ValueString()))
: std::nullopt);
if (!user) {
throw QueryRuntimeException("User or role '{}' already exists.",
username);
@ -146,9 +147,10 @@ Callback HandleAuthQuery(AuthQuery *auth_query, auth::Auth *auth,
if (!user) {
throw QueryRuntimeException("User '{}' doesn't exist.", username);
}
user->UpdatePassword(password.IsString()
? std::make_optional(password.ValueString())
: std::nullopt);
user->UpdatePassword(
password.IsString()
? std::make_optional(std::string(password.ValueString()))
: std::nullopt);
auth->SaveUser(*user);
return std::vector<std::vector<TypedValue>>();
};

32
src/query/typed_value.cpp
View File

@ -4,6 +4,7 @@
#include <cmath>
#include <iostream>
#include <memory>
#include <string_view>
#include <utility>
#include "glog/logging.h"
@ -39,7 +40,7 @@ TypedValue::TypedValue(const PropertyValue &value,
return;
case PropertyValue::Type::String:
type_ = Type::String;
new (&string_v) std::string(value.Value<std::string>());
new (&string_v) TString(value.Value<std::string>(), memory_);
return;
case PropertyValue::Type::List: {
type_ = Type::List;
@ -83,7 +84,7 @@ TypedValue::TypedValue(PropertyValue &&other, utils::MemoryResource *memory)
break;
case PropertyValue::Type::String:
type_ = Type::String;
new (&string_v) std::string(std::move(other.Value<std::string>()));
new (&string_v) TString(other.Value<std::string>(), memory_);
break;
case PropertyValue::Type::List: {
type_ = Type::List;
@ -127,7 +128,7 @@ TypedValue::TypedValue(const TypedValue &other, utils::MemoryResource *memory)
this->double_v = other.double_v;
return;
case TypedValue::Type::String:
new (&string_v) std::string(other.string_v);
new (&string_v) TString(other.string_v, memory_);
return;
case Type::List:
new (&list_v) TVector(other.list_v, memory_);
@ -166,7 +167,7 @@ TypedValue::TypedValue(TypedValue &&other, utils::MemoryResource *memory)
this->double_v = other.double_v;
break;
case TypedValue::Type::String:
new (&string_v) std::string(std::move(other.string_v));
new (&string_v) TString(std::move(other.string_v), memory_);
break;
case Type::List:
new (&list_v) TVector(std::move(other.list_v), memory_);
@ -198,7 +199,7 @@ TypedValue::operator PropertyValue() const {
case TypedValue::Type::Double:
return PropertyValue(double_v);
case TypedValue::Type::String:
return PropertyValue(string_v);
return PropertyValue(std::string(string_v));
case TypedValue::Type::List:
return PropertyValue(
std::vector<PropertyValue>(list_v.begin(), list_v.end()));
@ -242,7 +243,7 @@ TypedValue::operator PropertyValue() const {
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(std::string, String, string_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::value_map_t, Map, map_v)
DEFINE_VALUE_AND_TYPE_GETTERS(VertexAccessor, Vertex, vertex_v)
@ -346,7 +347,10 @@ 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 TypedValue::TString &, String,
string_v)
DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const std::string &, String, string_v)
DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const std::string_view &, String, string_v)
DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const TypedValue::TVector &, List, list_v)
TypedValue &TypedValue::operator=(const std::vector<TypedValue> &other) {
@ -378,7 +382,7 @@ DEFINE_TYPED_VALUE_COPY_ASSIGNMENT(const Path &, Path, path_v)
return *this; \
}
DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(std::string, String, string_v)
DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(TypedValue::TString, String, string_v)
DEFINE_TYPED_VALUE_MOVE_ASSIGNMENT(TypedValue::TVector, List, list_v)
TypedValue &TypedValue::operator=(std::vector<TypedValue> &&other) {
@ -423,7 +427,7 @@ TypedValue &TypedValue::operator=(const TypedValue &other) {
this->double_v = other.double_v;
return *this;
case TypedValue::Type::String:
new (&string_v) std::string(other.string_v);
new (&string_v) TString(other.string_v, memory_);
return *this;
case TypedValue::Type::List:
new (&list_v) TVector(other.list_v, memory_);
@ -471,7 +475,7 @@ TypedValue &TypedValue::operator=(TypedValue &&other) noexcept(false) {
this->double_v = other.double_v;
break;
case TypedValue::Type::String:
new (&string_v) std::string(std::move(other.string_v));
new (&string_v) TString(std::move(other.string_v), memory_);
break;
case TypedValue::Type::List:
new (&list_v) TVector(std::move(other.list_v), memory_);
@ -508,10 +512,7 @@ void TypedValue::DestroyValue() {
// 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();
string_v.~TString();
break;
case Type::List:
list_v.~TVector();
@ -680,7 +681,8 @@ TypedValue operator!(const TypedValue &a) {
* @return A string.
*/
std::string ValueToString(const TypedValue &value) {
if (value.IsString()) return value.ValueString();
// TODO: Should this allocate a string through value.GetMemoryResource()?
if (value.IsString()) return std::string(value.ValueString());
if (value.IsInt()) return std::to_string(value.Value<int64_t>());
if (value.IsDouble()) return fmt::format("{}", value.Value<double>());
// unsupported situations
@ -894,7 +896,7 @@ size_t TypedValue::Hash::operator()(const TypedValue &value) const {
case TypedValue::Type::Double:
return std::hash<double>{}(value.Value<double>());
case TypedValue::Type::String:
return std::hash<std::string>{}(value.ValueString());
return std::hash<std::string_view>{}(value.ValueString());
case TypedValue::Type::List: {
return utils::FnvCollection<TypedValue::TVector, TypedValue, Hash>{}(
value.ValueList());

64
src/query/typed_value.hpp
View File

@ -5,6 +5,7 @@
#include <map>
#include <memory>
#include <string>
#include <string_view>
#include <unordered_set>
#include <utility>
#include <vector>
@ -67,6 +68,10 @@ class TypedValue {
Path
};
/** Concrete value type of character string */
using TString =
std::basic_string<char, std::char_traits<char>, utils::Allocator<char>>;
/**
* Unordered set of TypedValue items. Can contain at most one Null element,
* and treats an integral and floating point value as same if they are equal
@ -150,13 +155,40 @@ class TypedValue {
TypedValue(const std::string &value,
utils::MemoryResource *memory = utils::NewDeleteResource())
: memory_(memory), type_(Type::String) {
new (&string_v) std::string(value);
new (&string_v) TString(value, memory_);
}
TypedValue(const char *value,
utils::MemoryResource *memory = utils::NewDeleteResource())
: memory_(memory), type_(Type::String) {
new (&string_v) std::string(value);
new (&string_v) TString(value, memory_);
}
explicit TypedValue(
const std::string_view &value,
utils::MemoryResource *memory = utils::NewDeleteResource())
: memory_(memory), type_(Type::String) {
new (&string_v) TString(value, memory_);
}
/**
* Construct a copy of other.
* utils::MemoryResource is obtained by calling
* std::allocator_traits<>::
* select_on_container_copy_construction(other.get_allocator()).
* Since we use utils::Allocator, which does not propagate, this means that
* memory_ will be the default utils::NewDeleteResource().
*/
TypedValue(const TString &other)
: TypedValue(other, std::allocator_traits<utils::Allocator<TypedValue>>::
select_on_container_copy_construction(
other.get_allocator())
.GetMemoryResource()) {}
/** Construct a copy using the given utils::MemoryResource */
TypedValue(const TString &other, utils::MemoryResource *memory)
: memory_(memory), type_(Type::String) {
new (&string_v) TString(other, memory_);
}
/** Construct a copy using the given utils::MemoryResource */
@ -219,13 +251,23 @@ class TypedValue {
TypedValue(const PropertyValue &value, utils::MemoryResource *memory);
// move constructors for non-primitive types
TypedValue(std::string &&value) noexcept : type_(Type::String) {
new (&string_v) std::string(std::move(value));
}
TypedValue(std::string &&value, utils::MemoryResource *memory) noexcept
/**
* Construct with the value of other.
* utils::MemoryResource is obtained from other. After the move, other will be
* left in unspecified state.
*/
TypedValue(TString &&other) noexcept
: TypedValue(std::move(other),
other.get_allocator().GetMemoryResource()) {}
/**
* Construct with the value of other and use the given MemoryResource
* After the move, other will be left in unspecified state.
*/
TypedValue(TString &&other, utils::MemoryResource *memory)
: memory_(memory), type_(Type::String) {
new (&string_v) std::string(std::move(value));
new (&string_v) TString(std::move(other), memory_);
}
/**
@ -330,7 +372,9 @@ class TypedValue {
TypedValue &operator=(bool);
TypedValue &operator=(int64_t);
TypedValue &operator=(double);
TypedValue &operator=(const TString &);
TypedValue &operator=(const std::string &);
TypedValue &operator=(const std::string_view &);
TypedValue &operator=(const TVector &);
TypedValue &operator=(const std::vector<TypedValue> &);
TypedValue &operator=(const TypedValue::value_map_t &);
@ -342,7 +386,7 @@ class TypedValue {
TypedValue &operator=(const TypedValue &other);
// move assignment operators
TypedValue &operator=(std::string &&);
TypedValue &operator=(TString &&);
TypedValue &operator=(TVector &&);
TypedValue &operator=(std::vector<TypedValue> &&);
TypedValue &operator=(TypedValue::value_map_t &&);
@ -383,7 +427,7 @@ class TypedValue {
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(std::string, String)
DECLARE_VALUE_AND_TYPE_GETTERS(TString, String)
/**
* Get the list value.
@ -432,7 +476,7 @@ class TypedValue {
// but most of algorithms (concatenations, serialisation...) has linear time
// complexity so it shouldn't be a problem. This is maybe even faster
// because of data locality.
std::string string_v;
TString string_v;
TVector list_v;
// clang doesn't allow unordered_map to have incomplete type as value so we
// we use map.

2
tests/unit/distributed_interpretation.cpp
View File

@ -153,7 +153,7 @@ TEST_F(DistributedInterpretationTest, RemoteExpandTest2) {
std::vector<std::string> row;
row.reserve(res.size());
for (const auto &col : res) {
row.push_back(col.ValueString());
row.emplace_back(col.ValueString());
}
got.push_back(row);
}

5
tests/unit/query_expression_evaluator.cpp
View File

@ -1171,8 +1171,9 @@ TEST_F(FunctionTest, Labels) {
v.add_label(dba.Label("label2"));
std::vector<std::string> labels;
auto _labels = EvaluateFunction("LABELS", {v}).ValueList();
labels.reserve(_labels.size());
for (auto label : _labels) {
labels.push_back(label.ValueString());
labels.emplace_back(label.ValueString());
}
ASSERT_THAT(labels, UnorderedElementsAre("label1", "label2"));
ASSERT_THROW(EvaluateFunction("LABELS", {2}), QueryRuntimeException);
@ -1251,7 +1252,7 @@ TEST_F(FunctionTest, Keys) {
auto prop_keys_to_string = [](TypedValue t) {
std::vector<std::string> keys;
for (auto property : t.ValueList()) {
keys.push_back(property.ValueString());
keys.emplace_back(property.ValueString());
}
return keys;
};