From de2e2048ef5058a5c0882ed2707fde324d85ffc0 Mon Sep 17 00:00:00 2001 From: DavIvek Date: Tue, 12 Mar 2024 13:55:40 +0100 Subject: [PATCH] Support label creation via property values (#1762) --- src/query/frontend/ast/ast.hpp | 42 +++++-- .../frontend/ast/cypher_main_visitor.cpp | 25 ++-- .../frontend/opencypher/grammar/Cypher.g4 | 5 +- .../frontend/semantic/symbol_generator.cpp | 47 ++++++++ .../frontend/semantic/symbol_generator.hpp | 6 + src/query/plan/operator.cpp | 94 +++++++++------ src/query/plan/operator.hpp | 21 ++-- src/query/plan/preprocess.cpp | 14 ++- src/query/plan/pretty_print.cpp | 15 ++- src/query/plan/rule_based_planner.hpp | 34 +++--- tests/e2e/load_csv/load_csv.py | 39 ++++++- .../tests/memgraph_V1/features/with.feature | 110 ++++++++++++++++++ tests/unit/plan_pretty_print.cpp | 5 +- tests/unit/query_common.hpp | 4 +- .../query_plan_create_set_remove_delete.cpp | 48 +++++--- tests/unit/query_plan_match_filter_return.cpp | 46 +++++--- tests/unit/query_plan_operator_to_string.cpp | 14 ++- .../unit/query_plan_read_write_typecheck.cpp | 4 +- 18 files changed, 440 insertions(+), 133 deletions(-) diff --git a/src/query/frontend/ast/ast.hpp b/src/query/frontend/ast/ast.hpp index b8d8c9e1a..f136975bc 100644 --- a/src/query/frontend/ast/ast.hpp +++ b/src/query/frontend/ast/ast.hpp @@ -1249,6 +1249,8 @@ class AllPropertiesLookup : public memgraph::query::Expression { friend class AstStorage; }; +using QueryLabelType = std::variant; + class LabelsTest : public memgraph::query::Expression { public: static const utils::TypeInfo kType; @@ -1281,6 +1283,16 @@ class LabelsTest : public memgraph::query::Expression { protected: LabelsTest(Expression *expression, const std::vector &labels) : expression_(expression), labels_(labels) {} + LabelsTest(Expression *expression, const std::vector &labels) : expression_(expression) { + labels_.reserve(labels.size()); + for (const auto &label : labels) { + if (const auto *label_ix = std::get_if(&label)) { + labels_.push_back(*label_ix); + } else { + throw SemanticException("You can't use labels in filter expressions."); + } + } + } private: friend class AstStorage; @@ -1771,7 +1783,7 @@ class NodeAtom : public memgraph::query::PatternAtom { return visitor.PostVisit(*this); } - std::vector labels_; + std::vector labels_; std::variant, memgraph::query::ParameterLookup *> properties_; @@ -1781,7 +1793,11 @@ class NodeAtom : public memgraph::query::PatternAtom { object->identifier_ = identifier_ ? identifier_->Clone(storage) : nullptr; object->labels_.resize(labels_.size()); for (auto i = 0; i < object->labels_.size(); ++i) { - object->labels_[i] = storage->GetLabelIx(labels_[i].name); + if (const auto *label = std::get_if(&labels_[i])) { + object->labels_[i] = storage->GetLabelIx(label->name); + } else { + object->labels_[i] = std::get(labels_[i])->Clone(storage); + } } if (const auto *properties = std::get_if>(&properties_)) { auto &new_obj_properties = std::get>(object->properties_); @@ -2657,20 +2673,25 @@ class SetLabels : public memgraph::query::Clause { } memgraph::query::Identifier *identifier_{nullptr}; - std::vector labels_; + std::vector labels_; SetLabels *Clone(AstStorage *storage) const override { SetLabels *object = storage->Create(); object->identifier_ = identifier_ ? identifier_->Clone(storage) : nullptr; object->labels_.resize(labels_.size()); for (auto i = 0; i < object->labels_.size(); ++i) { - object->labels_[i] = storage->GetLabelIx(labels_[i].name); + if (const auto *label = std::get_if(&labels_[i])) { + object->labels_[i] = storage->GetLabelIx(label->name); + } else { + object->labels_[i] = std::get(labels_[i])->Clone(storage); + } } return object; } protected: - SetLabels(Identifier *identifier, const std::vector &labels) : identifier_(identifier), labels_(labels) {} + SetLabels(Identifier *identifier, std::vector labels) + : identifier_(identifier), labels_(std::move(labels)) {} private: friend class AstStorage; @@ -2720,20 +2741,25 @@ class RemoveLabels : public memgraph::query::Clause { } memgraph::query::Identifier *identifier_{nullptr}; - std::vector labels_; + std::vector labels_; RemoveLabels *Clone(AstStorage *storage) const override { RemoveLabels *object = storage->Create(); object->identifier_ = identifier_ ? identifier_->Clone(storage) : nullptr; object->labels_.resize(labels_.size()); for (auto i = 0; i < object->labels_.size(); ++i) { - object->labels_[i] = storage->GetLabelIx(labels_[i].name); + if (const auto *label = std::get_if(&labels_[i])) { + object->labels_[i] = storage->GetLabelIx(label->name); + } else { + object->labels_[i] = std::get(labels_[i])->Clone(storage); + } } return object; } protected: - RemoveLabels(Identifier *identifier, const std::vector &labels) : identifier_(identifier), labels_(labels) {} + RemoveLabels(Identifier *identifier, std::vector labels) + : identifier_(identifier), labels_(std::move(labels)) {} private: friend class AstStorage; diff --git a/src/query/frontend/ast/cypher_main_visitor.cpp b/src/query/frontend/ast/cypher_main_visitor.cpp index 467c73125..ceebe2815 100644 --- a/src/query/frontend/ast/cypher_main_visitor.cpp +++ b/src/query/frontend/ast/cypher_main_visitor.cpp @@ -1933,7 +1933,7 @@ antlrcpp::Any CypherMainVisitor::visitNodePattern(MemgraphCypher::NodePatternCon anonymous_identifiers.push_back(&node->identifier_); } if (ctx->nodeLabels()) { - node->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); + node->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); } if (ctx->properties()) { // This can return either properties or parameters @@ -1947,16 +1947,27 @@ antlrcpp::Any CypherMainVisitor::visitNodePattern(MemgraphCypher::NodePatternCon } antlrcpp::Any CypherMainVisitor::visitNodeLabels(MemgraphCypher::NodeLabelsContext *ctx) { - std::vector labels; + std::vector labels; for (auto *node_label : ctx->nodeLabel()) { - if (node_label->labelName()->symbolicName()) { + auto *label_name = node_label->labelName(); + if (label_name->symbolicName()) { labels.emplace_back(AddLabel(std::any_cast(node_label->accept(this)))); - } else { + } else if (label_name->parameter()) { // If we have a parameter, we have to resolve it. const auto *param_lookup = std::any_cast(node_label->accept(this)); const auto label_name = parameters_->AtTokenPosition(param_lookup->token_position_).ValueString(); labels.emplace_back(storage_->GetLabelIx(label_name)); query_info_.is_cacheable = false; // We can't cache queries with label parameters. + } else { + auto variable = std::any_cast(label_name->variable()->accept(this)); + users_identifiers.insert(variable); + auto *expression = static_cast(storage_->Create(variable)); + for (auto *lookup : label_name->propertyLookup()) { + auto key = std::any_cast(lookup->accept(this)); + auto *property_lookup = storage_->Create(expression, key); + expression = property_lookup; + } + labels.emplace_back(expression); } } return labels; @@ -2504,7 +2515,7 @@ antlrcpp::Any CypherMainVisitor::visitListIndexingOrSlicing(MemgraphCypher::List antlrcpp::Any CypherMainVisitor::visitExpression2a(MemgraphCypher::Expression2aContext *ctx) { auto *expression = std::any_cast(ctx->expression2b()->accept(this)); if (ctx->nodeLabels()) { - auto labels = std::any_cast>(ctx->nodeLabels()->accept(this)); + auto labels = std::any_cast>(ctx->nodeLabels()->accept(this)); expression = storage_->Create(expression, labels); } return expression; @@ -2830,7 +2841,7 @@ antlrcpp::Any CypherMainVisitor::visitSetItem(MemgraphCypher::SetItemContext *ct // SetLabels auto *set_labels = storage_->Create(); set_labels->identifier_ = storage_->Create(std::any_cast(ctx->variable()->accept(this))); - set_labels->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); + set_labels->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); return static_cast(set_labels); } @@ -2853,7 +2864,7 @@ antlrcpp::Any CypherMainVisitor::visitRemoveItem(MemgraphCypher::RemoveItemConte // RemoveLabels auto *remove_labels = storage_->Create(); remove_labels->identifier_ = storage_->Create(std::any_cast(ctx->variable()->accept(this))); - remove_labels->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); + remove_labels->labels_ = std::any_cast>(ctx->nodeLabels()->accept(this)); return static_cast(remove_labels); } diff --git a/src/query/frontend/opencypher/grammar/Cypher.g4 b/src/query/frontend/opencypher/grammar/Cypher.g4 index 55cb53ef3..7fa218598 100644 --- a/src/query/frontend/opencypher/grammar/Cypher.g4 +++ b/src/query/frontend/opencypher/grammar/Cypher.g4 @@ -193,7 +193,10 @@ nodeLabels : nodeLabel ( nodeLabel )* ; nodeLabel : ':' labelName ; -labelName : symbolicName | parameter; +labelName : symbolicName + | parameter + | variable ( propertyLookup )+ + ; relTypeName : symbolicName ; diff --git a/src/query/frontend/semantic/symbol_generator.cpp b/src/query/frontend/semantic/symbol_generator.cpp index 2cfbee584..c12915634 100644 --- a/src/query/frontend/semantic/symbol_generator.cpp +++ b/src/query/frontend/semantic/symbol_generator.cpp @@ -568,6 +568,44 @@ bool SymbolGenerator::PostVisit(SetProperty & /*set_property*/) { return true; } +bool SymbolGenerator::PreVisit(SetLabels &set_labels) { + auto &scope = scopes_.back(); + scope.in_set_labels = true; + for (auto &label : set_labels.labels_) { + if (auto *expression = std::get_if(&label)) { + (*expression)->Accept(*this); + } + } + + return true; +} + +bool SymbolGenerator::PostVisit(SetLabels & /*set_labels*/) { + auto &scope = scopes_.back(); + scope.in_set_labels = false; + + return true; +} + +bool SymbolGenerator::PreVisit(RemoveLabels &remove_labels) { + auto &scope = scopes_.back(); + scope.in_remove_labels = true; + for (auto &label : remove_labels.labels_) { + if (auto *expression = std::get_if(&label)) { + (*expression)->Accept(*this); + } + } + + return true; +} + +bool SymbolGenerator::PostVisit(RemoveLabels & /*remove_labels*/) { + auto &scope = scopes_.back(); + scope.in_remove_labels = false; + + return true; +} + // Pattern and its subparts. bool SymbolGenerator::PreVisit(Pattern &pattern) { @@ -602,6 +640,15 @@ bool SymbolGenerator::PreVisit(NodeAtom &node_atom) { }; scope.in_node_atom = true; + + if (scope.in_create) { // you can use expressions with labels only in create + for (auto &label : node_atom.labels_) { + if (auto *expression = std::get_if(&label)) { + (*expression)->Accept(*this); + } + } + } + if (auto *properties = std::get_if>(&node_atom.properties_)) { bool props_or_labels = !properties->empty() || !node_atom.labels_.empty(); diff --git a/src/query/frontend/semantic/symbol_generator.hpp b/src/query/frontend/semantic/symbol_generator.hpp index e5b46fbfe..41122625a 100644 --- a/src/query/frontend/semantic/symbol_generator.hpp +++ b/src/query/frontend/semantic/symbol_generator.hpp @@ -68,6 +68,10 @@ class SymbolGenerator : public HierarchicalTreeVisitor { bool PostVisit(Foreach &) override; bool PreVisit(SetProperty & /*set_property*/) override; bool PostVisit(SetProperty & /*set_property*/) override; + bool PreVisit(SetLabels &) override; + bool PostVisit(SetLabels & /*set_labels*/) override; + bool PreVisit(RemoveLabels &) override; + bool PostVisit(RemoveLabels & /*remove_labels*/) override; // Expressions ReturnType Visit(Identifier &) override; @@ -130,6 +134,8 @@ class SymbolGenerator : public HierarchicalTreeVisitor { bool in_set_property{false}; bool in_call_subquery{false}; bool has_return{false}; + bool in_set_labels{false}; + bool in_remove_labels{false}; // True when visiting a pattern atom (node or edge) identifier, which can be // reused or created in the pattern itself. bool in_pattern_atom_identifier{false}; diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index bf2194641..29f64f950 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -48,6 +48,7 @@ #include "query/procedure/module.hpp" #include "query/typed_value.hpp" #include "range/v3/all.hpp" +#include "storage/v2/id_types.hpp" #include "storage/v2/property_value.hpp" #include "storage/v2/view.hpp" #include "utils/algorithm.hpp" @@ -179,6 +180,20 @@ inline void AbortCheck(ExecutionContext const &context) { if (auto const reason = MustAbort(context); reason != AbortReason::NO_ABORT) throw HintedAbortError(reason); } +std::vector EvaluateLabels(const std::vector &labels, + ExpressionEvaluator &evaluator, DbAccessor *dba) { + std::vector result; + result.reserve(labels.size()); + for (const auto &label : labels) { + if (const auto *label_atom = std::get_if(&label)) { + result.emplace_back(*label_atom); + } else { + result.emplace_back(dba->NameToLabel(std::get(label)->Accept(evaluator).ValueString())); + } + } + return result; +} + } // namespace // NOLINTNEXTLINE(cppcoreguidelines-macro-usage) @@ -214,12 +229,13 @@ CreateNode::CreateNode(const std::shared_ptr &input, NodeCreati // Creates a vertex on this GraphDb. Returns a reference to vertex placed on the // frame. -VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, Frame *frame, ExecutionContext &context) { +VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, Frame *frame, ExecutionContext &context, + std::vector &labels, ExpressionEvaluator &evaluator) { auto &dba = *context.db_accessor; auto new_node = dba.InsertVertex(); context.execution_stats[ExecutionStats::Key::CREATED_NODES] += 1; - for (auto label : node_info.labels) { - auto maybe_error = new_node.AddLabel(label); + for (const auto &label : labels) { + auto maybe_error = std::invoke([&] { return new_node.AddLabel(label); }); if (maybe_error.HasError()) { switch (maybe_error.GetError()) { case storage::Error::SERIALIZATION_ERROR: @@ -234,10 +250,6 @@ VertexAccessor &CreateLocalVertex(const NodeCreationInfo &node_info, Frame *fram } context.execution_stats[ExecutionStats::Key::CREATED_LABELS] += 1; } - // Evaluator should use the latest accessors, as modified in this query, when - // setting properties on new nodes. - ExpressionEvaluator evaluator(frame, context.symbol_table, context.evaluation_context, context.db_accessor, - storage::View::NEW); // TODO: PropsSetChecked allocates a PropertyValue, make it use context.memory // when we update PropertyValue with custom allocator. std::map properties; @@ -277,16 +289,21 @@ CreateNode::CreateNodeCursor::CreateNodeCursor(const CreateNode &self, utils::Me bool CreateNode::CreateNodeCursor::Pull(Frame &frame, ExecutionContext &context) { OOMExceptionEnabler oom_exception; SCOPED_PROFILE_OP("CreateNode"); -#ifdef MG_ENTERPRISE - if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && - !context.auth_checker->Has(self_.node_info_.labels, - memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { - throw QueryRuntimeException("Vertex not created due to not having enough permission!"); - } -#endif + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, + storage::View::NEW); if (input_cursor_->Pull(frame, context)) { - auto created_vertex = CreateLocalVertex(self_.node_info_, &frame, context); + // we have to resolve the labels before we can check for permissions + auto labels = EvaluateLabels(self_.node_info_.labels, evaluator, context.db_accessor); + +#ifdef MG_ENTERPRISE + if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && + !context.auth_checker->Has(labels, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { + throw QueryRuntimeException("Vertex not created due to not having enough permission!"); + } +#endif + + auto created_vertex = CreateLocalVertex(self_.node_info_, &frame, context, labels, evaluator); if (context.trigger_context_collector) { context.trigger_context_collector->RegisterCreatedObject(created_vertex); } @@ -370,6 +387,9 @@ bool CreateExpand::CreateExpandCursor::Pull(Frame &frame, ExecutionContext &cont SCOPED_PROFILE_OP_BY_REF(self_); if (!input_cursor_->Pull(frame, context)) return false; + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, + storage::View::NEW); + auto labels = EvaluateLabels(self_.node_info_.labels, evaluator, context.db_accessor); #ifdef MG_ENTERPRISE if (license::global_license_checker.IsEnterpriseValidFast()) { @@ -381,7 +401,7 @@ bool CreateExpand::CreateExpandCursor::Pull(Frame &frame, ExecutionContext &cont if (context.auth_checker && !(context.auth_checker->Has(self_.edge_info_.edge_type, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE) && - context.auth_checker->Has(self_.node_info_.labels, fine_grained_permission))) { + context.auth_checker->Has(labels, fine_grained_permission))) { throw QueryRuntimeException("Edge not created due to not having enough permission!"); } } @@ -391,14 +411,8 @@ bool CreateExpand::CreateExpandCursor::Pull(Frame &frame, ExecutionContext &cont ExpectType(self_.input_symbol_, vertex_value, TypedValue::Type::Vertex); auto &v1 = vertex_value.ValueVertex(); - // Similarly to CreateNode, newly created edges and nodes should use the - // storage::View::NEW. - // E.g. we pickup new properties: `CREATE (n {p: 42}) -[:r {ep: n.p}]-> ()` - ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, - storage::View::NEW); - // get the destination vertex (possibly an existing node) - auto &v2 = OtherVertex(frame, context); + auto &v2 = OtherVertex(frame, context, labels, evaluator); // create an edge between the two nodes auto *dba = context.db_accessor; @@ -429,13 +443,15 @@ void CreateExpand::CreateExpandCursor::Shutdown() { input_cursor_->Shutdown(); } void CreateExpand::CreateExpandCursor::Reset() { input_cursor_->Reset(); } -VertexAccessor &CreateExpand::CreateExpandCursor::OtherVertex(Frame &frame, ExecutionContext &context) { +VertexAccessor &CreateExpand::CreateExpandCursor::OtherVertex(Frame &frame, ExecutionContext &context, + std::vector &labels, + ExpressionEvaluator &evaluator) { if (self_.existing_node_) { TypedValue &dest_node_value = frame[self_.node_info_.symbol]; ExpectType(self_.node_info_.symbol, dest_node_value, TypedValue::Type::Vertex); return dest_node_value.ValueVertex(); } else { - auto &created_vertex = CreateLocalVertex(self_.node_info_, &frame, context); + auto &created_vertex = CreateLocalVertex(self_.node_info_, &frame, context, labels, evaluator); if (context.trigger_context_collector) { context.trigger_context_collector->RegisterCreatedObject(created_vertex); } @@ -3208,8 +3224,8 @@ void SetProperties::SetPropertiesCursor::Shutdown() { input_cursor_->Shutdown(); void SetProperties::SetPropertiesCursor::Reset() { input_cursor_->Reset(); } SetLabels::SetLabels(const std::shared_ptr &input, Symbol input_symbol, - const std::vector &labels) - : input_(input), input_symbol_(std::move(input_symbol)), labels_(labels) {} + std::vector labels) + : input_(input), input_symbol_(std::move(input_symbol)), labels_(std::move(labels)) {} ACCEPT_WITH_INPUT(SetLabels) @@ -3229,16 +3245,18 @@ SetLabels::SetLabelsCursor::SetLabelsCursor(const SetLabels &self, utils::Memory bool SetLabels::SetLabelsCursor::Pull(Frame &frame, ExecutionContext &context) { OOMExceptionEnabler oom_exception; SCOPED_PROFILE_OP("SetLabels"); + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, + storage::View::NEW); + if (!input_cursor_->Pull(frame, context)) return false; + auto labels = EvaluateLabels(self_.labels_, evaluator, context.db_accessor); #ifdef MG_ENTERPRISE if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && - !context.auth_checker->Has(self_.labels_, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { + !context.auth_checker->Has(labels, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { throw QueryRuntimeException("Couldn't set label due to not having enough permission!"); } #endif - if (!input_cursor_->Pull(frame, context)) return false; - TypedValue &vertex_value = frame[self_.input_symbol_]; // Skip setting labels on Null (can occur in optional match). if (vertex_value.IsNull()) return true; @@ -3253,7 +3271,7 @@ bool SetLabels::SetLabelsCursor::Pull(Frame &frame, ExecutionContext &context) { } #endif - for (auto label : self_.labels_) { + for (auto label : labels) { auto maybe_value = vertex.AddLabel(label); if (maybe_value.HasError()) { switch (maybe_value.GetError()) { @@ -3368,8 +3386,8 @@ void RemoveProperty::RemovePropertyCursor::Shutdown() { input_cursor_->Shutdown( void RemoveProperty::RemovePropertyCursor::Reset() { input_cursor_->Reset(); } RemoveLabels::RemoveLabels(const std::shared_ptr &input, Symbol input_symbol, - const std::vector &labels) - : input_(input), input_symbol_(std::move(input_symbol)), labels_(labels) {} + std::vector labels) + : input_(input), input_symbol_(std::move(input_symbol)), labels_(std::move(labels)) {} ACCEPT_WITH_INPUT(RemoveLabels) @@ -3389,16 +3407,18 @@ RemoveLabels::RemoveLabelsCursor::RemoveLabelsCursor(const RemoveLabels &self, u bool RemoveLabels::RemoveLabelsCursor::Pull(Frame &frame, ExecutionContext &context) { OOMExceptionEnabler oom_exception; SCOPED_PROFILE_OP("RemoveLabels"); + ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, + storage::View::NEW); + if (!input_cursor_->Pull(frame, context)) return false; + auto labels = EvaluateLabels(self_.labels_, evaluator, context.db_accessor); #ifdef MG_ENTERPRISE if (license::global_license_checker.IsEnterpriseValidFast() && context.auth_checker && - !context.auth_checker->Has(self_.labels_, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { + !context.auth_checker->Has(labels, memgraph::query::AuthQuery::FineGrainedPrivilege::CREATE_DELETE)) { throw QueryRuntimeException("Couldn't remove label due to not having enough permission!"); } #endif - if (!input_cursor_->Pull(frame, context)) return false; - TypedValue &vertex_value = frame[self_.input_symbol_]; // Skip removing labels on Null (can occur in optional match). if (vertex_value.IsNull()) return true; @@ -3413,7 +3433,7 @@ bool RemoveLabels::RemoveLabelsCursor::Pull(Frame &frame, ExecutionContext &cont } #endif - for (auto label : self_.labels_) { + for (auto label : labels) { auto maybe_value = vertex.RemoveLabel(label); if (maybe_value.HasError()) { switch (maybe_value.GetError()) { diff --git a/src/query/plan/operator.hpp b/src/query/plan/operator.hpp index 6563c2bb0..5a8ef0625 100644 --- a/src/query/plan/operator.hpp +++ b/src/query/plan/operator.hpp @@ -285,6 +285,7 @@ class Once : public memgraph::query::plan::LogicalOperator { }; using PropertiesMapList = std::vector>; +using StorageLabelType = std::variant; struct NodeCreationInfo { static const utils::TypeInfo kType; @@ -292,18 +293,18 @@ struct NodeCreationInfo { NodeCreationInfo() = default; - NodeCreationInfo(Symbol symbol, std::vector labels, + NodeCreationInfo(Symbol symbol, std::vector labels, std::variant properties) : symbol{std::move(symbol)}, labels{std::move(labels)}, properties{std::move(properties)} {}; - NodeCreationInfo(Symbol symbol, std::vector labels, PropertiesMapList properties) + NodeCreationInfo(Symbol symbol, std::vector labels, PropertiesMapList properties) : symbol{std::move(symbol)}, labels{std::move(labels)}, properties{std::move(properties)} {}; - NodeCreationInfo(Symbol symbol, std::vector labels, ParameterLookup *properties) + NodeCreationInfo(Symbol symbol, std::vector labels, ParameterLookup *properties) : symbol{std::move(symbol)}, labels{std::move(labels)}, properties{properties} {}; Symbol symbol; - std::vector labels; + std::vector labels; std::variant properties; NodeCreationInfo Clone(AstStorage *storage) const { @@ -506,7 +507,8 @@ class CreateExpand : public memgraph::query::plan::LogicalOperator { const UniqueCursorPtr input_cursor_; // Get the existing node (if existing_node_ == true), or create a new node - VertexAccessor &OtherVertex(Frame &frame, ExecutionContext &context); + VertexAccessor &OtherVertex(Frame &frame, ExecutionContext &context, + std::vector &labels, ExpressionEvaluator &evaluator); }; }; @@ -1477,8 +1479,7 @@ class SetLabels : public memgraph::query::plan::LogicalOperator { SetLabels() = default; - SetLabels(const std::shared_ptr &input, Symbol input_symbol, - const std::vector &labels); + SetLabels(const std::shared_ptr &input, Symbol input_symbol, std::vector labels); bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override; UniqueCursorPtr MakeCursor(utils::MemoryResource *) const override; std::vector ModifiedSymbols(const SymbolTable &) const override; @@ -1489,7 +1490,7 @@ class SetLabels : public memgraph::query::plan::LogicalOperator { std::shared_ptr input_; Symbol input_symbol_; - std::vector labels_; + std::vector labels_; std::unique_ptr Clone(AstStorage *storage) const override { auto object = std::make_unique(); @@ -1567,7 +1568,7 @@ class RemoveLabels : public memgraph::query::plan::LogicalOperator { RemoveLabels() = default; RemoveLabels(const std::shared_ptr &input, Symbol input_symbol, - const std::vector &labels); + std::vector labels); bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override; UniqueCursorPtr MakeCursor(utils::MemoryResource *) const override; std::vector ModifiedSymbols(const SymbolTable &) const override; @@ -1578,7 +1579,7 @@ class RemoveLabels : public memgraph::query::plan::LogicalOperator { std::shared_ptr input_; Symbol input_symbol_; - std::vector labels_; + std::vector labels_; std::unique_ptr Clone(AstStorage *storage) const override { auto object = std::make_unique(); diff --git a/src/query/plan/preprocess.cpp b/src/query/plan/preprocess.cpp index ca605a46a..2c783fa15 100644 --- a/src/query/plan/preprocess.cpp +++ b/src/query/plan/preprocess.cpp @@ -358,11 +358,17 @@ void Filters::CollectPatternFilters(Pattern &pattern, SymbolTable &symbol_table, }; auto add_node_filter = [&](NodeAtom *node) { const auto &node_symbol = symbol_table.at(*node->identifier_); - if (!node->labels_.empty()) { - // Create a LabelsTest and store it. - auto *labels_test = storage.Create(node->identifier_, node->labels_); + std::vector labels; + for (auto label : node->labels_) { + if (const auto *label_node = std::get_if(&label)) { + throw SemanticException("Property lookup not supported in MATCH/MERGE clause!"); + } + labels.push_back(std::get(label)); + } + if (!labels.empty()) { + auto *labels_test = storage.Create(node->identifier_, labels); auto label_filter = FilterInfo{FilterInfo::Type::Label, labels_test, std::unordered_set{node_symbol}}; - label_filter.labels = node->labels_; + label_filter.labels = labels; all_filters_.emplace_back(label_filter); } add_properties(node); diff --git a/src/query/plan/pretty_print.cpp b/src/query/plan/pretty_print.cpp index 2e6fad9d0..5dd272052 100644 --- a/src/query/plan/pretty_print.cpp +++ b/src/query/plan/pretty_print.cpp @@ -340,7 +340,7 @@ json ToJson(NamedExpression *nexpr) { return json; } -json ToJson(const std::vector> &properties, const DbAccessor &dba) { +json ToJson(const PropertiesMapList &properties, const DbAccessor &dba) { json json; for (const auto &prop_pair : properties) { json.emplace(ToJson(prop_pair.first, dba), ToJson(prop_pair.second)); @@ -348,6 +348,18 @@ json ToJson(const std::vector> &pro return json; } +json ToJson(const std::vector &labels, const DbAccessor &dba) { + json json; + for (const auto &label : labels) { + if (const auto *label_node = std::get_if(&label)) { + json.emplace_back(ToJson(*label_node)); + } else { + json.emplace_back(ToJson(std::get(label), dba)); + } + } + return json; +} + json ToJson(const NodeCreationInfo &node_info, const DbAccessor &dba) { json self; self["symbol"] = ToJson(node_info.symbol); @@ -654,7 +666,6 @@ bool PlanToJsonVisitor::PreVisit(SetLabels &op) { self["name"] = "SetLabels"; self["input_symbol"] = ToJson(op.input_symbol_); self["labels"] = ToJson(op.labels_, *dba_); - op.input_->Accept(*this); self["input"] = PopOutput(); diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index 27f46e764..52281de60 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -293,6 +293,19 @@ class RuleBasedPlanner { storage::EdgeTypeId GetEdgeType(EdgeTypeIx edge_type) { return context_->db->NameToEdgeType(edge_type.name); } + std::vector GetLabelIds(const std::vector &labels) { + std::vector label_ids; + label_ids.reserve(labels.size()); + for (const auto &label : labels) { + if (const auto *label_atom = std::get_if(&label)) { + label_ids.emplace_back(GetLabel(*label_atom)); + } else { + label_ids.emplace_back(std::get(label)); + } + } + return label_ids; + } + std::unique_ptr HandleMatching(std::unique_ptr last_op, const SingleQueryPart &single_query_part, SymbolTable &symbol_table, std::unordered_set &bound_symbols) { @@ -328,11 +341,6 @@ class RuleBasedPlanner { std::unordered_set &bound_symbols) { auto node_to_creation_info = [&](const NodeAtom &node) { const auto &node_symbol = symbol_table.at(*node.identifier_); - std::vector labels; - labels.reserve(node.labels_.size()); - for (const auto &label : node.labels_) { - labels.push_back(GetLabel(label)); - } auto properties = std::invoke([&]() -> std::variant { if (const auto *node_properties = @@ -346,7 +354,7 @@ class RuleBasedPlanner { } return std::get(node.properties_); }); - return NodeCreationInfo{node_symbol, labels, properties}; + return NodeCreationInfo{node_symbol, GetLabelIds(node.labels_), properties}; }; auto base = [&](NodeAtom *node) -> std::unique_ptr { @@ -423,23 +431,13 @@ class RuleBasedPlanner { return std::make_unique(std::move(input_op), input_symbol, set->expression_, op); } else if (auto *set = utils::Downcast(clause)) { const auto &input_symbol = symbol_table.at(*set->identifier_); - std::vector labels; - labels.reserve(set->labels_.size()); - for (const auto &label : set->labels_) { - labels.push_back(GetLabel(label)); - } - return std::make_unique(std::move(input_op), input_symbol, labels); + return std::make_unique(std::move(input_op), input_symbol, GetLabelIds(set->labels_)); } else if (auto *rem = utils::Downcast(clause)) { return std::make_unique(std::move(input_op), GetProperty(rem->property_lookup_->property_), rem->property_lookup_); } else if (auto *rem = utils::Downcast(clause)) { const auto &input_symbol = symbol_table.at(*rem->identifier_); - std::vector labels; - labels.reserve(rem->labels_.size()); - for (const auto &label : rem->labels_) { - labels.push_back(GetLabel(label)); - } - return std::make_unique(std::move(input_op), input_symbol, labels); + return std::make_unique(std::move(input_op), input_symbol, GetLabelIds(rem->labels_)); } return nullptr; } diff --git a/tests/e2e/load_csv/load_csv.py b/tests/e2e/load_csv/load_csv.py index 371803ed1..6483676e6 100644 --- a/tests/e2e/load_csv/load_csv.py +++ b/tests/e2e/load_csv/load_csv.py @@ -53,8 +53,45 @@ def test_given_one_row_in_db_when_load_csv_after_match_then_pass(): assert len(list(results)) == 4 -def test_load_csv_with_parameters(): +def test_creating_labels_with_load_csv_variable(): memgraph = Memgraph("localhost", 7687) + + results = list( + memgraph.execute_and_fetch( + f"""LOAD CSV FROM '{get_file_path(SIMPLE_CSV_FILE)}' WITH HEADER AS row + CREATE (p:row.name) + RETURN p + """ + ) + ) + + assert len(results) == 4 + assert results[0]["p"]._labels == {"Joseph"} + assert results[1]["p"]._labels == {"Peter"} + assert results[2]["p"]._labels == {"Ella"} + assert results[3]["p"]._labels == {"Joe"} + + +def test_create_relationships_with_load_csv_variable2(): + memgraph = Memgraph("localhost", 7687) + + results = list( + memgraph.execute_and_fetch( + f"""LOAD CSV FROM '{get_file_path(SIMPLE_CSV_FILE)}' WITH HEADER AS row + CREATE (p:row.name:Person:row.id) + RETURN p + """ + ) + ) + + assert len(results) == 4 + assert results[0]["p"]._labels == {"Joseph", "Person", "1"} + assert results[1]["p"]._labels == {"Peter", "Person", "2"} + assert results[2]["p"]._labels == {"Ella", "Person", "3"} + assert results[3]["p"]._labels == {"Joe", "Person", "4"} + + +def test_load_csv_with_parameters(): URI = "bolt://localhost:7687" AUTH = ("", "") diff --git a/tests/gql_behave/tests/memgraph_V1/features/with.feature b/tests/gql_behave/tests/memgraph_V1/features/with.feature index f1882e8d7..53c63b5b0 100644 --- a/tests/gql_behave/tests/memgraph_V1/features/with.feature +++ b/tests/gql_behave/tests/memgraph_V1/features/with.feature @@ -264,3 +264,113 @@ Feature: With | id | | 0 | | 1 | + + Scenario: With test 17: + Given an empty graph + And having executed: + """ + CREATE ({name: "node1"}) + """ + When executing query: + """ + MATCH (n) WITH n AS node + CREATE (m:node.name) + """ + When executing query: + """ + MATCH (n:node1) RETURN n; + """ + Then the result should be: + | n | + | (:node1) | + + Scenario: With test 18: + Given an empty graph + And having executed: + """ + CREATE ({name: "LabelToAdd"}) + """ + When executing query: + """ + MATCH (n) WITH n AS node + SET node:node.name + """ + When executing query: + """ + MATCH (n) RETURN n; + """ + Then the result should be: + | n | + | (:LabelToAdd {name: 'LabelToAdd'}) | + + Scenario: With test 19: + Given an empty graph + And having executed: + """ + CREATE (:labelToRemove {name: 'labelToRemove'}) + """ + When executing query: + """ + MATCH (n) WITH n AS node + REMOVE node:node.name + """ + When executing query: + """ + MATCH (n) RETURN n; + """ + Then the result should be: + | n | + | ({name: 'labelToRemove'}) | + + Scenario: With test 20: + Given an empty graph + And having executed: + """ + CREATE ({name: 'label1'}) + """ + When executing query: + """ + MATCH (n) WITH n AS node + SET node:node.name:label2 + """ + When executing query: + """ + MATCH (n) RETURN n; + """ + Then the result should be: + | n | + | (:label1:label2 {name: 'label1'}) | + + Scenario: With test 21: + Given an empty graph + And having executed: + """ + CREATE ({name: 'label1'}) + """ + When executing query: + """ + MATCH (n) WITH n AS node + SET node:label2:node.name + """ + When executing query: + """ + MATCH (n) RETURN n; + """ + Then the result should be: + | n | + | (:label2:label1 {name: 'label1'}) | + + Scenario: With test 22: + Given an empty graph + And having executed: + """ + WITH {value: {label: "labelvalue"}} as label + CREATE (n:label.value.label); + """ + When executing query: + """ + MATCH (n) RETURN n; + """ + Then the result should be: + | n | + | (:labelvalue) | diff --git a/tests/unit/plan_pretty_print.cpp b/tests/unit/plan_pretty_print.cpp index ef2395931..0bc7b35cf 100644 --- a/tests/unit/plan_pretty_print.cpp +++ b/tests/unit/plan_pretty_print.cpp @@ -12,6 +12,7 @@ #include #include "disk_test_utils.hpp" +#include "query/frontend/ast/ast.hpp" #include "query/frontend/semantic/symbol_table.hpp" #include "query/plan/operator.hpp" #include "query/plan/pretty_print.hpp" @@ -515,7 +516,7 @@ TYPED_TEST(PrintToJsonTest, SetLabels) { std::shared_ptr last_op = std::make_shared(nullptr, node_sym); last_op = std::make_shared( last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); this->Check(last_op.get(), R"( { @@ -554,7 +555,7 @@ TYPED_TEST(PrintToJsonTest, RemoveLabels) { std::shared_ptr last_op = std::make_shared(nullptr, node_sym); last_op = std::make_shared( last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); this->Check(last_op.get(), R"( { diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index c18e06abf..6f9b1260a 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -425,7 +425,7 @@ auto GetSet(AstStorage &storage, const std::string &name, Expression *expr, bool /// Create a set labels clause for given identifier name and labels. auto GetSet(AstStorage &storage, const std::string &name, std::vector label_names) { - std::vector labels; + std::vector labels; labels.reserve(label_names.size()); for (const auto &label : label_names) { labels.push_back(storage.GetLabelIx(label)); @@ -438,7 +438,7 @@ auto GetRemove(AstStorage &storage, PropertyLookup *prop_lookup) { return storag /// Create a remove labels clause for given identifier name and labels. auto GetRemove(AstStorage &storage, const std::string &name, std::vector label_names) { - std::vector labels; + std::vector labels; labels.reserve(label_names.size()); for (const auto &label : label_names) { labels.push_back(storage.GetLabelIx(label)); diff --git a/tests/unit/query_plan_create_set_remove_delete.cpp b/tests/unit/query_plan_create_set_remove_delete.cpp index 1fa400940..b32fa91b1 100644 --- a/tests/unit/query_plan_create_set_remove_delete.cpp +++ b/tests/unit/query_plan_create_set_remove_delete.cpp @@ -497,7 +497,7 @@ class MatchCreateNodeWithAuthFixture : public QueryPlanTest { NodeCreationInfo m{}; m.symbol = symbol_table.CreateSymbol("m", true); - std::vector labels{dba.NameToLabel("l2")}; + std::vector labels{dba.NameToLabel("l2")}; m.labels = labels; // creation op auto create_node = std::make_shared(n_scan_all.op_, m); @@ -627,7 +627,7 @@ class MatchCreateExpandWithAuthFixture : public QueryPlanTest { // data for the second node NodeCreationInfo m; m.symbol = cycle ? n_scan_all.sym_ : symbol_table.CreateSymbol("m", true); - std::vector labels{dba.NameToLabel("l2")}; + std::vector labels{dba.NameToLabel("l2")}; m.labels = labels; EdgeCreationInfo r; @@ -1231,12 +1231,14 @@ TYPED_TEST(QueryPlanTest, SetLabels) { ASSERT_TRUE(dba.InsertVertex().AddLabel(label1).HasValue()); ASSERT_TRUE(dba.InsertVertex().AddLabel(label1).HasValue()); dba.AdvanceCommand(); + std::vector labels; + labels.emplace_back(label2); + labels.emplace_back(label3); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); - auto label_set = - std::make_shared(n.op_, n.sym_, std::vector{label2, label3}); + auto label_set = std::make_shared(n.op_, n.sym_, labels); auto context = MakeContext(this->storage, symbol_table, &dba); EXPECT_EQ(2, PullAll(*label_set, &context)); @@ -1255,12 +1257,14 @@ TYPED_TEST(QueryPlanTest, SetLabelsWithFineGrained) { ASSERT_TRUE(dba.InsertVertex().AddLabel(labels[0]).HasValue()); ASSERT_TRUE(dba.InsertVertex().AddLabel(labels[0]).HasValue()); dba.AdvanceCommand(); + std::vector labels_variant; + labels_variant.emplace_back(labels[1]); + labels_variant.emplace_back(labels[2]); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); - auto label_set = - std::make_shared(n.op_, n.sym_, std::vector{labels[1], labels[2]}); + auto label_set = std::make_shared(n.op_, n.sym_, labels_variant); memgraph::glue::FineGrainedAuthChecker auth_checker{user, &dba}; auto context = MakeContextWithFineGrainedChecker(this->storage, symbol_table, &dba, &auth_checker); @@ -1396,12 +1400,14 @@ TYPED_TEST(QueryPlanTest, RemoveLabels) { ASSERT_TRUE(v2.AddLabel(label1).HasValue()); ASSERT_TRUE(v2.AddLabel(label3).HasValue()); dba.AdvanceCommand(); + std::vector labels; + labels.emplace_back(label1); + labels.emplace_back(label2); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); - auto label_remove = - std::make_shared(n.op_, n.sym_, std::vector{label1, label2}); + auto label_remove = std::make_shared(n.op_, n.sym_, labels); auto context = MakeContext(this->storage, symbol_table, &dba); EXPECT_EQ(2, PullAll(*label_remove, &context)); @@ -1425,12 +1431,14 @@ TYPED_TEST(QueryPlanTest, RemoveLabelsFineGrainedFiltering) { ASSERT_TRUE(v2.AddLabel(labels[0]).HasValue()); ASSERT_TRUE(v2.AddLabel(labels[2]).HasValue()); dba.AdvanceCommand(); + std::vector labels_variant; + labels_variant.emplace_back(labels[0]); + labels_variant.emplace_back(labels[1]); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); - auto label_remove = std::make_shared( - n.op_, n.sym_, std::vector{labels[0], labels[1]}); + auto label_remove = std::make_shared(n.op_, n.sym_, labels_variant); memgraph::glue::FineGrainedAuthChecker auth_checker{user, &dba}; auto context = MakeContextWithFineGrainedChecker(this->storage, symbol_table, &dba, &auth_checker); @@ -1569,15 +1577,16 @@ TYPED_TEST(QueryPlanTest, SetRemove) { auto label1 = dba.NameToLabel("label1"); auto label2 = dba.NameToLabel("label2"); dba.AdvanceCommand(); + std::vector labels; + labels.emplace_back(label1); + labels.emplace_back(label2); // Create operations which match (v) and set and remove v :label. // The expected result is single (v) as it was at the start. SymbolTable symbol_table; // MATCH (n) SET n :label1 :label2 REMOVE n :label1 :label2 auto scan_all = MakeScanAll(this->storage, symbol_table, "n"); - auto set = std::make_shared(scan_all.op_, scan_all.sym_, - std::vector{label1, label2}); - auto rem = - std::make_shared(set, scan_all.sym_, std::vector{label1, label2}); + auto set = std::make_shared(scan_all.op_, scan_all.sym_, labels); + auto rem = std::make_shared(set, scan_all.sym_, labels); auto context = MakeContext(this->storage, symbol_table, &dba); EXPECT_EQ(1, PullAll(*rem, &context)); dba.AdvanceCommand(); @@ -1773,10 +1782,12 @@ TYPED_TEST(QueryPlanTest, SetLabelsOnNull) { auto storage_dba = this->db->Access(ReplicationRole::MAIN); memgraph::query::DbAccessor dba(storage_dba.get()); auto label = dba.NameToLabel("label"); + std::vector labels; + labels.emplace_back(label); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); auto optional = std::make_shared(nullptr, n.op_, std::vector{n.sym_}); - auto set_op = std::make_shared(optional, n.sym_, std::vector{label}); + auto set_op = std::make_shared(optional, n.sym_, labels); EXPECT_EQ(0, CountIterable(dba.Vertices(memgraph::storage::View::OLD))); auto context = MakeContext(this->storage, symbol_table, &dba); EXPECT_EQ(1, PullAll(*set_op, &context)); @@ -1801,11 +1812,12 @@ TYPED_TEST(QueryPlanTest, RemoveLabelsOnNull) { auto storage_dba = this->db->Access(ReplicationRole::MAIN); memgraph::query::DbAccessor dba(storage_dba.get()); auto label = dba.NameToLabel("label"); + std::vector labels; + labels.emplace_back(label); SymbolTable symbol_table; auto n = MakeScanAll(this->storage, symbol_table, "n"); auto optional = std::make_shared(nullptr, n.op_, std::vector{n.sym_}); - auto remove_op = - std::make_shared(optional, n.sym_, std::vector{label}); + auto remove_op = std::make_shared(optional, n.sym_, labels); EXPECT_EQ(0, CountIterable(dba.Vertices(memgraph::storage::View::OLD))); auto context = MakeContext(this->storage, symbol_table, &dba); EXPECT_EQ(1, PullAll(*remove_op, &context)); @@ -1906,7 +1918,7 @@ TYPED_TEST(QueryPlanTest, DeleteRemoveLabels) { auto n = MakeScanAll(this->storage, symbol_table, "n"); auto n_get = this->storage.template Create("n")->MapTo(n.sym_); auto delete_op = std::make_shared(n.op_, std::vector{n_get}, false); - std::vector labels{dba.NameToLabel("label")}; + std::vector labels{dba.NameToLabel("label")}; auto rem_op = std::make_shared(delete_op, n.sym_, labels); auto accumulate_op = std::make_shared(rem_op, rem_op->ModifiedSymbols(symbol_table), true); diff --git a/tests/unit/query_plan_match_filter_return.cpp b/tests/unit/query_plan_match_filter_return.cpp index d5468b6b5..925c90c3f 100644 --- a/tests/unit/query_plan_match_filter_return.cpp +++ b/tests/unit/query_plan_match_filter_return.cpp @@ -315,11 +315,12 @@ TYPED_TEST(QueryPlan, NodeFilterLabelsAndProperties) { // make a scan all auto n = MakeScanAll(this->storage, symbol_table, "n"); - n.node_->labels_.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label))); + std::vector labels; + labels.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label))); std::get<0>(n.node_->properties_)[this->storage.GetPropertyIx(property.first)] = LITERAL(42); // node filtering - auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, n.node_->labels_), + auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, labels), EQ(PROPERTY_LOOKUP(dba, n.node_->identifier_, property), LITERAL(42))); auto node_filter = std::make_shared(n.op_, std::vector>{}, filter_expr); @@ -366,11 +367,12 @@ TYPED_TEST(QueryPlan, NodeFilterMultipleLabels) { // make a scan all auto n = MakeScanAll(this->storage, symbol_table, "n"); - n.node_->labels_.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label1))); - n.node_->labels_.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label2))); + std::vector labels; + labels.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label1))); + labels.emplace_back(this->storage.GetLabelIx(dba.LabelToName(label2))); // node filtering - auto *filter_expr = this->storage.template Create(n.node_->identifier_, n.node_->labels_); + auto *filter_expr = this->storage.template Create(n.node_->identifier_, labels); auto node_filter = std::make_shared(n.op_, std::vector>{}, filter_expr); // make a named expression and a produce @@ -2805,9 +2807,10 @@ TYPED_TEST(QueryPlan, OptionalMatchThenExpandToMissingNode) { // OPTIONAL MATCH (n :missing) auto n = MakeScanAll(this->storage, symbol_table, "n"); auto label_missing = "missing"; - n.node_->labels_.emplace_back(this->storage.GetLabelIx(label_missing)); + std::vector labels; + labels.emplace_back(this->storage.GetLabelIx(label_missing)); - auto *filter_expr = this->storage.template Create(n.node_->identifier_, n.node_->labels_); + auto *filter_expr = this->storage.template Create(n.node_->identifier_, labels); auto node_filter = std::make_shared(n.op_, std::vector>{}, filter_expr); auto optional = std::make_shared(nullptr, node_filter, std::vector{n.sym_}); // WITH n @@ -3619,7 +3622,8 @@ class ExistsFixture : public testing::Test { exists_expression->MapTo(symbol_table.CreateAnonymousSymbol()); auto scan_all = MakeScanAll(storage, symbol_table, "n"); - scan_all.node_->labels_.emplace_back(storage.GetLabelIx(match_label)); + std::vector labels; + labels.emplace_back(storage.GetLabelIx(match_label)); std::shared_ptr last_op = std::make_shared( nullptr, scan_all.sym_, dest_sym, edge_sym, direction, edge_types, false, memgraph::storage::View::OLD); @@ -3656,8 +3660,7 @@ class ExistsFixture : public testing::Test { last_op = std::make_shared(std::move(last_op), storage.Create(1)); last_op = std::make_shared(std::move(last_op), symbol_table.at(*exists_expression)); - auto *total_expression = - AND(storage.Create(scan_all.node_->identifier_, scan_all.node_->labels_), exists_expression); + auto *total_expression = AND(storage.Create(scan_all.node_->identifier_, labels), exists_expression); auto filter = std::make_shared(scan_all.op_, std::vector>{last_op}, total_expression); @@ -3709,7 +3712,8 @@ class ExistsFixture : public testing::Test { exists_expression2->MapTo(symbol_table.CreateAnonymousSymbol()); auto scan_all = MakeScanAll(storage, symbol_table, "n"); - scan_all.node_->labels_.emplace_back(storage.GetLabelIx(match_label)); + std::vector labels; + labels.emplace_back(storage.GetLabelIx(match_label)); std::shared_ptr last_op = std::make_shared( nullptr, scan_all.sym_, dest_sym, edge_sym, direction, first_edge_type, false, memgraph::storage::View::OLD); @@ -3721,7 +3725,7 @@ class ExistsFixture : public testing::Test { last_op2 = std::make_shared(std::move(last_op2), storage.Create(1)); last_op2 = std::make_shared(std::move(last_op2), symbol_table.at(*exists_expression2)); - Expression *total_expression = storage.Create(scan_all.node_->identifier_, scan_all.node_->labels_); + Expression *total_expression = storage.Create(scan_all.node_->identifier_, labels); if (or_flag) { total_expression = AND(total_expression, OR(exists_expression, exists_expression2)); @@ -3841,7 +3845,11 @@ TYPED_TEST(SubqueriesFeature, BasicCartesianWithFilter) { // MATCH (n) WHERE n.prop = 2 CALL { MATCH (m) RETURN m } RETURN n, m auto n = MakeScanAll(this->storage, this->symbol_table, "n"); - auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, n.node_->labels_), + std::vector labels; + for (const auto &label : n.node_->labels_) { + labels.emplace_back(std::get(label)); + } + auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, labels), EQ(PROPERTY_LOOKUP(this->dba, n.node_->identifier_, this->prop), LITERAL(2))); auto filter = std::make_shared(n.op_, std::vector>{}, filter_expr); @@ -3866,11 +3874,15 @@ TYPED_TEST(SubqueriesFeature, BasicCartesianWithFilterInsideSubquery) { // MATCH (n) CALL { MATCH (m) WHERE m.prop = 2 RETURN m } RETURN n, m auto n = MakeScanAll(this->storage, this->symbol_table, "n"); + std::vector labels; + for (const auto &label : n.node_->labels_) { + labels.emplace_back(std::get(label)); + } auto return_n = NEXPR("n", IDENT("n")->MapTo(n.sym_))->MapTo(this->symbol_table.CreateSymbol("named_expression_1", true)); auto m = MakeScanAll(this->storage, this->symbol_table, "m"); - auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, n.node_->labels_), + auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, labels), EQ(PROPERTY_LOOKUP(this->dba, n.node_->identifier_, this->prop), LITERAL(2))); auto filter = std::make_shared(m.op_, std::vector>{}, filter_expr); @@ -3891,7 +3903,11 @@ TYPED_TEST(SubqueriesFeature, BasicCartesianWithFilterNoResults) { // MATCH (n) WHERE n.prop = 3 CALL { MATCH (m) RETURN m } RETURN n, m auto n = MakeScanAll(this->storage, this->symbol_table, "n"); - auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, n.node_->labels_), + std::vector labels; + for (const auto &label : n.node_->labels_) { + labels.emplace_back(std::get(label)); + } + auto *filter_expr = AND(this->storage.template Create(n.node_->identifier_, labels), EQ(PROPERTY_LOOKUP(this->dba, n.node_->identifier_, this->prop), LITERAL(3))); auto filter = std::make_shared(n.op_, std::vector>{}, filter_expr); diff --git a/tests/unit/query_plan_operator_to_string.cpp b/tests/unit/query_plan_operator_to_string.cpp index 9696050f2..d60d38251 100644 --- a/tests/unit/query_plan_operator_to_string.cpp +++ b/tests/unit/query_plan_operator_to_string.cpp @@ -290,9 +290,10 @@ TYPED_TEST(OperatorToStringTest, SetProperties) { TYPED_TEST(OperatorToStringTest, SetLabels) { auto node_sym = this->GetSymbol("node"); std::shared_ptr last_op = std::make_shared(nullptr, node_sym); - last_op = std::make_shared( - last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector labels; + labels.emplace_back(this->dba.NameToLabel("label1")); + labels.emplace_back(this->dba.NameToLabel("label2")); + last_op = std::make_shared(last_op, node_sym, labels); std::string expected_string{"SetLabels"}; EXPECT_EQ(last_op->ToString(), expected_string); @@ -311,9 +312,10 @@ TYPED_TEST(OperatorToStringTest, RemoveProperty) { TYPED_TEST(OperatorToStringTest, RemoveLabels) { auto node_sym = this->GetSymbol("node"); std::shared_ptr last_op = std::make_shared(nullptr, node_sym); - last_op = std::make_shared( - last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector labels; + labels.emplace_back(this->dba.NameToLabel("label1")); + labels.emplace_back(this->dba.NameToLabel("label2")); + last_op = std::make_shared(last_op, node_sym, labels); std::string expected_string{"RemoveLabels"}; EXPECT_EQ(last_op->ToString(), expected_string); diff --git a/tests/unit/query_plan_read_write_typecheck.cpp b/tests/unit/query_plan_read_write_typecheck.cpp index f9f14902b..a6af9a03e 100644 --- a/tests/unit/query_plan_read_write_typecheck.cpp +++ b/tests/unit/query_plan_read_write_typecheck.cpp @@ -183,10 +183,10 @@ TYPED_TEST(ReadWriteTypeCheckTest, SetRemovePropertiesLabels) { plan::SetProperties::Op::REPLACE); last_op = std::make_shared( last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); last_op = std::make_shared( last_op, node_sym, - std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); + std::vector{this->dba.NameToLabel("label1"), this->dba.NameToLabel("label2")}); this->CheckPlanType(last_op.get(), RWType::RW); }