Skip to content

Fixes #3244 : label filtering in Cypher MATCH relationship patterns #3252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lvca merged 3 commits into from
Jan 27, 2026
+389 −20
Merged

Fixes #3244 : label filtering in Cypher MATCH relationship patterns #3252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
324f220
fix: Cypher target node label filtering and bound variable tracking
claude Jan 27, 2026
5f468a9
fix: use JUnit assertions instead of AssertJ to avoid ambiguous overload
claude Jan 27, 2026
1fc22ba
Merge branch 'ArcadeData:main' into claude/fix-arcadedb-cypher-8z1Ym
ExtReMLapin Jan 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 55 additions & 8 deletions engine/src/main/java/com/arcadedb/query/opencypher/executor/CypherExecutionPlan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,10 @@ private AbstractExecutionStep buildExecutionStepsWithOrder(final CommandContext
// Get function factory from evaluator for steps that need it
final CypherFunctionFactory functionFactory = expressionEvaluator != null ? expressionEvaluator.getFunctionFactory() : null;

// Track variables bound across MATCH clauses so subsequent MATCHes
// can detect already-bound variables and avoid Cartesian products
final Set<String> boundVariables = new HashSet<>();

// OPTIMIZATION: Check for simple COUNT(*) pattern that can use Type.count() O(1) operation
// Pattern: MATCH (a:TypeName) RETURN COUNT(a) as alias
final AbstractExecutionStep typeCountStep = tryCreateTypeCountOptimization(context);
Expand Down Expand Up @@ -625,7 +629,7 @@ public String prettyPrint(final int depth, final int indent) {

case MATCH:
final MatchClause matchClause = entry.getTypedClause();
currentStep = buildMatchStep(matchClause, currentStep, context);
currentStep = buildMatchStep(matchClause, currentStep, context, boundVariables);
break;

case WITH:
Expand Down Expand Up @@ -801,9 +805,23 @@ private AbstractExecutionStep buildWithStep(final WithClause withClause,

/**
* Builds execution step for a MATCH clause.
* Backward-compatible overload without bound variable tracking.
*/
private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, AbstractExecutionStep currentStep,
final CommandContext context) {
return buildMatchStep(matchClause, currentStep, context, new HashSet<>());
}

/**
* Builds execution step for a MATCH clause with bound variable tracking.
*
* @param matchClause the MATCH clause to build
* @param currentStep current step in the execution chain
* @param context command context
* @param boundVariables set of variable names already bound in previous steps (updated in-place)
*/
private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, AbstractExecutionStep currentStep,
final CommandContext context, final Set<String> boundVariables) {
if (!matchClause.hasPathPatterns()) {
return currentStep;
}
Expand All @@ -826,6 +844,13 @@ private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, Abst
final String variable = nodePattern.getVariable() != null ? nodePattern.getVariable() : ("n" + patternIndex);
matchVariables.add(variable);

// Check if this variable was already bound in a previous MATCH clause
if (boundVariables.contains(variable)) {
// Variable already bound - skip creating a new MatchNodeStep
// The bound value will be used from the input result
continue;
}

// OPTIMIZATION: Extract ID filter for this variable to avoid Cartesian product
final String idFilter = extractIdFilter(whereClause, variable);
final MatchNodeStep matchStep = new MatchNodeStep(variable, nodePattern, context, idFilter);
Expand All @@ -848,8 +873,11 @@ private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, Abst
final NodePattern sourceNode = pathPattern.getFirstNode();
final String sourceVar = sourceNode.getVariable() != null ? sourceNode.getVariable() : "a";

// Check if source node variable is already bound (either from previous MATCH or
// from being in the boundVariables set). Previously this only checked for
// unlabeled/unpropertied nodes, which broke when labels were repeated.
final boolean sourceAlreadyBound = stepBeforeMatch != null &&
!sourceNode.hasLabels() && !sourceNode.hasProperties();
(boundVariables.contains(sourceVar) || (!sourceNode.hasLabels() && !sourceNode.hasProperties()));

if (!sourceAlreadyBound) {
matchVariables.add(sourceVar);
Expand Down Expand Up @@ -898,7 +926,10 @@ private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, Abst
if (relPattern.isVariableLength()) {
nextStep = new ExpandPathStep(sourceVar, pathVariable, targetVar, relPattern, context);
} else {
nextStep = new MatchRelationshipStep(sourceVar, relVar, targetVar, relPattern, pathVariable, context);
// Pass target node pattern for label filtering and bound variables
// for identity checking on already-bound target variables
nextStep = new MatchRelationshipStep(sourceVar, relVar, targetVar, relPattern, pathVariable,
targetNode, boundVariables, context);
}

if (isOptional && matchChainStart == null) {
Expand Down Expand Up @@ -942,6 +973,9 @@ private AbstractExecutionStep buildMatchStep(final MatchClause matchClause, Abst
currentStep = optionalStep;
}

// Update bound variables with newly bound variables from this MATCH
boundVariables.addAll(matchVariables);

return currentStep;
}

Expand Down Expand Up @@ -988,6 +1022,10 @@ public String prettyPrint(final int depth, final int indent) {
};
}

// Track variables bound across MATCH clauses so subsequent MATCHes
// can detect already-bound variables and avoid Cartesian products
final Set<String> legacyBoundVariables = new HashSet<>();

// Step 1: MATCH clauses - fetch nodes
// Process ALL MATCH clauses (not just the first)
if (!statement.getMatchClauses().isEmpty()) {
Expand Down Expand Up @@ -1015,6 +1053,12 @@ public String prettyPrint(final int depth, final int indent) {
final String variable = nodePattern.getVariable() != null ? nodePattern.getVariable() : ("n" + patternIndex);
matchVariables.add(variable); // Track variable for OPTIONAL MATCH

// Check if this variable was already bound in a previous MATCH clause
if (legacyBoundVariables.contains(variable)) {
// Variable already bound - skip creating a new MatchNodeStep
continue;
}

// OPTIMIZATION: Extract ID filter from WHERE clause (if present) for pushdown
final WhereClause matchWhere = matchClause.hasWhereClause() ? matchClause.getWhereClause() : statement.getWhereClause();
final String idFilter = extractIdFilter(matchWhere, variable);
Expand Down Expand Up @@ -1042,10 +1086,9 @@ public String prettyPrint(final int depth, final int indent) {
final String sourceVar = sourceNode.getVariable() != null ? sourceNode.getVariable() : "a";

// Check if source node is already bound (for multiple MATCH clauses or OPTIONAL MATCH)
// If the source node has no labels/properties and there's a previous step,
// it's likely referring to an already-bound variable - skip creating MatchNodeStep
// Check both legacy bound variables AND the old heuristic (no labels/properties)
final boolean sourceAlreadyBound = stepBeforeMatch != null &&
!sourceNode.hasLabels() && !sourceNode.hasProperties();
(legacyBoundVariables.contains(sourceVar) || (!sourceNode.hasLabels() && !sourceNode.hasProperties()));

if (!sourceAlreadyBound) {
// Only track the source variable if we're creating a new binding for it
Expand Down Expand Up @@ -1109,8 +1152,9 @@ public String prettyPrint(final int depth, final int indent) {
// Variable-length path - pass path variable for named path support
nextStep = new ExpandPathStep(sourceVar, pathVariable, targetVar, relPattern, context);
} else {
// Fixed-length relationship - pass path variable
nextStep = new MatchRelationshipStep(sourceVar, relVar, targetVar, relPattern, pathVariable, context);
// Fixed-length relationship - pass path variable, target node pattern, and bound variables
nextStep = new MatchRelationshipStep(sourceVar, relVar, targetVar, relPattern, pathVariable,
targetNode, legacyBoundVariables, context);
}

// Chain the relationship step
Expand Down Expand Up @@ -1168,6 +1212,9 @@ public String prettyPrint(final int depth, final int indent) {
// The output of OptionalMatchStep becomes currentStep
currentStep = optionalStep;
}

// Update bound variables with newly bound variables from this MATCH
legacyBoundVariables.addAll(matchVariables);
} else {
// Phase 1: Use raw pattern string - create a simple stub
final ResultInternal stubResult = new ResultInternal();
Expand Down
61 changes: 60 additions & 1 deletion engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.arcadedb.graph.Edge;
import com.arcadedb.graph.Vertex;
import com.arcadedb.query.opencypher.ast.Direction;
import com.arcadedb.query.opencypher.ast.NodePattern;
import com.arcadedb.query.opencypher.ast.RelationshipPattern;
import com.arcadedb.query.opencypher.traversal.TraversalPath;
import com.arcadedb.query.sql.executor.AbstractExecutionStep;
Expand All @@ -34,6 +35,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.NoSuchElementException;
import java.util.Set;

/**
* Execution step for matching relationship patterns.
Expand All @@ -50,6 +52,8 @@ public class MatchRelationshipStep extends AbstractExecutionStep {
private final String targetVariable;
private final RelationshipPattern pattern;
private final String pathVariable;
private final NodePattern targetNodePattern;
private final Set<String> boundVariableNames;

/**
* Creates a match relationship step.
Expand Down Expand Up @@ -77,12 +81,32 @@ public MatchRelationshipStep(final String sourceVariable, final String relations
*/
public MatchRelationshipStep(final String sourceVariable, final String relationshipVariable, final String targetVariable,
final RelationshipPattern pattern, final String pathVariable, final CommandContext context) {
this(sourceVariable, relationshipVariable, targetVariable, pattern, pathVariable, null, null, context);
}

/**
* Creates a match relationship step with target node filtering and bound variable awareness.
*
* @param sourceVariable variable name for source vertex
* @param relationshipVariable variable name for relationship (can be null)
* @param targetVariable variable name for target vertex
* @param pattern relationship pattern to match
* @param pathVariable path variable name (e.g., p in p = (a)-[r]->(b)), can be null
* @param targetNodePattern target node pattern for label filtering (can be null)
* @param boundVariableNames set of variable names already bound in previous steps (can be null)
* @param context command context
*/
public MatchRelationshipStep(final String sourceVariable, final String relationshipVariable, final String targetVariable,
final RelationshipPattern pattern, final String pathVariable, final NodePattern targetNodePattern,
final Set<String> boundVariableNames, final CommandContext context) {
super(context);
this.sourceVariable = sourceVariable;
this.relationshipVariable = relationshipVariable;
this.targetVariable = targetVariable;
this.pattern = pattern;
this.pathVariable = pathVariable;
this.targetNodePattern = targetNodePattern;
this.boundVariableNames = boundVariableNames;
}

@Override
Expand Down Expand Up @@ -130,11 +154,29 @@ private void fetchMore(final int n) {
final Edge edge = currentEdges.next();
final Vertex targetVertex = getTargetVertex(edge, (Vertex) lastResult.getProperty(sourceVariable));

// Filter by target type if specified
// Filter by edge type if specified
if (pattern.hasTypes() && !matchesEdgeType(edge)) {
continue;
}

// Filter by target node label if specified in the pattern
if (targetNodePattern != null && targetNodePattern.hasLabels()) {
if (!matchesTargetLabel(targetVertex)) {
continue;
}
}

// If the target variable is already bound from a previous step,
// verify the traversed vertex matches the bound value (identity check)
if (boundVariableNames != null && boundVariableNames.contains(targetVariable)) {
final Object boundValue = lastResult.getProperty(targetVariable);
if (boundValue instanceof Vertex) {
if (!((Vertex) boundValue).getIdentity().equals(targetVertex.getIdentity())) {
continue;
}
}
}

// Create result with edge and target vertex
final ResultInternal result = new ResultInternal();

Expand Down Expand Up @@ -232,6 +274,23 @@ private Vertex getTargetVertex(final Edge edge, final Vertex sourceVertex) {
}
}

/**
* Checks if a target vertex matches the label constraints from the target node pattern.
*/
private boolean matchesTargetLabel(final Vertex vertex) {
if (targetNodePattern == null || !targetNodePattern.hasLabels()) {
return true;
}

final String vertexType = vertex.getTypeName();
for (final String label : targetNodePattern.getLabels()) {
if (label.equals(vertexType)) {
return true;
}
}
return false;
}

/**
* Checks if an edge matches the type filter.
*/
Expand Down
Loading
Loading