Add `refactor.mergeNodes` procedure #623

Josipmrden · 2025-06-09T12:53:26Z

Add refactor.mergeNodes procedure

coderabbitai · 2025-06-09T12:53:32Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Josipmrden · 2025-06-16T09:41:45Z

as51340 · 2025-06-16T13:53:52Z

cpp/refactor_module/algorithm/refactor.cpp

+
+  try {
+    const auto nodes = arguments[0].ValueList();
+    const auto config = arguments[1].ValueMap();


create local variable when you use it for the first time

is it now fine?

as51340 · 2025-06-16T13:57:51Z

cpp/refactor_module/algorithm/refactor.cpp

+    bool mergeRels = false;
+    if (config.KeyExists(kMergeNodesRelationshipsStrategy)) {
+      if (!config.At(kMergeNodesRelationshipsStrategy).IsBool()) {
+        throw mgp::ValueException(std::string(kMergeRelationshipsInvalidValueError).c_str());
+      }
+
+      mergeRels = config.At(kMergeNodesRelationshipsStrategy).ValueBool();
+    }


Suggested change

bool mergeRels = false;

if (config.KeyExists(kMergeNodesRelationshipsStrategy)) {

if (!config.At(kMergeNodesRelationshipsStrategy).IsBool()) {

throw mgp::ValueException(std::string(kMergeRelationshipsInvalidValueError).c_str());

}

mergeRels = config.At(kMergeNodesRelationshipsStrategy).ValueBool();

}

if (config.KeyExists(kMergeNodesRelationshipsStrategy) && !config.At(kMergeNodesRelationshipsStrategy).IsBool()) {

throw mgp::ValueException(std::string(kMergeRelationshipsInvalidValueError).c_str());

}

bool mergeRels = config.At(kMergeNodesRelationshipsStrategy).ValueBool();

we didn't handle here the part where the value in config doesn't exist?

i didn't change anything here because of that reason, so just leaving for discussion of that edge case

you're right, use IIFE then to initialize boolean

as51340 · 2025-06-16T13:58:31Z

cpp/refactor_module/algorithm/refactor.cpp

+      auto source_node = nodes[i].ValueNode();
+
+      // Get properties strategy from config
+      std::string prop_strategy = std::string(kMergeNodesPropertiesCombine);


put that outside the loop to avoid creating strings all the time

as51340 · 2025-06-16T13:59:12Z

cpp/refactor_module/algorithm/refactor.cpp

+      if (config.KeyExists(kMergeNodesPropertiesStrategy)) {
+        prop_strategy = config.At(kMergeNodesPropertiesStrategy).ValueString();
+      }
+      if (config.KeyExists(kMergeNodesPropertiesStrategyAlternative)) {
+        prop_strategy = config.At(kMergeNodesPropertiesStrategyAlternative).ValueString();
+      }


this can also be outside the loop

as51340 · 2025-06-16T14:00:37Z

cpp/refactor_module/algorithm/refactor.cpp

+      std::transform(prop_strategy.begin(), prop_strategy.end(), prop_strategy.begin(), ::tolower);
+      // Validate property strategy


Use cpp std::transform(s.begin(), s.end(), s.begin(),
[](unsigned char c) { return std::tolower(c); });

as51340 · 2025-06-16T14:00:53Z

cpp/refactor_module/algorithm/refactor.cpp

+      if (prop_strategy != std::string(kMergeNodesPropertiesCombine) &&
+          prop_strategy != std::string(kMergeNodesPropertiesDiscard) &&
+          prop_strategy != std::string(kMergeNodesPropertiesOverride) &&
+          prop_strategy != std::string(kMergeNodesPropertiesOverwrite)) {
+        throw mgp::ValueException(std::string(kMergeNodesInvalidPropertyStrategyError).c_str());
+      }
+


out side the loop

as51340 · 2025-06-16T14:01:27Z

cpp/refactor_module/algorithm/refactor.cpp

+      }
+
+      // Handle properties based on strategy
+      if (prop_strategy == std::string(kMergeNodesPropertiesCombine)) {


string creation outside the loop

as51340 · 2025-06-16T14:02:12Z

cpp/refactor_module/algorithm/refactor.cpp

+            target_node.SetProperty(key, source_props[key]);
+          }
+        }
+      } else if (prop_strategy == std::string(kMergeNodesPropertiesDiscard)) {


string creation outside the loop

as51340 · 2025-06-16T14:02:33Z

cpp/refactor_module/algorithm/refactor.cpp

+      } else if (prop_strategy == std::string(kMergeNodesPropertiesOverride) ||
+                 prop_strategy == std::string(kMergeNodesPropertiesOverwrite)) {


string creation outside the loop

as51340 · 2025-06-17T11:58:59Z

cpp/refactor_module/algorithm/refactor.cpp

+    bool mergeRels = false;
+    if (config.KeyExists(kMergeNodesRelationshipsStrategy)) {
+      if (!config.At(kMergeNodesRelationshipsStrategy).IsBool()) {
+        throw mgp::ValueException(std::string(kMergeRelationshipsInvalidValueError).c_str());
+      }
+
+      mergeRels = config.At(kMergeNodesRelationshipsStrategy).ValueBool();
+    }


you're right, use IIFE then to initialize boolean

as51340 · 2025-06-17T12:00:50Z

cpp/refactor_module/algorithm/refactor.cpp

+        auto source_props = source_node.Properties();
+        for (const auto &[key, value] : source_props) {
+          if (!source_props.contains(key)) {
+            // nothing to combine
+            continue;
+          }


I think this is wrong. You are iterating over source_props key-value pairs and checking source_props contains key

as51340 · 2025-06-17T12:01:48Z

cpp/refactor_module/algorithm/refactor.cpp

+      } else if (prop_strategy == mergeNodesPropertiesDiscardString) {
+        // Keep only target node properties
+        // No action needed


No need to specify empty block, don't use else if then, write the comment about this case

as51340 · 2025-06-17T12:02:08Z

cpp/refactor_module/algorithm/refactor.cpp

+      // Copy labels from source to target
+      auto source_labels = source_node.Labels();
+      for (size_t j = 0; j < source_labels.Size(); ++j) {
+        target_node.AddLabel(source_labels[j]);


maybe you can move label?

sonarqubecloud · 2025-06-18T10:47:15Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Add merge nodes procedure

f8eb271

Josipmrden added 4 commits June 16, 2025 11:04

Refactor tests for base level

6912496

Add test with .*

9cb40b0

Add more tests

34f0a29

Rename tests

da4ea92

Josipmrden changed the title ~~Add merge nodes procedure~~ Add refactor.mergeNodes procedure Jun 16, 2025

Josipmrden requested review from as51340 and DavIvek June 16, 2025 09:40

Josipmrden self-assigned this Jun 16, 2025

Josipmrden added apoc feature feature customer customer labels Jun 16, 2025

Josipmrden added this to the mage-v3.4.0 milestone Jun 16, 2025

Josipmrden added the Docs needed Docs needed label Jun 16, 2025

Merge branch 'main' into refactor-merge-nodes

9da869e

Josipmrden marked this pull request as ready for review June 16, 2025 09:42

Josipmrden mentioned this pull request Jun 16, 2025

Add refactor.mergeNodes query module memgraph/documentation#1322

Open

11 tasks

as51340 requested changes Jun 16, 2025

View reviewed changes

Josipmrden added 2 commits June 16, 2025 17:17

Address PR comments

56c5581

Add const

cddc074

Josipmrden requested a review from as51340 June 17, 2025 08:00

as51340 requested changes Jun 17, 2025

View reviewed changes

Josipmrden added 2 commits June 17, 2025 16:45

IIFE

e1d5042

Address PR comments

0a33364

Josipmrden requested a review from as51340 June 17, 2025 14:49

Handle additional condition in IIFE

95b8a24

matea16 mentioned this pull request Jun 18, 2025

Memgraph 3.4 memgraph/documentation#1308

Open

4 tasks

Merge branch 'main' into refactor-merge-nodes

6b4acb5

Add move to label

72a582d

		std::transform(prop_strategy.begin(), prop_strategy.end(), prop_strategy.begin(), ::tolower);
		// Validate property strategy

		} else if (prop_strategy == std::string(kMergeNodesPropertiesOverride) \|\|
		prop_strategy == std::string(kMergeNodesPropertiesOverwrite)) {

Add refactor.mergeNodes procedure #623

Are you sure you want to change the base?

Add refactor.mergeNodes procedure #623

Uh oh!

Conversation

Josipmrden commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Jun 9, 2025

Review skipped

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Josipmrden commented Jun 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request type

Related issues

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

Documentation checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 18, 2025

Quality Gate passed

Uh oh!

Uh oh!

Add `refactor.mergeNodes` procedure #623

Add `refactor.mergeNodes` procedure #623

Josipmrden commented Jun 9, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

Josipmrden commented Jun 16, 2025 •

edited

Loading