From 454e55e091695e598faed33e7f17b0199795a5f3 Mon Sep 17 00:00:00 2001 From: Andi Skrgat Date: Mon, 30 Oct 2023 12:35:10 +0100 Subject: [PATCH 1/3] Add failing unit test --- tests/unit/interpreter.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index 0779adb41d..e7b3c2d87f 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -1268,6 +1268,36 @@ TYPED_TEST(InterpreterTest, ExecutionStatsValues) { } } +TYPED_TEST(InterpreterTest, ExecutionStatsValuesPropertiesSet) { + { + auto [stream, qid] = this->Prepare( + "CREATE (u:Employee {Uid: 'EMP_AAAAA', FirstName: 'Bong', LastName: 'Revilla'}) RETURN u.name AS name;"); + this->Pull(&stream); + auto stats = stream.GetSummary().at("stats").ValueMap(); + } + { + auto [stream, gid] = this->Prepare("MATCH (node:Employee) SET node.FirstName = 'Andi'"); + this->Pull(&stream); + auto stats = stream.GetSummary().at("stats").ValueMap(); + for (const auto &[key, value] : stats) { + spdlog::info("{}", key); + } + } + + { + auto [stream, qid] = this->Prepare( + "MATCH (node:Employee) WHERE node.Uid='EMP_AAAAA' SET node={FirstName: 'James', LastName: 'Revilla', Uid: " + "'EMP_AAAAA', CreatedOn: 'null', CreatedBy: 'null', LastModifiedOn: '1698226931701', LastModifiedBy: 'null', " + "Description: 'null'};"); + this->Pull(&stream); + auto stats = stream.GetSummary().at("stats").ValueMap(); + // ASSERT_EQ(stats["properties-set"].ValueInt(), 8); + for (const auto &[key, value] : stats) { + spdlog::info("{}", key); + } + } +} + TYPED_TEST(InterpreterTest, NotificationsValidStructure) { { auto [stream, qid] = this->Prepare("MATCH (n) DELETE n;"); From 982298e5364739a5ec656ac16ffc31cfce52a97d Mon Sep 17 00:00:00 2001 From: Andi Skrgat Date: Mon, 6 Nov 2023 10:43:24 +0100 Subject: [PATCH 2/3] Add fix --- src/query/plan/operator.cpp | 1 + tests/unit/interpreter.cpp | 15 +-------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 68d894d344..45957fac36 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -2896,6 +2896,7 @@ void SetPropertiesOnRecord(TRecordAccessor *record, const TypedValue &rhs, SetPr auto update_props = [&, record](PropertiesMap &new_properties) { auto updated_properties = UpdatePropertiesChecked(record, new_properties); + context->execution_stats[ExecutionStats::Key::UPDATED_PROPERTIES] += new_properties.size(); if (should_register_change) { for (const auto &[id, old_value, new_value] : updated_properties) { diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index e7b3c2d87f..f8f1da0ac9 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -1273,17 +1273,7 @@ TYPED_TEST(InterpreterTest, ExecutionStatsValuesPropertiesSet) { auto [stream, qid] = this->Prepare( "CREATE (u:Employee {Uid: 'EMP_AAAAA', FirstName: 'Bong', LastName: 'Revilla'}) RETURN u.name AS name;"); this->Pull(&stream); - auto stats = stream.GetSummary().at("stats").ValueMap(); - } - { - auto [stream, gid] = this->Prepare("MATCH (node:Employee) SET node.FirstName = 'Andi'"); - this->Pull(&stream); - auto stats = stream.GetSummary().at("stats").ValueMap(); - for (const auto &[key, value] : stats) { - spdlog::info("{}", key); - } } - { auto [stream, qid] = this->Prepare( "MATCH (node:Employee) WHERE node.Uid='EMP_AAAAA' SET node={FirstName: 'James', LastName: 'Revilla', Uid: " @@ -1291,10 +1281,7 @@ TYPED_TEST(InterpreterTest, ExecutionStatsValuesPropertiesSet) { "Description: 'null'};"); this->Pull(&stream); auto stats = stream.GetSummary().at("stats").ValueMap(); - // ASSERT_EQ(stats["properties-set"].ValueInt(), 8); - for (const auto &[key, value] : stats) { - spdlog::info("{}", key); - } + ASSERT_EQ(stats["properties-set"].ValueInt(), 8); } } From 4e28b9e5558f44ce19e14d0cef415516f1a11e59 Mon Sep 17 00:00:00 2001 From: Andi Skrgat Date: Mon, 6 Nov 2023 13:55:27 +0100 Subject: [PATCH 3/3] Address PR comments --- src/query/plan/operator.cpp | 1 + tests/unit/interpreter.cpp | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 45957fac36..e4f96b6846 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -2896,6 +2896,7 @@ void SetPropertiesOnRecord(TRecordAccessor *record, const TypedValue &rhs, SetPr auto update_props = [&, record](PropertiesMap &new_properties) { auto updated_properties = UpdatePropertiesChecked(record, new_properties); + // NOLINTNEXTLINE(bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions) context->execution_stats[ExecutionStats::Key::UPDATED_PROPERTIES] += new_properties.size(); if (should_register_change) { diff --git a/tests/unit/interpreter.cpp b/tests/unit/interpreter.cpp index f8f1da0ac9..ca05290794 100644 --- a/tests/unit/interpreter.cpp +++ b/tests/unit/interpreter.cpp @@ -27,6 +27,7 @@ #include "query/exceptions.hpp" #include "query/interpreter.hpp" #include "query/interpreter_context.hpp" +#include "query/metadata.hpp" #include "query/stream.hpp" #include "query/typed_value.hpp" #include "query_common.hpp" @@ -1281,7 +1282,8 @@ TYPED_TEST(InterpreterTest, ExecutionStatsValuesPropertiesSet) { "Description: 'null'};"); this->Pull(&stream); auto stats = stream.GetSummary().at("stats").ValueMap(); - ASSERT_EQ(stats["properties-set"].ValueInt(), 8); + auto key = memgraph::query::ExecutionStatsKeyToString(memgraph::query::ExecutionStats::Key::UPDATED_PROPERTIES); + ASSERT_EQ(stats[key].ValueInt(), 8); } }