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

Skip Null in SET and REMOVE clauses

Browse Source 
Summary:
openCypher expects removing/setting properties and labels on Null
vertices/edges does not produce an error. Instead, Nulls are simply
skipped.

Reviewers: florijan, mislav.bradac, buda

Reviewed By: mislav.bradac

Subscribers: pullbot

Differential Revision: https://phabricator.memgraph.io/D375
This commit is contained in:
Teon Banek 2017-05-17 12:15:24 +02:00
parent 8d8ef19694
commit 30a4f40093
2 changed files with 97 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/query/plan/operator.cpp
View File

@ -513,6 +513,9 @@ bool SetProperty::SetPropertyCursor::Pull(Frame &frame,
case TypedValue::Type::Edge: case TypedValue::Type::Edge:
lhs.Value<EdgeAccessor>().PropsSet(self_.lhs_->property_, rhs); lhs.Value<EdgeAccessor>().PropsSet(self_.lhs_->property_, rhs);
break; break;
case TypedValue::Type::Null:
// Skip setting properties on Null (can occur in optional match).
break;
default: default:
throw QueryRuntimeException( throw QueryRuntimeException(
"Properties can only be set on Vertices and Edges"); "Properties can only be set on Vertices and Edges");
@ -553,6 +556,9 @@ bool SetProperties::SetPropertiesCursor::Pull(Frame &frame,
case TypedValue::Type::Edge: case TypedValue::Type::Edge:
Set(lhs.Value<EdgeAccessor>(), rhs); Set(lhs.Value<EdgeAccessor>(), rhs);
break; break;
case TypedValue::Type::Null:
// Skip setting properties on Null (can occur in optional match).
break;
default: default:
throw QueryRuntimeException( throw QueryRuntimeException(
"Properties can only be set on Vertices and Edges"); "Properties can only be set on Vertices and Edges");
@ -620,6 +626,9 @@ bool SetLabels::SetLabelsCursor::Pull(Frame &frame,
if (!input_cursor_->Pull(frame, symbol_table)) return false; if (!input_cursor_->Pull(frame, symbol_table)) return false;
TypedValue &vertex_value = frame[self_.input_symbol_]; TypedValue &vertex_value = frame[self_.input_symbol_];
// Skip setting labels on Null (can occur in optional match).
if (vertex_value.IsNull()) return true;
auto &vertex = vertex_value.Value<VertexAccessor>(); auto &vertex = vertex_value.Value<VertexAccessor>();
vertex.SwitchNew(); vertex.SwitchNew();
for (auto label : self_.labels_) vertex.add_label(label); for (auto label : self_.labels_) vertex.add_label(label);
@ -658,6 +667,9 @@ bool RemoveProperty::RemovePropertyCursor::Pull(
case TypedValue::Type::Edge: case TypedValue::Type::Edge:
lhs.Value<EdgeAccessor>().PropsErase(self_.lhs_->property_); lhs.Value<EdgeAccessor>().PropsErase(self_.lhs_->property_);
break; break;
case TypedValue::Type::Null:
// Skip removing properties on Null (can occur in optional match).
break;
default: default:
// TODO consider throwing a TypedValueException here // TODO consider throwing a TypedValueException here
// deal with this when we'll be overhauling error-feedback // deal with this when we'll be overhauling error-feedback
@ -689,6 +701,9 @@ bool RemoveLabels::RemoveLabelsCursor::Pull(Frame &frame,
if (!input_cursor_->Pull(frame, symbol_table)) return false; if (!input_cursor_->Pull(frame, symbol_table)) return false;
TypedValue &vertex_value = frame[self_.input_symbol_]; TypedValue &vertex_value = frame[self_.input_symbol_];
// Skip removing labels on Null (can occur in optional match).
if (vertex_value.IsNull()) return true;
auto &vertex = vertex_value.Value<VertexAccessor>(); auto &vertex = vertex_value.Value<VertexAccessor>();
vertex.SwitchNew(); vertex.SwitchNew();
for (auto label : self_.labels_) vertex.remove_label(label); for (auto label : self_.labels_) vertex.remove_label(label);

82
tests/unit/query_plan_create_set_remove_delete.cpp
View File

@ -854,3 +854,85 @@ TEST(QueryPlan, MergeNoInput) {
dba->advance_command(); dba->advance_command();
EXPECT_EQ(1, CountIterable(dba->vertices())); EXPECT_EQ(1, CountIterable(dba->vertices()));
} }
TEST(QueryPlan, SetPropertyOnNull) {
// SET (Null).prop = 42
Dbms dbms;
auto dba = dbms.active();
AstTreeStorage storage;
SymbolTable symbol_table;
auto prop = dba->property("prop");
auto null = LITERAL(TypedValue::Null);
auto literal = LITERAL(42);
auto n_prop = storage.Create<PropertyLookup>(null, prop);
auto once = std::make_shared<Once>();
auto set_op = std::make_shared<plan::SetProperty>(once, n_prop, literal);
EXPECT_EQ(1, PullAll(set_op, *dba, symbol_table));
}
TEST(QueryPlan, SetPropertiesOnNull) {
// OPTIONAL MATCH (n) SET n = n
Dbms dbms;
auto dba = dbms.active();
AstTreeStorage storage;
SymbolTable symbol_table;
auto n = MakeScanAll(storage, symbol_table, "n");
auto n_ident = IDENT("n");
symbol_table[*n_ident] = n.sym_;
auto optional = std::make_shared<plan::Optional>(nullptr, n.op_,
std::vector<Symbol>{n.sym_});
auto set_op = std::make_shared<plan::SetProperties>(
optional, n.sym_, n_ident, plan::SetProperties::Op::REPLACE);
EXPECT_EQ(0, CountIterable(dba->vertices()));
EXPECT_EQ(1, PullAll(set_op, *dba, symbol_table));
}
TEST(QueryPlan, SetLabelsOnNull) {
// OPTIONAL MATCH (n) SET n :label
Dbms dbms;
auto dba = dbms.active();
auto label = dba->label("label");
AstTreeStorage storage;
SymbolTable symbol_table;
auto n = MakeScanAll(storage, symbol_table, "n");
auto n_ident = IDENT("n");
symbol_table[*n_ident] = n.sym_;
auto optional = std::make_shared<plan::Optional>(nullptr, n.op_,
std::vector<Symbol>{n.sym_});
auto set_op = std::make_shared<plan::SetLabels>(
optional, n.sym_, std::vector<GraphDbTypes::Label>{label});
EXPECT_EQ(0, CountIterable(dba->vertices()));
EXPECT_EQ(1, PullAll(set_op, *dba, symbol_table));
}
TEST(QueryPlan, RemovePropertyOnNull) {
// REMOVE (Null).prop
Dbms dbms;
auto dba = dbms.active();
AstTreeStorage storage;
SymbolTable symbol_table;
auto prop = dba->property("prop");
auto null = LITERAL(TypedValue::Null);
auto n_prop = storage.Create<PropertyLookup>(null, prop);
auto once = std::make_shared<Once>();
auto remove_op = std::make_shared<plan::RemoveProperty>(once, n_prop);
EXPECT_EQ(1, PullAll(remove_op, *dba, symbol_table));
}
TEST(QueryPlan, RemoveLabelsOnNull) {
// OPTIONAL MATCH (n) REMOVE n :label
Dbms dbms;
auto dba = dbms.active();
auto label = dba->label("label");
AstTreeStorage storage;
SymbolTable symbol_table;
auto n = MakeScanAll(storage, symbol_table, "n");
auto n_ident = IDENT("n");
symbol_table[*n_ident] = n.sym_;
auto optional = std::make_shared<plan::Optional>(nullptr, n.op_,
std::vector<Symbol>{n.sym_});
auto remove_op = std::make_shared<plan::RemoveLabels>(
optional, n.sym_, std::vector<GraphDbTypes::Label>{label});
EXPECT_EQ(0, CountIterable(dba->vertices()));
EXPECT_EQ(1, PullAll(remove_op, *dba, symbol_table));
}