From 20a9885d83601d2952762084bbeaebc677c728cd Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Mon, 22 Jun 2026 16:42:52 +0200 Subject: [PATCH 1/2] Fix ALL() in rel comparison for recursive paths + defensive return instead of DASSERT From: https://github.com/predictable-labs/ryugraph/pull/2 --- src/binder/expression/node_rel_expression.cpp | 5 ++++- src/planner/operator/logical_distinct.cpp | 4 ++-- test/test_files/path/path.test | 8 ++++++++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/binder/expression/node_rel_expression.cpp b/src/binder/expression/node_rel_expression.cpp index 5780f4076c..0654954641 100644 --- a/src/binder/expression/node_rel_expression.cpp +++ b/src/binder/expression/node_rel_expression.cpp @@ -35,7 +35,10 @@ void NodeOrRelExpression::addEntries(const std::vector& entr void NodeOrRelExpression::addPropertyExpression(std::shared_ptr property) { auto propertyName = property->getPropertyName(); - DASSERT(!propertyNameToIdx.contains(propertyName)); + // Idempotent: skip duplicate property expressions + if (propertyNameToIdx.contains(propertyName)) { + return; + } propertyNameToIdx.insert({propertyName, propertyExprs.size()}); propertyExprs.push_back(std::move(property)); } diff --git a/src/planner/operator/logical_distinct.cpp b/src/planner/operator/logical_distinct.cpp index 3785fa49f2..5f6b024f57 100644 --- a/src/planner/operator/logical_distinct.cpp +++ b/src/planner/operator/logical_distinct.cpp @@ -10,7 +10,7 @@ void LogicalDistinct::computeFactorizedSchema() { createEmptySchema(); auto groupPos = schema->createGroup(); for (auto& expression : getKeysAndPayloads()) { - schema->insertToGroupAndScope(expression, groupPos); + schema->insertToGroupAndScopeMayRepeat(expression, groupPos); } } @@ -18,7 +18,7 @@ void LogicalDistinct::computeFlatSchema() { createEmptySchema(); schema->createGroup(); for (auto& expression : getKeysAndPayloads()) { - schema->insertToGroupAndScope(expression, 0); + schema->insertToGroupAndScopeMayRepeat(expression, 0); } } diff --git a/test/test_files/path/path.test b/test/test_files/path/path.test index e39fd8dfa1..b5f9539168 100644 --- a/test/test_files/path/path.test +++ b/test/test_files/path/path.test @@ -122,3 +122,11 @@ Expected: (LIST) -> INT64 (ARRAY) -> INT64 (MAP) -> INT64 (STRING) -> INT64 + +-CASE RecursiveRelComparisonBug + +-LOG RecursivePathWithAllAndRelComparison +# Bug: Segfault when using ALL() with relationship comparison in recursive path +-STATEMENT MATCH (a:person)-[t:knows]->(d:person) WHERE a.ID <> d.ID WITH a, d, t MATCH p = (a)-[:knows*1..3]->(d) WHERE length(p) >= 2 AND ALL (r IN relationships(p) WHERE r <> t) WITH DISTINCT t RETURN count(t); +---- 1 +12 \ No newline at end of file From 67d74d760131c419275db09ca7d0f7ed3e7f4b15 Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Mon, 22 Jun 2026 22:00:47 +0200 Subject: [PATCH 2/2] Simplify bindComparisonExpression Co-authored-by: Arun Sharma --- .../bind_comparison_expression.cpp | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/src/binder/bind_expression/bind_comparison_expression.cpp b/src/binder/bind_expression/bind_comparison_expression.cpp index d7697e8948..bbc6f66b19 100644 --- a/src/binder/bind_expression/bind_comparison_expression.cpp +++ b/src/binder/bind_expression/bind_comparison_expression.cpp @@ -27,7 +27,14 @@ std::shared_ptr ExpressionBinder::bindComparisonExpression( return bindComparisonExpression(parsedExpression.getExpressionType(), children); } -static bool isNodeOrRel(const Expression& expression) { +// A node or rel *pattern* expression (e.g. from a MATCH) has an internal ID property and can +// be rewritten to an internal-ID comparison. A plain variable holding a NODE/REL value (e.g. a +// lambda variable bound from `relationships(p)`) is a VariableExpression with a NODE/REL logical +// type but no internal ID, so it must go through the regular value comparison path. +static bool isNodeOrRelPattern(const Expression& expression) { + if (expression.expressionType != ExpressionType::PATTERN) { + return false; + } switch (expression.getDataType().getLogicalTypeID()) { case LogicalTypeID::NODE: case LogicalTypeID::REL: @@ -41,20 +48,11 @@ std::shared_ptr ExpressionBinder::bindComparisonExpression( ExpressionType expressionType, const expression_vector& children) { // Rewrite node or rel comparison DASSERT(children.size() == 2); - if (isNodeOrRel(*children[0]) && isNodeOrRel(*children[1])) { + if (isNodeOrRelPattern(*children[0]) && isNodeOrRelPattern(*children[1])) { expression_vector newChildren; - // For pattern expressions (NODE/REL), use getInternalID() directly - // For non-pattern expressions (VARIABLE, FUNCTION, etc.), extract the _ID field + newChildren.reserve(children.size()); for (const auto& child : children) { - if (child->expressionType == ExpressionType::PATTERN) { - newChildren.push_back(child->constCast().getInternalID()); - } else { - expression_vector extractChildren; - extractChildren.push_back(child); - extractChildren.push_back(createLiteralExpression(InternalKeyword::ID)); - newChildren.push_back( - bindScalarFunctionExpression(extractChildren, StructExtractFunctions::name)); - } + newChildren.push_back(child->constCast().getInternalID()); } return bindComparisonExpression(expressionType, newChildren); }