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

* fix: Cypher target node label filtering and bound variable tracking

Two bugs caused incorrect results in multi-MATCH/OPTIONAL MATCH Cypher queries:

1. MatchRelationshipStep never filtered target vertices by label. A pattern
   like (a:CHUNK)<-[r:in]-(b:NER) would return ANY connected vertex as b,
   not just NER vertices. Fixed by passing the target NodePattern and checking
   vertex type against label constraints.

2. Already-bound variables with labels were treated as unbound. The
   sourceAlreadyBound check required !sourceNode.hasLabels(), so writing
   (searchedChunk:CHUNK) in a subsequent MATCH created a full type scan
   instead of using the already-bound value. Fixed by tracking bound
   variable names across MATCH clauses.

Also adds identity checking: when a target variable is already bound from a
previous step, the traversed vertex must match the bound vertex identity.

https://claude.ai/code/session_017hhsQf7fzBvXcGSeg48hfP

* fix: use JUnit assertions instead of AssertJ to avoid ambiguous overload

AssertJ's assertThat() has ambiguous overloads for Object return types
from getProperty(). Switched to JUnit assertEquals/assertTrue/assertFalse.

https://claude.ai/code/session_017hhsQf7fzBvXcGSeg48hfP

---------

Co-authored-by: Claude <[email protected]>
(cherry picked from commit def0349)

Author: 2 people

engine/src/main/java/com/arcadedb/query/opencypher/executor/CypherExecutionPlan.java

@@ -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);
@@ -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:
@@ -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;
     }
@@ -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);
@@ -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);
@@ -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) {
@@ -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;
   }
 
@@ -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()) {
@@ -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);
@@ -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
@@ -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
@@ -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();

engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java

@@ -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;
@@ -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.
@@ -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.
@@ -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
@@ -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();
 
@@ -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.
    */