Fix small bugs

This commit is contained in:
Kostas Kyrimis 2022-11-30 17:24:46 +02:00
parent 9621532d3d
commit 94ef57c459
4 changed files with 47 additions and 35 deletions

View File

@ -14,7 +14,6 @@
#include <iterator> #include <iterator>
#include <vector> #include <vector>
#include "pretty_print_ast_to_original_expression.hpp"
#include "storage/v3/bindings/db_accessor.hpp" #include "storage/v3/bindings/db_accessor.hpp"
#include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp" #include "storage/v3/bindings/pretty_print_ast_to_original_expression.hpp"
#include "storage/v3/expr.hpp" #include "storage/v3/expr.hpp"
@ -245,7 +244,7 @@ ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const
auto pks = PrimaryKeysFromAccessor(acc, view, schema); auto pks = PrimaryKeysFromAccessor(acc, view, schema);
if (pks) { if (pks) {
ret.GetValue().merge(*pks); ret.GetValue().merge(std::move(*pks));
} }
return ret; return ret;

View File

@ -32,7 +32,7 @@ using EdgeFiller =
using msgs::Value; using msgs::Value;
template <typename T> template <typename T>
concept ObjectAccessor = utils::SameAsAnyOf<T, VertexAccessor, EdgeAccessor, std::pair<VertexAccessor, EdgeAccessor>>; concept OrderableObject = utils::SameAsAnyOf<T, VertexAccessor, EdgeAccessor, std::pair<VertexAccessor, EdgeAccessor>>;
inline bool TypedValueCompare(const TypedValue &a, const TypedValue &b) { inline bool TypedValueCompare(const TypedValue &a, const TypedValue &b) {
// in ordering null comes after everything else // in ordering null comes after everything else
@ -126,7 +126,7 @@ class TypedValueVectorCompare final {
std::vector<Ordering> ordering_; std::vector<Ordering> ordering_;
}; };
template <ObjectAccessor TObjectAccessor> template <OrderableObject TObjectAccessor>
struct Element { struct Element {
std::vector<TypedValue> properties_order_by; std::vector<TypedValue> properties_order_by;
TObjectAccessor object_acc; TObjectAccessor object_acc;
@ -164,9 +164,6 @@ std::vector<Element<VertexAccessor>> OrderByVertices(DbAccessor &dba, TIterable
return ordered; return ordered;
} }
template <typename T>
concept EdgeObjectAccessor = utils::SameAsAnyOf<T, EdgeAccessor, std::pair<VertexAccessor, EdgeAccessor>>;
std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable, std::vector<Element<EdgeAccessor>> OrderByEdges(DbAccessor &dba, std::vector<EdgeAccessor> &iterable,
std::vector<msgs::OrderBy> &order_by_edges, std::vector<msgs::OrderBy> &order_by_edges,
const VertexAccessor &vertex_acc); const VertexAccessor &vertex_acc);
@ -198,9 +195,9 @@ std::vector<TypedValue> EvaluateEdgeExpressions(DbAccessor &dba, const VertexAcc
const std::vector<std::string> &expressions); const std::vector<std::string> &expressions);
template <typename T> template <typename T>
concept TAccessor = utils::SameAsAnyOf<T, VertexAccessor, EdgeAccessor>; concept PropertiesAccessor = utils::SameAsAnyOf<T, VertexAccessor, EdgeAccessor>;
template <typename TAccessor> template <PropertiesAccessor TAccessor>
ShardResult<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const TAccessor &acc, ShardResult<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(const TAccessor &acc,
const std::vector<PropertyId> &props, const std::vector<PropertyId> &props,
View view) { View view) {
@ -222,7 +219,7 @@ ShardResult<std::map<PropertyId, Value>> CollectSpecificPropertiesFromAccessor(c
ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view, ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const VertexAccessor &acc, View view,
const Schemas::Schema &schema); const Schemas::Schema &schema);
namespace impl { namespace impl {
template <typename TAccessor> template <PropertiesAccessor TAccessor>
ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesImpl(const TAccessor &acc, View view) { ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesImpl(const TAccessor &acc, View view) {
std::map<PropertyId, Value> ret; std::map<PropertyId, Value> ret;
auto props = acc.Properties(view); auto props = acc.Properties(view);
@ -240,7 +237,7 @@ ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesImpl(const TAccesso
} }
} // namespace impl } // namespace impl
template <typename TAccessor> template <PropertiesAccessor TAccessor>
ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const TAccessor &acc, View view) { ShardResult<std::map<PropertyId, Value>> CollectAllPropertiesFromAccessor(const TAccessor &acc, View view) {
return impl::CollectAllPropertiesImpl<TAccessor>(acc, view); return impl::CollectAllPropertiesImpl<TAccessor>(acc, view);
} }

View File

@ -517,10 +517,13 @@ msgs::WriteResponses ShardRsm::ApplyWrite(msgs::CommitRequest &&req) {
msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) { msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
if (!req.vertex_ids.empty() && !req.vertices_and_edges.empty()) { if (!req.vertex_ids.empty() && !req.vertices_and_edges.empty()) {
auto error = CreateErrorResponse( auto shard_error = SHARD_ERROR(ErrorCode::NONEXISTENT_OBJECT);
{common::ErrorCode::VERTEX_HAS_EDGES, std::experimental::source_location::current()}, req.transaction_id, ""); auto error = CreateErrorResponse(shard_error, req.transaction_id, "");
return msgs::GetPropertiesResponse{.error = {}}; return msgs::GetPropertiesResponse{.error = {}};
} }
if (req.property_ids && req.property_ids->empty()) {
return {};
}
auto shard_acc = shard_->Access(req.transaction_id); auto shard_acc = shard_->Access(req.transaction_id);
auto dba = DbAccessor{&shard_acc}; auto dba = DbAccessor{&shard_acc};
@ -535,8 +538,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
return result; return result;
}; };
auto collect_props = [&req](const VertexAccessor &v_acc, const std::optional<EdgeAccessor> &e_acc) { auto collect_props = [&req](const VertexAccessor &v_acc,
if (req.property_ids) { const std::optional<EdgeAccessor> &e_acc) -> ShardResult<std::map<PropertyId, Value>> {
if (!req.property_ids) {
if (e_acc) { if (e_acc) {
return CollectAllPropertiesFromAccessor(*e_acc, view); return CollectAllPropertiesFromAccessor(*e_acc, view);
} }
@ -544,9 +548,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
} }
if (e_acc) { if (e_acc) {
return CollectSpecificPropertiesFromAccessor(v_acc, *req.property_ids, view); return CollectSpecificPropertiesFromAccessor(*e_acc, *req.property_ids, view);
} }
return CollectSpecificPropertiesFromAccessor(*e_acc, *req.property_ids, view); return CollectSpecificPropertiesFromAccessor(v_acc, *req.property_ids, view);
}; };
auto find_edge = [](const VertexAccessor &v, msgs::EdgeId e) -> std::optional<EdgeAccessor> { auto find_edge = [](const VertexAccessor &v, msgs::EdgeId e) -> std::optional<EdgeAccessor> {
@ -577,9 +581,9 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
return {maybe_id.GetError()}; return {maybe_id.GetError()};
} }
const auto &id = maybe_id.GetValue(); const auto &id = maybe_id.GetValue();
std::optional<msgs::EdgeId> e_type; std::optional<msgs::EdgeId> e_id;
if (e_acc) { if (e_acc) {
e_type = msgs::EdgeId{e_acc->Gid().AsUint()}; e_id = msgs::EdgeId{e_acc->Gid().AsUint()};
} }
msgs::VertexId v_id{msgs::Label{id.primary_label}, ConvertValueVector(id.primary_key)}; msgs::VertexId v_id{msgs::Label{id.primary_label}, ConvertValueVector(id.primary_key)};
auto maybe_props = collect_props(v_acc, e_acc); auto maybe_props = collect_props(v_acc, e_acc);
@ -587,7 +591,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
return {maybe_props.GetError()}; return {maybe_props.GetError()};
} }
auto props = transform_props(std::move(maybe_props.GetValue())); auto props = transform_props(std::move(maybe_props.GetValue()));
auto result = msgs::GetPropertiesResultRow{.vertex = std::move(v_id), .edge = e_type, .props = std::move(props)}; auto result = msgs::GetPropertiesResultRow{.vertex = std::move(v_id), .edge = e_id, .props = std::move(props)};
if (has_expr_to_evaluate) { if (has_expr_to_evaluate) {
std::vector<Value> e_results; std::vector<Value> e_results;
if (e_acc) { if (e_acc) {
@ -610,11 +614,11 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
return limit; return limit;
}; };
auto collect_response = [get_limit, &req](auto &elements, auto result_row_functor) { auto collect_response = [get_limit, &req](auto &elements, auto create_result_row) {
msgs::GetPropertiesResponse response; msgs::GetPropertiesResponse response;
const auto limit = get_limit(elements); const auto limit = get_limit(elements);
for (size_t index = 0; index != limit; ++index) { for (size_t index = 0; index != limit; ++index) {
auto result_row = result_row_functor(elements[index]); auto result_row = create_result_row(elements[index]);
if (result_row.HasError()) { if (result_row.HasError()) {
return msgs::GetPropertiesResponse{.error = CreateErrorResponse(result_row.GetError(), req.transaction_id, "")}; return msgs::GetPropertiesResponse{.error = CreateErrorResponse(result_row.GetError(), req.transaction_id, "")};
} }
@ -626,15 +630,15 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
std::vector<VertexAccessor> vertices; std::vector<VertexAccessor> vertices;
std::vector<EdgeAccessor> edges; std::vector<EdgeAccessor> edges;
auto parse_and_filter = [dba, &vertices](auto &cont, auto projection, auto filter, auto maybe_get_edge) mutable { auto parse_and_filter = [dba, &vertices](auto &container, auto projection, auto filter, auto maybe_get_edge) mutable {
for (const auto &elem : cont) { for (const auto &elem : container) {
const auto &[label, pk_v] = projection(elem); const auto &[label, pk_v] = projection(elem);
auto pk = ConvertPropertyVector(pk_v); auto pk = ConvertPropertyVector(pk_v);
auto v_acc = dba.FindVertex(pk, view); auto v_acc = dba.FindVertex(pk, view);
if (!v_acc || filter(*v_acc, maybe_get_edge(elem))) { if (!v_acc || filter(*v_acc, maybe_get_edge(elem))) {
continue; continue;
} }
vertices.push_back({*v_acc}); vertices.push_back(*v_acc);
} }
}; };
auto identity = [](auto &elem) { return elem; }; auto identity = [](auto &elem) { return elem; };
@ -648,11 +652,15 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
auto filter_edge = [dba, &edges, &req, find_edge](const auto &acc, const auto &edge) mutable { auto filter_edge = [dba, &edges, &req, find_edge](const auto &acc, const auto &edge) mutable {
auto e_acc = find_edge(acc, edge); auto e_acc = find_edge(acc, edge);
if (!req.filter || !e_acc || !FilterOnEdge(dba, acc, *e_acc, {*req.filter})) { if (!e_acc) {
return false; return true;
}
if (req.filter && !FilterOnEdge(dba, acc, *e_acc, {*req.filter})) {
return true;
} }
edges.push_back(*e_acc); edges.push_back(*e_acc);
return true; return false;
}; };
// Handler logic here // Handler logic here
@ -673,6 +681,7 @@ msgs::ReadResponses ShardRsm::HandleRead(msgs::GetPropertiesRequest &&req) {
return collect_response(vertices, return collect_response(vertices,
[emplace_result_row](auto &acc) mutable { return emplace_result_row(acc, std::nullopt); }); [emplace_result_row](auto &acc) mutable { return emplace_result_row(acc, std::nullopt); });
} }
if (!req.order_by.empty()) { if (!req.order_by.empty()) {
auto elements = OrderByEdges(dba, edges, req.order_by, vertices); auto elements = OrderByEdges(dba, edges, req.order_by, vertices);
return collect_response(elements, [emplace_result_row](auto &element) mutable { return collect_response(elements, [emplace_result_row](auto &element) mutable {

View File

@ -489,7 +489,9 @@ msgs::GetPropertiesResponse AttemptToGetProperties(ShardClient &client, std::vec
std::optional<std::string> order_by = std::nullopt) { std::optional<std::string> order_by = std::nullopt) {
msgs::GetPropertiesRequest req{}; msgs::GetPropertiesRequest req{};
req.transaction_id.logical_id = GetTransactionId(); req.transaction_id.logical_id = GetTransactionId();
req.property_ids = std::move(properties); if (!properties.empty()) {
req.property_ids = std::move(properties);
}
if (filter_prop) { if (filter_prop) {
std::string filter_expr = (!edge) ? "MG_SYMBOL_NODE.prop1 >= " : "MG_SYMBOL_EDGE.e_prop = "; std::string filter_expr = (!edge) ? "MG_SYMBOL_NODE.prop1 >= " : "MG_SYMBOL_EDGE.e_prop = ";
@ -1294,13 +1296,15 @@ void TestGetProperties(ShardClient &client) {
const auto prop_id_5 = PropertyId::FromUint(5); const auto prop_id_5 = PropertyId::FromUint(5);
// Vertices // Vertices
{ {
// No properties const auto result = AttemptToGetProperties(client, {}, {v_id, v_id_2}, {});
const auto result = AttemptToGetProperties(client, {}, {v_id, v_id_2}, {}, std::nullopt, unique_prop_val_2);
MG_ASSERT(!result.error); MG_ASSERT(!result.error);
MG_ASSERT(result.result_row.empty()); MG_ASSERT(result.result_row.size() == 2);
for (const auto &elem : result.result_row) {
MG_ASSERT(elem.props.size() == 3);
}
} }
{ {
// All properties // Specific properties
const auto result = AttemptToGetProperties(client, {prop_id_2, prop_id_4, prop_id_5}, {v_id, v_id_2, v_id_3}, {}); const auto result = AttemptToGetProperties(client, {prop_id_2, prop_id_4, prop_id_5}, {v_id, v_id_2, v_id_3}, {});
MG_ASSERT(!result.error); MG_ASSERT(!result.error);
MG_ASSERT(!result.result_row.empty()); MG_ASSERT(!result.result_row.empty());
@ -1393,11 +1397,14 @@ void TestGetProperties(ShardClient &client) {
unique_edge_prop_id, edge_prop_val_2, {edge_type_id})); unique_edge_prop_id, edge_prop_val_2, {edge_type_id}));
const auto edge_prop_id = PropertyId::FromUint(unique_edge_prop_id); const auto edge_prop_id = PropertyId::FromUint(unique_edge_prop_id);
std::vector<msgs::EdgeId> edge_ids = {{edge_gid}, {edge_gid_2}}; std::vector<msgs::EdgeId> edge_ids = {{edge_gid}, {edge_gid_2}};
// no properties // all properties
{ {
const auto result = AttemptToGetProperties(client, {}, {v_id_2, v_id_3}, edge_ids); const auto result = AttemptToGetProperties(client, {}, {v_id_2, v_id_3}, edge_ids);
MG_ASSERT(!result.error); MG_ASSERT(!result.error);
MG_ASSERT(result.result_row.empty()); MG_ASSERT(result.result_row.size() == 2);
for (const auto &elem : result.result_row) {
MG_ASSERT(elem.props.size() == 1);
}
} }
// properties for two vertices // properties for two vertices
{ {