diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index d20470644..3e0310b91 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -2408,6 +2408,14 @@ Filter::Filter(const std::shared_ptr &input, const std::vector> &pattern_filters, Expression *expression) : input_(input ? input : std::make_shared()), pattern_filters_(pattern_filters), expression_(expression) {} +Filter::Filter(const std::shared_ptr &input, + const std::vector> &pattern_filters, Expression *expression, + const Filters &all_filters) + : input_(input ? input : std::make_shared()), + pattern_filters_(pattern_filters), + expression_(expression), + all_filters_(all_filters) {} + bool Filter::Accept(HierarchicalLogicalOperatorVisitor &visitor) { if (visitor.PreVisit(*this)) { input_->Accept(visitor); @@ -2447,17 +2455,18 @@ Filter::FilterCursor::FilterCursor(const Filter &self, utils::MemoryResource *me bool Filter::FilterCursor::Pull(Frame &frame, ExecutionContext &context) { OOMExceptionEnabler oom_exception; - SCOPED_PROFILE_OP("Filter"); - + SCOPED_PROFILE_OP_BY_REF(self_); + // Like all filters, newly set values should not affect filtering of old // nodes and edges. ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, storage::View::OLD, context.frame_change_collector); - while (input_cursor_->Pull(frame, context)) { + while (input_cursor_->Pull(frame, context) || UNLIKELY(context.is_profile_query)) { for (const auto &pattern_filter_cursor : pattern_filter_cursors_) { pattern_filter_cursor->Pull(frame, context); } if (EvaluateFilter(evaluator, self_.expression_)) return true; + if (UNLIKELY(context.is_profile_query)) return false; } return false; } diff --git a/src/query/plan/operator.hpp b/src/query/plan/operator.hpp index 1b65708b3..2414de9eb 100644 --- a/src/query/plan/operator.hpp +++ b/src/query/plan/operator.hpp @@ -22,6 +22,7 @@ #include "query/common.hpp" #include "query/frontend/ast/ast.hpp" #include "query/frontend/semantic/symbol.hpp" +#include "query/plan/preprocess.hpp" #include "query/typed_value.hpp" #include "storage/v2/id_types.hpp" #include "utils/bound.hpp" @@ -1109,8 +1110,11 @@ class Filter : public memgraph::query::plan::LogicalOperator { Filter() {} - Filter(const std::shared_ptr &input_, - const std::vector> &pattern_filters_, Expression *expression_); + Filter(const std::shared_ptr &input, + const std::vector> &pattern_filters, Expression *expression); + Filter(const std::shared_ptr &input, + const std::vector> &pattern_filters, Expression *expression, + const Filters &all_filters); bool Accept(HierarchicalLogicalOperatorVisitor &visitor) override; UniqueCursorPtr MakeCursor(utils::MemoryResource *) const override; std::vector ModifiedSymbols(const SymbolTable &) const override; @@ -1122,6 +1126,45 @@ class Filter : public memgraph::query::plan::LogicalOperator { std::shared_ptr input_; std::vector> pattern_filters_; Expression *expression_; + const memgraph::query::plan::Filters all_filters_; + + static std::string SingleFilterName(const query::plan::FilterInfo &single_filter) { + using Type = query::plan::FilterInfo::Type; + if (single_filter.type == Type::Generic) { + return fmt::format("Generic {{{}}}", utils::IterableToString(single_filter.used_symbols, ", ", + [](const auto &symbol) { return symbol.name(); })); + } else if (single_filter.type == Type::Id) { + return fmt::format("id({})", single_filter.id_filter->symbol_.name()); + } else if (single_filter.type == Type::Label) { + if (single_filter.expression->GetTypeInfo() != LabelsTest::kType) { + LOG_FATAL("Label filters not using LabelsTest are not supported for query inspection!"); + } + auto filter_expression = static_cast(single_filter.expression); + + if (filter_expression->expression_->GetTypeInfo() != Identifier::kType) { + return fmt::format("(:{})", utils::IterableToString(filter_expression->labels_, ":", + [](const auto &label) { return label.name; })); + } + auto identifier_expression = static_cast(filter_expression->expression_); + + return fmt::format( + "({} :{})", identifier_expression->name_, + utils::IterableToString(filter_expression->labels_, ":", [](const auto &label) { return label.name; })); + } else if (single_filter.type == Type::Pattern) { + return "Pattern"; + } else if (single_filter.type == Type::Property) { + return fmt::format("{{{}.{}}}", single_filter.property_filter->symbol_.name(), + single_filter.property_filter->property_.name); + } else { + LOG_FATAL("Unexpected FilterInfo::Type"); + } + } + + std::string ToString() const override { + return fmt::format("Filter {}", utils::IterableToString(all_filters_, ", ", [](const auto &single_filter) { + return Filter::SingleFilterName(single_filter); + })); + } std::unique_ptr Clone(AstStorage *storage) const override { auto object = std::make_unique(); diff --git a/src/query/plan/preprocess.hpp b/src/query/plan/preprocess.hpp index f261545f7..e9f7aeeb9 100644 --- a/src/query/plan/preprocess.hpp +++ b/src/query/plan/preprocess.hpp @@ -20,7 +20,6 @@ #include "query/frontend/ast/ast.hpp" #include "query/frontend/semantic/symbol_table.hpp" -#include "query/plan/operator.hpp" namespace memgraph::query::plan { @@ -245,7 +244,7 @@ class PatternFilterVisitor : public ExpressionVisitor { /// Stores the symbols and expression used to filter a property. class PropertyFilter { public: - using Bound = ScanAllByLabelPropertyRange::Bound; + using Bound = utils::Bound; /// Depending on type, this PropertyFilter may be a value equality, regex /// matched value or a range with lower and (or) upper bounds, IN list filter. @@ -334,6 +333,8 @@ class Filters final { auto erase(iterator first, iterator last) { return all_filters_.erase(first, last); } auto erase(const_iterator first, const_iterator last) { return all_filters_.erase(first, last); } + void SetFilters(std::vector &&all_filters) { all_filters_ = std::move(all_filters); } + auto FilteredLabels(const Symbol &symbol) const { std::unordered_set labels; for (const auto &filter : all_filters_) { @@ -410,9 +411,8 @@ class Filters final { void CollectFilterExpression(Expression *, const SymbolTable &); private: - void AnalyzeAndStoreFilter(Expression *, const SymbolTable &); - std::vector all_filters_; + void AnalyzeAndStoreFilter(Expression *, const SymbolTable &); }; /// Normalized representation of a single or multiple Match clauses. diff --git a/src/query/plan/pretty_print.cpp b/src/query/plan/pretty_print.cpp index 8315cf175..a2df9422c 100644 --- a/src/query/plan/pretty_print.cpp +++ b/src/query/plan/pretty_print.cpp @@ -31,7 +31,7 @@ PRE_VISIT(CreateNode); bool PlanPrinter::PreVisit(CreateExpand &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } @@ -39,59 +39,59 @@ bool PlanPrinter::PreVisit(CreateExpand &op) { PRE_VISIT(Delete); bool PlanPrinter::PreVisit(query::plan::ScanAll &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } bool PlanPrinter::PreVisit(query::plan::ScanAllByLabel &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(query::plan::ScanAllByLabelPropertyValue &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(query::plan::ScanAllByLabelPropertyRange &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(query::plan::ScanAllByLabelProperty &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(ScanAllById &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } bool PlanPrinter::PreVisit(query::plan::Expand &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(query::plan::ExpandVariable &op) { op.dba_ = dba_; - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); op.dba_ = nullptr; return true; } bool PlanPrinter::PreVisit(query::plan::Produce &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } @@ -106,7 +106,7 @@ PRE_VISIT(EmptyResult); PRE_VISIT(EvaluatePatternFilter); bool PlanPrinter::PreVisit(query::plan::Aggregate &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } @@ -114,7 +114,7 @@ PRE_VISIT(Skip); PRE_VISIT(Limit); bool PlanPrinter::PreVisit(query::plan::OrderBy &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } @@ -137,19 +137,19 @@ PRE_VISIT(Unwind); PRE_VISIT(Distinct); bool PlanPrinter::PreVisit(query::plan::Union &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); Branch(*op.right_op_); op.left_op_->Accept(*this); return false; } bool PlanPrinter::PreVisit(query::plan::CallProcedure &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } bool PlanPrinter::PreVisit(query::plan::LoadCsv &op) { - WithPrintLn([&](auto &out) { out << "* " << op.ToString(); }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); return true; } @@ -186,7 +186,7 @@ bool PlanPrinter::PreVisit(query::plan::Foreach &op) { } bool PlanPrinter::PreVisit(query::plan::Filter &op) { - WithPrintLn([](auto &out) { out << "* Filter"; }); + WithPrintLn([&op](auto &out) { out << "* " << op.ToString(); }); for (const auto &pattern_filter : op.pattern_filters_) { Branch(*pattern_filter); } diff --git a/src/query/plan/rule_based_planner.hpp b/src/query/plan/rule_based_planner.hpp index dac9462ad..d19d4bfa3 100644 --- a/src/query/plan/rule_based_planner.hpp +++ b/src/query/plan/rule_based_planner.hpp @@ -858,11 +858,13 @@ class RuleBasedPlanner { std::unique_ptr GenFilters(std::unique_ptr last_op, const std::unordered_set &bound_symbols, Filters &filters, AstStorage &storage, const SymbolTable &symbol_table) { + auto all_filters = filters; auto pattern_filters = ExtractPatternFilters(filters, symbol_table, storage, bound_symbols); auto *filter_expr = impl::ExtractFilters(bound_symbols, filters, storage); if (filter_expr) { - last_op = std::make_unique(std::move(last_op), std::move(pattern_filters), filter_expr); + last_op = + std::make_unique(std::move(last_op), std::move(pattern_filters), filter_expr, std::move(all_filters)); } return last_op; } diff --git a/tests/e2e/analyze_graph/optimize_indexes.py b/tests/e2e/analyze_graph/optimize_indexes.py index 7bede8bd4..325993ca1 100644 --- a/tests/e2e/analyze_graph/optimize_indexes.py +++ b/tests/e2e/analyze_graph/optimize_indexes.py @@ -62,7 +62,7 @@ def test_analyze_graph_delete_statistics(delete_query, multi_db): # After deleting statistics, id2 should be chosen because it has less vertices expected_explain_after_delete_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id2}})",), (f" * Once",), ] @@ -96,7 +96,7 @@ def test_analyze_full_graph(analyze_query, multi_db): # Choose id2 before tha analysis because it has less vertices expected_explain_before_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id2}})",), (f" * Once",), ] @@ -117,7 +117,7 @@ def test_analyze_full_graph(analyze_query, multi_db): # After analyzing graph, id1 index should be chosen because it has smaller average group size expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id1}})",), (f" * Once",), ] @@ -152,7 +152,7 @@ def test_cardinality_different_avg_group_size_uniform_dist(multi_db): assert analyze_graph_results[1 - first_index] == ("Label", "id2", 100, 20, 5, 0, 0) expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id1}})",), (f" * Once",), ] @@ -183,7 +183,7 @@ def test_cardinality_same_avg_group_size_uniform_dist_diff_vertex_count(multi_db assert analyze_graph_results[1 - first_index] == ("Label", "id2", 50, 50, 1, 0, 0) expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id2}})",), (f" * Once",), ] @@ -214,7 +214,7 @@ def test_large_diff_in_num_vertices_v1(multi_db): assert analyze_graph_results[1 - first_index] == ("Label", "id2", 99, 1, 99, 0, 0) expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id2}})",), (f" * Once",), ] @@ -245,7 +245,7 @@ def test_large_diff_in_num_vertices_v2(multi_db): assert analyze_graph_results[1 - first_index] == ("Label", "id2", 1000, 1000, 1, 0, 0) expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id1}})",), (f" * Once",), ] @@ -286,7 +286,7 @@ def test_same_avg_group_size_diff_distribution(multi_db): assert analyze_graph_results[1 - first_index] == ("Label", "id2", 100, 5, 20, 0, 0) expected_explain_after_analysis = [ (f" * Produce {{n}}",), - (f" * Filter",), + (f" * Filter (n :Label), {{n.id1}}, {{n.id2}}",), (f" * ScanAllByLabelPropertyValue (n :Label {{id2}})",), (f" * Once",), ] diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index c5d616e24..0779adb41 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -745,7 +745,7 @@ TYPED_TEST(InterpreterTest, ExplainQueryWithParams) { this->Interpret("EXPLAIN MATCH (n) WHERE n.id = $id RETURN *;", {{"id", memgraph::storage::PropertyValue(42)}}); ASSERT_EQ(stream.GetHeader().size(), 1U); EXPECT_EQ(stream.GetHeader().front(), "QUERY PLAN"); - std::vector expected_rows{" * Produce {n}", " * Filter", " * ScanAll (n)", " * Once"}; + std::vector expected_rows{" * Produce {n}", " * Filter {n.id}", " * ScanAll (n)", " * Once"}; ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); auto expected_it = expected_rows.begin(); for (const auto &row : stream.GetResults()) { @@ -834,7 +834,7 @@ TYPED_TEST(InterpreterTest, ProfileQueryWithParams) { this->Interpret("PROFILE MATCH (n) WHERE n.id = $id RETURN *;", {{"id", memgraph::storage::PropertyValue(42)}}); std::vector expected_header{"OPERATOR", "ACTUAL HITS", "RELATIVE TIME", "ABSOLUTE TIME"}; EXPECT_EQ(stream.GetHeader(), expected_header); - std::vector expected_rows{"* Produce {n}", "* Filter", "* ScanAll (n)", "* Once"}; + std::vector expected_rows{"* Produce {n}", "* Filter {n.id}", "* ScanAll (n)", "* Once"}; ASSERT_EQ(stream.GetResults().size(), expected_rows.size()); auto expected_it = expected_rows.begin(); for (const auto &row : stream.GetResults()) { diff --git a/tests/unit/query_common.hpp b/tests/unit/query_common.hpp index 69be6b490..e06d63908 100644 --- a/tests/unit/query_common.hpp +++ b/tests/unit/query_common.hpp @@ -533,6 +533,7 @@ auto GetForeach(AstStorage &storage, NamedExpression *named_expr, const std::vec this->storage.template Create( \ (memgraph::query::Expression *){map_variable}, \ std::unordered_map{elements}) +#define LABELS_TEST(expr, labels) this->storage.template Create(expr, labels) #define PROPERTY_PAIR(dba, property_name) std::make_pair(property_name, dba.NameToProperty(property_name)) #define PROPERTY_LOOKUP(dba, ...) memgraph::query::test_common::GetPropertyLookup(this->storage, dba, __VA_ARGS__) #define ALL_PROPERTIES_LOOKUP(expr) memgraph::query::test_common::GetAllPropertiesLookup(this->storage, expr) diff --git a/tests/unit/query_plan_operator_to_string.cpp b/tests/unit/query_plan_operator_to_string.cpp index 9d49033b8..a164d23b1 100644 --- a/tests/unit/query_plan_operator_to_string.cpp +++ b/tests/unit/query_plan_operator_to_string.cpp @@ -14,6 +14,7 @@ #include "disk_test_utils.hpp" #include "query/frontend/semantic/symbol_table.hpp" #include "query/plan/operator.hpp" +#include "query/plan/preprocess.hpp" #include "query/plan/pretty_print.hpp" #include "query_common.hpp" @@ -209,12 +210,38 @@ TYPED_TEST(OperatorToStringTest, ConstructNamedPath) { } TYPED_TEST(OperatorToStringTest, Filter) { - std::shared_ptr last_op = std::make_shared(nullptr, this->GetSymbol("node1")); - last_op = - std::make_shared(last_op, std::vector>{}, - EQ(PROPERTY_LOOKUP(this->dba, "node1", this->dba.NameToProperty("prop")), LITERAL(5))); + auto node = this->GetSymbol("person"); + auto node_ident = IDENT("person"); + auto property = this->dba.NameToProperty("name"); + auto property_ix = this->storage.GetPropertyIx("name"); - std::string expected_string{"Filter"}; + FilterInfo generic_filter_info = {.type = FilterInfo::Type::Generic, .used_symbols = {node}}; + + auto id_filter = IdFilter(this->symbol_table, node, LITERAL(42)); + FilterInfo id_filter_info = {.type = FilterInfo::Type::Id, .id_filter = id_filter}; + + std::vector labels{this->storage.GetLabelIx("Customer"), this->storage.GetLabelIx("Visitor")}; + auto labels_test = LABELS_TEST(node_ident, labels); + FilterInfo label_filter_info = {.type = FilterInfo::Type::Label, .expression = labels_test}; + + auto labels_test_2 = LABELS_TEST(PROPERTY_LOOKUP(this->dba, "person", property), labels); + FilterInfo label_filter_2_info = {.type = FilterInfo::Type::Label, .expression = labels_test_2}; + + auto property_filter = PropertyFilter(node, property_ix, PropertyFilter::Type::EQUAL); + FilterInfo property_filter_info = {.type = FilterInfo::Type::Property, .property_filter = property_filter}; + + FilterInfo pattern_filter_info = {.type = FilterInfo::Type::Pattern}; + + Filters filters; + filters.SetFilters({generic_filter_info, id_filter_info, label_filter_info, label_filter_2_info, property_filter_info, + pattern_filter_info}); + + std::shared_ptr last_op = std::make_shared(nullptr, node); + last_op = std::make_shared(last_op, std::vector>{}, + EQ(PROPERTY_LOOKUP(this->dba, "person", property), LITERAL(5)), filters); + + std::string expected_string{ + "Filter Generic {person}, id(person), (person :Customer:Visitor), (:Customer:Visitor), {person.name}, Pattern"}; EXPECT_EQ(last_op->ToString(), expected_string); }