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

Fix total_weight not being returned with RETURN *

Browse Source 
Reviewers: teon.banek, mtomic

Reviewed By: teon.banek

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D1665
This commit is contained in:
Lovro Lugovic 2018-10-18 11:16:32 +02:00
parent 4aa9af028c
commit 60ec94fb30
7 changed files with 93 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/query/frontend/ast/ast.lcp
View File

@ -1623,8 +1623,7 @@ cpp<#
(lcp:define-struct return-body ()
((distinct :bool :initval "false"
:documentation "True if distinct results should be produced."
)
:documentation "True if distinct results should be produced.")
(all-identifiers :bool :initval "false"
:documentation "True if asterisk was found in the return body.")
(named-expressions "std::vector<NamedExpression *>"

4
src/query/plan/rule_based_planner.hpp
View File

@ -462,6 +462,10 @@ class RuleBasedPlanner {
bound_symbols.erase(filter_lambda.inner_edge_symbol);
bound_symbols.erase(filter_lambda.inner_node_symbol);
if (total_weight) {
bound_symbols.insert(*total_weight);
}
// TODO: Pass weight lambda.
last_op = std::make_unique<ExpandVariable>(
node_symbol, edge_symbol, edge->type_, expansion.direction,

5
src/utils/visitor.hpp
View File

@ -68,7 +68,6 @@ class VisitorBase<R, T> {
virtual ReturnType Visit(T &) = 0;
};
template <class... T>
class CompositeVisitorBase;
@ -192,7 +191,7 @@ using LeafVisitor = Visitor<bool, TVisitable...>;
/// // Custom implementation for *composite* AddOp expression.
/// }
///
/// void Visit(Identifier &identifier) override {
/// bool Visit(Identifier &identifier) override {
/// // Custom implementation for *leaf* Identifier expression.
/// }
/// };
@ -220,7 +219,7 @@ class CompositeVisitor : public detail::CompositeVisitorBase<TVisitable...> {
/// @code
/// class Expression : public Visitable<ExpressionVisitor> { ... };
///
/// class Identifier : public ExpressionVisitor {
/// class Identifier : public Expression {
/// public:
/// // Use default Accept implementation, since this is a *leaf* type.
/// DEFVISITABLE(ExpressionVisitor)

50
tests/unit/query_common.hpp
View File

@ -148,19 +148,41 @@ auto GetEdge(AstStorage &storage, const std::string &name,
///
/// Name is used to create the Identifier which is assigned to the edge.
auto GetEdgeVariable(AstStorage &storage, const std::string &name,
EdgeAtom::Type type = EdgeAtom::Type::DEPTH_FIRST,
EdgeAtom::Direction dir = EdgeAtom::Direction::BOTH,
const std::vector<storage::EdgeType> &edge_types = {},
Identifier *inner_edge = nullptr,
Identifier *inner_node = nullptr) {
auto r_val =
storage.Create<EdgeAtom>(storage.Create<Identifier>(name),
EdgeAtom::Type::DEPTH_FIRST, dir, edge_types);
Identifier *flambda_inner_edge = nullptr,
Identifier *flambda_inner_node = nullptr,
Identifier *wlambda_inner_edge = nullptr,
Identifier *wlambda_inner_node = nullptr,
Expression *wlambda_expression = nullptr,
Identifier *total_weight = nullptr) {
auto r_val = storage.Create<EdgeAtom>(storage.Create<Identifier>(name), type,
dir, edge_types);
r_val->filter_lambda_.inner_edge =
inner_edge ? inner_edge
: storage.Create<Identifier>(utils::RandomString(20));
flambda_inner_edge ? flambda_inner_edge
: storage.Create<Identifier>(utils::RandomString(20));
r_val->filter_lambda_.inner_node =
inner_node ? inner_node
: storage.Create<Identifier>(utils::RandomString(20));
flambda_inner_node ? flambda_inner_node
: storage.Create<Identifier>(utils::RandomString(20));
if (type == EdgeAtom::Type::WEIGHTED_SHORTEST_PATH) {
r_val->weight_lambda_.inner_edge =
wlambda_inner_edge
? wlambda_inner_edge
: storage.Create<Identifier>(utils::RandomString(20));
r_val->weight_lambda_.inner_node =
wlambda_inner_node
? wlambda_inner_node
: storage.Create<Identifier>(utils::RandomString(20));
r_val->weight_lambda_.expression =
wlambda_expression ? wlambda_expression
: storage.Create<query::PrimitiveLiteral>(1);
r_val->total_weight_ = total_weight;
}
return r_val;
}
@ -268,9 +290,13 @@ void FillReturnBody(AstStorage &, ReturnBody &body,
}
void FillReturnBody(AstStorage &storage, ReturnBody &body,
const std::string &name) {
auto *ident = storage.Create<query::Identifier>(name);
auto *named_expr = storage.Create<query::NamedExpression>(name, ident);
body.named_expressions.emplace_back(named_expr);
if (name == "*") {
body.all_identifiers = true;
} else {
auto *ident = storage.Create<query::Identifier>(name);
auto *named_expr = storage.Create<query::NamedExpression>(name, ident);
body.named_expressions.emplace_back(named_expr);
}
}
void FillReturnBody(AstStorage &, ReturnBody &body, Limit limit) {
body.limit = limit.expression;

40
tests/unit/query_plan.cpp
View File

@ -33,6 +33,7 @@ using query::SingleQuery;
using query::Symbol;
using query::SymbolGenerator;
using query::SymbolTable;
using Type = query::EdgeAtom::Type;
using Direction = query::EdgeAtom::Direction;
using Bound = ScanAllByLabelPropertyRange::Bound;
@ -1183,7 +1184,7 @@ TYPED_TEST(TestPlanner, MatchExpandVariableInlinedFilter) {
auto type = dba.EdgeType("type");
auto prop = PROPERTY_PAIR("prop");
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::BOTH, {type});
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::BOTH, {type});
edge->properties_[prop] = LITERAL(42);
auto *query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r")));
@ -1199,7 +1200,7 @@ TYPED_TEST(TestPlanner, MatchExpandVariableNotInlinedFilter) {
auto type = dba.EdgeType("type");
auto prop = PROPERTY_PAIR("prop");
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::BOTH, {type});
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::BOTH, {type});
edge->properties_[prop] = EQ(PROPERTY_LOOKUP("m", prop), LITERAL(42));
auto *query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r")));
@ -1207,10 +1208,41 @@ TYPED_TEST(TestPlanner, MatchExpandVariableNotInlinedFilter) {
ExpectFilter(), ExpectProduce());
}
TYPED_TEST(TestPlanner, MatchExpandVariableTotalWeightSymbol) {
// Test MATCH p = (a {id: 0})-[r* wShortest (e, v | 1) total_weight]->(b)
// RETURN *
FakeDbAccessor dba;
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Type::WEIGHTED_SHORTEST_PATH, Direction::BOTH,
{}, nullptr, nullptr, nullptr, nullptr, nullptr,
IDENT("total_weight"));
auto *query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("*")));
auto symbol_table = MakeSymbolTable(*query);
auto planner = MakePlanner<TypeParam>(dba, storage, symbol_table, query);
auto *root = dynamic_cast<Produce *>(&planner.plan());
ASSERT_TRUE(root);
const auto &nes = root->named_expressions_;
EXPECT_TRUE(nes.size() == 4);
std::vector<std::string> names(nes.size());
std::transform(nes.begin(), nes.end(), names.begin(),
[](const auto *ne) { return ne->name_; });
EXPECT_TRUE(root->named_expressions_.size() == 4);
EXPECT_TRUE(utils::Contains(names, "m"));
EXPECT_TRUE(utils::Contains(names, "n"));
EXPECT_TRUE(utils::Contains(names, "r"));
EXPECT_TRUE(utils::Contains(names, "total_weight"));
}
TYPED_TEST(TestPlanner, UnwindMatchVariable) {
// Test UNWIND [1,2,3] AS depth MATCH (n) -[r*d]-> (m) RETURN r
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::OUT);
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::OUT);
edge->lower_bound_ = IDENT("d");
edge->upper_bound_ = IDENT("d");
auto *query = QUERY(
@ -1295,7 +1327,7 @@ TYPED_TEST(TestPlanner, MatchWhereAndSplit) {
TYPED_TEST(TestPlanner, ReturnAsteriskOmitsLambdaSymbols) {
// Test MATCH (n) -[r* (ie, in | true)]- (m) RETURN *
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::BOTH);
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::BOTH);
edge->filter_lambda_.inner_edge = IDENT("ie");
edge->filter_lambda_.inner_node = IDENT("in");
edge->filter_lambda_.expression = LITERAL(true);

12
tests/unit/query_semantic.cpp
View File

@ -723,7 +723,8 @@ TEST_F(TestSymbolGenerator, MatchVariablePathUsingUnboundIdentifier) {
TEST_F(TestSymbolGenerator, CreateVariablePath) {
// Test CREATE (n) -[r *]-> (m) raises a SemanticException, since variable
// paths cannot be created.
auto edge = EDGE_VARIABLE("r", EdgeAtom::Direction::OUT);
auto edge =
EDGE_VARIABLE("r", EdgeAtom::Type::DEPTH_FIRST, EdgeAtom::Direction::OUT);
auto query = QUERY(SINGLE_QUERY(CREATE(PATTERN(NODE("n"), edge, NODE("m")))));
EXPECT_THROW(query->Accept(symbol_generator), SemanticException);
}
@ -731,7 +732,8 @@ TEST_F(TestSymbolGenerator, CreateVariablePath) {
TEST_F(TestSymbolGenerator, MergeVariablePath) {
// Test MERGE (n) -[r *]-> (m) raises a SemanticException, since variable
// paths cannot be created.
auto edge = EDGE_VARIABLE("r", EdgeAtom::Direction::OUT);
auto edge =
EDGE_VARIABLE("r", EdgeAtom::Type::DEPTH_FIRST, EdgeAtom::Direction::OUT);
auto query = QUERY(SINGLE_QUERY(MERGE(PATTERN(NODE("n"), edge, NODE("m")))));
EXPECT_THROW(query->Accept(symbol_generator), SemanticException);
}
@ -741,7 +743,8 @@ TEST_F(TestSymbolGenerator, RedeclareVariablePath) {
// This is just a temporary solution, before we add the support for using
// variable paths with already declared symbols. In the future, this test
// should be changed to check for type errors.
auto edge = EDGE_VARIABLE("n", EdgeAtom::Direction::OUT);
auto edge =
EDGE_VARIABLE("n", EdgeAtom::Type::DEPTH_FIRST, EdgeAtom::Direction::OUT);
auto query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("n")));
EXPECT_THROW(query->Accept(symbol_generator), RedeclareVariableError);
@ -752,7 +755,8 @@ TEST_F(TestSymbolGenerator, VariablePathSameIdentifier) {
// `r` cannot be used inside the range expression, since it is bound by the
// variable expansion itself.
auto prop = dba.Property("prop");
auto edge = EDGE_VARIABLE("r", EdgeAtom::Direction::OUT);
auto edge =
EDGE_VARIABLE("r", EdgeAtom::Type::DEPTH_FIRST, EdgeAtom::Direction::OUT);
edge->lower_bound_ = PROPERTY_LOOKUP("r", prop);
auto query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r")));

7
tests/unit/query_variable_start_planner.cpp
View File

@ -11,6 +11,7 @@
using namespace query::plan;
using query::AstStorage;
using Type = query::EdgeAtom::Type;
using Direction = query::EdgeAtom::Direction;
namespace std {
@ -225,7 +226,7 @@ TEST(TestVariableStartPlanner, MatchVariableExpand) {
dba->AdvanceCommand();
// Test MATCH (n) -[r*]-> (m) RETURN r
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::OUT);
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::OUT);
auto *query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r")));
// We expect to get a single column with the following rows:
@ -254,7 +255,7 @@ TEST(TestVariableStartPlanner, MatchVariableExpandReferenceNode) {
dba.AdvanceCommand();
// Test MATCH (n) -[r*..n.id]-> (m) RETURN r
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::OUT);
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::OUT);
edge->upper_bound_ = PROPERTY_LOOKUP("n", id);
auto *query = QUERY(
SINGLE_QUERY(MATCH(PATTERN(NODE("n"), edge, NODE("m"))), RETURN("r")));
@ -280,7 +281,7 @@ TEST(TestVariableStartPlanner, MatchVariableExpandBoth) {
dba->AdvanceCommand();
// Test MATCH (n {id:1}) -[r*]- (m) RETURN r
AstStorage storage;
auto edge = EDGE_VARIABLE("r", Direction::BOTH);
auto edge = EDGE_VARIABLE("r", Type::DEPTH_FIRST, Direction::BOTH);
auto node_n = NODE("n");
node_n->properties_[std::make_pair("id", id)] = LITERAL(1);
auto *query =