From 3c413a7e50536a29416d9bd4853447e20780f0bf Mon Sep 17 00:00:00 2001 From: Josipmrden Date: Sun, 12 Nov 2023 20:45:02 +0100 Subject: [PATCH] Fix hash join expression matching (#1496) --- src/query/plan/operator.cpp | 4 ++-- src/query/plan/rewrite/join.hpp | 8 ++++++++ .../tests/memgraph_V1/features/cartesian.feature | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/query/plan/operator.cpp b/src/query/plan/operator.cpp index 69748014e..792d278e8 100644 --- a/src/query/plan/operator.cpp +++ b/src/query/plan/operator.cpp @@ -5404,7 +5404,7 @@ class HashJoinCursor : public Cursor { // Check if the join value from the pulled frame is shared with any left frames ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, storage::View::OLD); - auto right_value = self_.hash_join_condition_->expression1_->Accept(evaluator); + auto right_value = self_.hash_join_condition_->expression2_->Accept(evaluator); if (hashtable_.contains(right_value)) { // If so, finish pulling for now and proceed to joining the pulled frame right_op_frame_.assign(frame.elems().begin(), frame.elems().end()); @@ -5452,7 +5452,7 @@ class HashJoinCursor : public Cursor { while (left_op_cursor_->Pull(frame, context)) { ExpressionEvaluator evaluator(&frame, context.symbol_table, context.evaluation_context, context.db_accessor, storage::View::OLD); - auto left_value = self_.hash_join_condition_->expression2_->Accept(evaluator); + auto left_value = self_.hash_join_condition_->expression1_->Accept(evaluator); if (left_value.type() != TypedValue::Type::Null) { hashtable_[left_value].emplace_back(frame.elems().begin(), frame.elems().end()); } diff --git a/src/query/plan/rewrite/join.hpp b/src/query/plan/rewrite/join.hpp index c16f5b60d..5ed2bb090 100644 --- a/src/query/plan/rewrite/join.hpp +++ b/src/query/plan/rewrite/join.hpp @@ -27,6 +27,7 @@ #include "query/plan/operator.hpp" #include "query/plan/preprocess.hpp" +#include "utils/algorithm.hpp" namespace memgraph::query::plan { @@ -523,6 +524,13 @@ class JoinRewriter final : public HierarchicalLogicalOperatorVisitor { auto rhs_property = rhs_lookup->property_; filter_exprs_for_removal_.insert(filter.expression); filters_.EraseFilter(filter); + + if (utils::Contains(right_symbols, lhs_symbol) && utils::Contains(left_symbols, rhs_symbol)) { + // We need to duplicate this because expressions are shared between plans + join_condition = join_condition->Clone(ast_storage_); + std::swap(join_condition->expression1_, join_condition->expression2_); + } + return std::make_unique(left_op, left_symbols, right_op, right_symbols, join_condition); } diff --git a/tests/gql_behave/tests/memgraph_V1/features/cartesian.feature b/tests/gql_behave/tests/memgraph_V1/features/cartesian.feature index 809a3d73a..b11d83f0e 100644 --- a/tests/gql_behave/tests/memgraph_V1/features/cartesian.feature +++ b/tests/gql_behave/tests/memgraph_V1/features/cartesian.feature @@ -186,6 +186,20 @@ Feature: Cartesian | a | b | | (:A {id: 1}) | (:B {id: 1}) | + Scenario: Multiple match with WHERE x = y 01 reversed + Given an empty graph + And having executed + """ + CREATE (:A {id: 1}), (:A {id: 2}), (:B {id: 1}) + """ + When executing query: + """ + MATCH (a:A) MATCH (b:B) WHERE b.id = a.id RETURN a, b + """ + Then the result should be: + | a | b | + | (:A {id: 1}) | (:B {id: 1}) | + Scenario: Multiple match with WHERE x = y 02 Given an empty graph And having executed