From b1c3168308fa613824c1f424bd52443fe6dbf471 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ante=20Pu=C5=A1i=C4=87?= <ante.f.pusic@gmail.com>
Date: Sat, 28 Oct 2023 15:34:52 +0200
Subject: [PATCH] Fix PROFILE infinite loop (#1431)
---
src/query/plan/operator.cpp | 21 ++++++-----
tests/e2e/CMakeLists.txt | 1 +
tests/e2e/inspect_query/CMakeLists.txt | 6 +++
tests/e2e/inspect_query/common.py | 45 +++++++++++++++++++++++
tests/e2e/inspect_query/inspect_query.py | 47 ++++++++++++++++++++++++
tests/e2e/inspect_query/workloads.yaml | 13 +++++++
6 files changed, 123 insertions(+), 10 deletions(-)
create mode 100644 tests/e2e/inspect_query/CMakeLists.txt
create mode 100644 tests/e2e/inspect_query/common.py
create mode 100644 tests/e2e/inspect_query/inspect_query.py
create mode 100644 tests/e2e/inspect_query/workloads.yaml
diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp
index 3e0310b91..5b8f0981e 100644
--- a/src/query/plan/operator.cpp
+++ b/src/query/plan/operator.cpp
@@ -2456,17 +2456,16 @@ Filter::FilterCursor::FilterCursor(const Filter &self, utils::MemoryResource *me
bool Filter::FilterCursor::Pull(Frame &frame, ExecutionContext &context) {
OOMExceptionEnabler oom_exception;
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) || UNLIKELY(context.is_profile_query)) {
+ while (input_cursor_->Pull(frame, context)) {
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;
}
@@ -2790,13 +2789,15 @@ SetProperties::SetPropertiesCursor::SetPropertiesCursor(const SetProperties &sel
namespace {
template <typename T>
-concept AccessorWithProperties = requires(T value, storage::PropertyId property_id,
- storage::PropertyValue property_value,
- std::map<storage::PropertyId, storage::PropertyValue> properties) {
- { value.ClearProperties() } -> std::same_as<storage::Result<std::map<storage::PropertyId, storage::PropertyValue>>>;
- {value.SetProperty(property_id, property_value)};
- {value.UpdateProperties(properties)};
-};
+concept AccessorWithProperties =
+ requires(T value, storage::PropertyId property_id, storage::PropertyValue property_value,
+ std::map<storage::PropertyId, storage::PropertyValue> properties) {
+ {
+ value.ClearProperties()
+ } -> std::same_as<storage::Result<std::map<storage::PropertyId, storage::PropertyValue>>>;
+ { value.SetProperty(property_id, property_value) };
+ { value.UpdateProperties(properties) };
+ };
/// Helper function that sets the given values on either a Vertex or an Edge.
///
diff --git a/tests/e2e/CMakeLists.txt b/tests/e2e/CMakeLists.txt
index 9f67dcf77..7c7edb60e 100644
--- a/tests/e2e/CMakeLists.txt
+++ b/tests/e2e/CMakeLists.txt
@@ -69,6 +69,7 @@ add_subdirectory(transaction_rollback)
add_subdirectory(index_hints)
add_subdirectory(query_modules)
add_subdirectory(constraints)
+add_subdirectory(inspect_query)
copy_e2e_python_files(pytest_runner pytest_runner.sh "")
copy_e2e_python_files(x x.sh "")
diff --git a/tests/e2e/inspect_query/CMakeLists.txt b/tests/e2e/inspect_query/CMakeLists.txt
new file mode 100644
index 000000000..f0dbdb7cc
--- /dev/null
+++ b/tests/e2e/inspect_query/CMakeLists.txt
@@ -0,0 +1,6 @@
+function(copy_inspect_query_e2e_python_files FILE_NAME)
+ copy_e2e_python_files(inspect_query ${FILE_NAME})
+endfunction()
+
+copy_inspect_query_e2e_python_files(common.py)
+copy_inspect_query_e2e_python_files(inspect_query.py)
diff --git a/tests/e2e/inspect_query/common.py b/tests/e2e/inspect_query/common.py
new file mode 100644
index 000000000..870c80d83
--- /dev/null
+++ b/tests/e2e/inspect_query/common.py
@@ -0,0 +1,45 @@
+# Copyright 2023 Memgraph Ltd.
+#
+# Use of this software is governed by the Business Source License
+# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
+# License, and you may not use this file except in compliance with the Business Source License.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0, included in the file
+# licenses/APL.txt.
+
+import typing
+
+import mgclient
+import pytest
+from gqlalchemy import Memgraph
+
+
+def execute_and_fetch_all(cursor: mgclient.Cursor, query: str, params: dict = {}) -> typing.List[tuple]:
+ cursor.execute(query, params)
+ return cursor.fetchall()
+
+
+@pytest.fixture
+def connect(**kwargs) -> mgclient.Connection:
+ connection = mgclient.connect(host="localhost", port=7687, **kwargs)
+ connection.autocommit = True
+ cursor = connection.cursor()
+ execute_and_fetch_all(cursor, "USE DATABASE memgraph")
+ try:
+ execute_and_fetch_all(cursor, "DROP DATABASE clean")
+ except:
+ pass
+ execute_and_fetch_all(cursor, "MATCH (n) DETACH DELETE n")
+ yield connection
+
+
+@pytest.fixture
+def memgraph(**kwargs) -> Memgraph:
+ memgraph = Memgraph()
+
+ yield memgraph
+
+ memgraph.drop_database()
+ memgraph.drop_indexes()
diff --git a/tests/e2e/inspect_query/inspect_query.py b/tests/e2e/inspect_query/inspect_query.py
new file mode 100644
index 000000000..2e6aa491e
--- /dev/null
+++ b/tests/e2e/inspect_query/inspect_query.py
@@ -0,0 +1,47 @@
+# Copyright 2023 Memgraph Ltd.
+#
+# Use of this software is governed by the Business Source License
+# included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
+# License, and you may not use this file except in compliance with the Business Source License.
+#
+# As of the Change Date specified in that file, in accordance with
+# the Business Source License, use of this software will be governed
+# by the Apache License, Version 2.0, included in the file
+# licenses/APL.txt.
+
+import sys
+
+import mgclient
+import pytest
+from common import memgraph
+
+
+def test_label_index_hint(memgraph):
+ explain_query = "EXPLAIN MATCH (n:Node) RETURN n.property1;"
+ profile_query = "PROFILE MATCH (n:Node) RETURN n.property1;"
+
+ expected_profile = ["* Produce {n.property1}", "* Filter (n :Node)", "* ScanAll (n)", "* Once"]
+ expected_explain = [" * Produce {n.property1}", " * Filter (n :Node)", " * ScanAll (n)", " * Once"]
+
+ explain_empty = [row["QUERY PLAN"] for row in memgraph.execute_and_fetch(explain_query)]
+ profile_empty = [row["OPERATOR"] for row in memgraph.execute_and_fetch(profile_query)]
+
+ assert explain_empty == expected_explain and profile_empty == expected_profile
+
+ memgraph.execute("CREATE (n:WrongLabel {property1: 2});")
+
+ explain_none_matched = [row["QUERY PLAN"] for row in memgraph.execute_and_fetch(explain_query)]
+ profile_none_matched = [row["OPERATOR"] for row in memgraph.execute_and_fetch(profile_query)]
+
+ assert explain_none_matched == expected_explain and profile_none_matched == expected_profile
+
+ memgraph.execute("CREATE (n:Node {property1: 2});")
+
+ explain_has_match = [row["QUERY PLAN"] for row in memgraph.execute_and_fetch(explain_query)]
+ profile_has_match = [row["OPERATOR"] for row in memgraph.execute_and_fetch(profile_query)]
+
+ assert explain_has_match == expected_explain and profile_has_match == expected_profile
+
+
+if __name__ == "__main__":
+ sys.exit(pytest.main([__file__, "-rA"]))
diff --git a/tests/e2e/inspect_query/workloads.yaml b/tests/e2e/inspect_query/workloads.yaml
new file mode 100644
index 000000000..c0e8d6688
--- /dev/null
+++ b/tests/e2e/inspect_query/workloads.yaml
@@ -0,0 +1,13 @@
+inspect_query_cluster: &inspect_query_cluster
+ cluster:
+ main:
+ args: ["--bolt-port", "7687", "--log-level=TRACE"]
+ log_file: "inspect_query.log"
+ setup_queries: []
+ validation_queries: []
+
+workloads:
+ - name: "Verify that query inspection works"
+ binary: "tests/e2e/pytest_runner.sh"
+ args: ["inspect_query/inspect_query.py"]
+ <<: *inspect_query_cluster