diff --git a/3951-inline-relationship-predicate.md b/3951-inline-relationship-predicate.md new file mode 100644 index 0000000000..eada7017bf --- /dev/null +++ b/3951-inline-relationship-predicate.md @@ -0,0 +1,49 @@ +# Fix: Inline relationship predicate ignored (#3951) + +## Root Cause + +ArcadeDB's OpenCypher engine was silently ignoring the `WHERE` clause written directly +inside a relationship bracket pattern, for example: + +```cypher +MATCH (a)-[r:KNOWS WHERE r.since < 2019]-(b) RETURN ... +``` + +The ANTLR grammar correctly defines the inline `WHERE expression` in `relationshipPattern`, +but three layers all failed to propagate it: + +1. **AST** - `RelationshipPattern` had no field for the WHERE predicate. +2. **Parser** - `CypherASTBuilder.visitRelationshipPattern()` never called `ctx.expression()`. +3. **Executor** - `MatchRelationshipStep` had no code path to evaluate such a predicate. + +## Changes + +### `RelationshipPattern.java` +- Added `whereExpression: BooleanExpression` field. +- Added `getWhereExpression()` and `hasWhereExpression()` accessors. +- Old two-arg and three-arg constructors delegate to the new four-arg constructor + (backward compatible). + +### `CypherASTBuilder.java` (`visitRelationshipPattern`) +- After extracting variable/types/properties, checks `ctx.expression()` and calls + the existing `parseBooleanExpression()` helper to produce a `BooleanExpression`. +- Passes it as the `whereExpression` argument to the new `RelationshipPattern` constructor. + +### `MatchRelationshipStep.java` (`processStandardPath`) +- Added `matchesEdgeWhereExpression(edge, lastResult)` helper that builds a temporary + `ResultInternal` with the relationship variable bound to the edge and calls + `whereExpression.evaluate(tempResult, context)`. +- The call is inserted right after the existing `matchesEdgeProperties` check, + so it short-circuits before heavier downstream processing. + +## Tests + +Added `OpenCypherPatternPredicateTest.InlineRelationshipPredicate` (3 methods): + +| Test | What it verifies | +|------|-----------------| +| `inlineWhereOnRelationshipIsApplied` | Undirected pattern - only 2018 edge returned | +| `inlineWhereOnRelationshipDirected` | Directed pattern - single row, Alice->Bob:2018 | +| `inlineWhereWithExternalWhereClause` | Inline predicate combined with outer WHERE | + +All 3 new tests pass. Full OpenCypher suite (5 377 tests) passes with no regressions. diff --git a/engine/src/main/java/com/arcadedb/query/opencypher/ast/RelationshipPattern.java b/engine/src/main/java/com/arcadedb/query/opencypher/ast/RelationshipPattern.java index 288391cc41..3a9f70cf57 100644 --- a/engine/src/main/java/com/arcadedb/query/opencypher/ast/RelationshipPattern.java +++ b/engine/src/main/java/com/arcadedb/query/opencypher/ast/RelationshipPattern.java @@ -39,15 +39,22 @@ public class RelationshipPattern implements PatternElement { private final String propertiesParameterName; private final Integer minHops; private final Integer maxHops; + private final BooleanExpression whereExpression; public RelationshipPattern(final String variable, final List types, final Direction direction, final Map properties, final Integer minHops, final Integer maxHops) { - this(variable, types, direction, properties, null, minHops, maxHops); + this(variable, types, direction, properties, null, minHops, maxHops, null); } public RelationshipPattern(final String variable, final List types, final Direction direction, final Map properties, final String propertiesParameterName, final Integer minHops, final Integer maxHops) { + this(variable, types, direction, properties, propertiesParameterName, minHops, maxHops, null); + } + + public RelationshipPattern(final String variable, final List types, final Direction direction, + final Map properties, final String propertiesParameterName, final Integer minHops, + final Integer maxHops, final BooleanExpression whereExpression) { this.variable = variable; this.types = types != null ? types : Collections.emptyList(); this.direction = direction != null ? direction : Direction.BOTH; @@ -55,6 +62,7 @@ public RelationshipPattern(final String variable, final List types, fina this.propertiesParameterName = propertiesParameterName; this.minHops = minHops; this.maxHops = maxHops; + this.whereExpression = whereExpression; } @Override @@ -143,6 +151,20 @@ public String getPropertiesParameterName() { return propertiesParameterName; } + /** + * Returns the inline WHERE predicate on the relationship pattern, or null if absent. + */ + public BooleanExpression getWhereExpression() { + return whereExpression; + } + + /** + * Returns true if this relationship pattern has an inline WHERE predicate. + */ + public boolean hasWhereExpression() { + return whereExpression != null; + } + /** * Returns the first type if present. * diff --git a/engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java b/engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java index 8e8f5b3ba8..0132ac832d 100644 --- a/engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java +++ b/engine/src/main/java/com/arcadedb/query/opencypher/executor/steps/MatchRelationshipStep.java @@ -466,6 +466,15 @@ private void processFastPath(final int n) { } private void processStandardPath(final int n) { + // Pre-populate once per source vertex; matchesEdgeWhereExpression only updates the edge binding per iteration + final ResultInternal whereEvalResult; + if (pattern.hasWhereExpression()) { + whereEvalResult = new ResultInternal(); + for (final String prop : lastResult.getPropertyNames()) + whereEvalResult.setProperty(prop, lastResult.getProperty(prop)); + } else + whereEvalResult = null; + while (currentEdges.hasNext() && buffer.size() < n) { final long begin = context.isProfiling() ? System.nanoTime() : 0; try { @@ -489,6 +498,10 @@ private void processStandardPath(final int n) { if (pattern.hasProperties() && !matchesEdgeProperties(edge)) continue; + // Filter by inline WHERE predicate on the relationship (e.g., [r:KNOWS WHERE r.since < 2019]) + if (whereEvalResult != null && !matchesEdgeWhereExpression(edge, whereEvalResult)) + continue; + // Relationship uniqueness: Cypher requires each relationship in a pattern // to be matched to a distinct edge (no edge traversed twice) if (isEdgeAlreadyUsed(lastResult, edge)) @@ -612,8 +625,8 @@ private boolean canUseFastPath(final Result result) { if (relationshipVariable != null && !relationshipVariable.isEmpty()) return false; - // 2. Edge property filter — GAV doesn't store edge properties - if (pattern.hasProperties()) + // 2. Edge property filter or inline WHERE — GAV doesn't store edge properties + if (pattern.hasProperties() || pattern.hasWhereExpression()) return false; // 3. Path variable — path reconstruction needs edge objects @@ -990,6 +1003,17 @@ else if ((s.startsWith("'") && s.endsWith("'")) || (s.startsWith("\"") && s.ends return true; } + /** + * Evaluates the inline WHERE predicate against a pre-populated result. + * The caller pre-populates {@code evalResult} with the current scope properties once + * per source vertex; this method only updates the relationship variable binding per edge. + */ + private boolean matchesEdgeWhereExpression(final Edge edge, final ResultInternal evalResult) { + if (relationshipVariable != null && !relationshipVariable.isEmpty()) + evalResult.setProperty(relationshipVariable, edge); + return pattern.getWhereExpression().evaluate(evalResult, context); + } + @Override public String prettyPrint(final int depth, final int indent) { final StringBuilder builder = new StringBuilder(); diff --git a/engine/src/main/java/com/arcadedb/query/opencypher/parser/CypherASTBuilder.java b/engine/src/main/java/com/arcadedb/query/opencypher/parser/CypherASTBuilder.java index 6b12666ea5..79629795e1 100644 --- a/engine/src/main/java/com/arcadedb/query/opencypher/parser/CypherASTBuilder.java +++ b/engine/src/main/java/com/arcadedb/query/opencypher/parser/CypherASTBuilder.java @@ -1553,7 +1553,13 @@ public RelationshipPattern visitRelationshipPattern(final Cypher25Parser.Relatio direction = Direction.BOTH; } - return new RelationshipPattern(variable, types, direction, properties, propertiesParameterName, minHops, maxHops); + // Inline WHERE predicate (e.g., [r:KNOWS WHERE r.since < 2019]) + BooleanExpression whereExpression = null; + if (ctx.expression() != null) + whereExpression = parseBooleanExpression(ctx.expression()); + + return new RelationshipPattern(variable, types, direction, properties, propertiesParameterName, minHops, maxHops, + whereExpression); } public Map visitProperties(final Cypher25Parser.PropertiesContext ctx) { diff --git a/engine/src/test/java/com/arcadedb/query/opencypher/OpenCypherPatternPredicateTest.java b/engine/src/test/java/com/arcadedb/query/opencypher/OpenCypherPatternPredicateTest.java index 94eb57442b..8b194ab417 100644 --- a/engine/src/test/java/com/arcadedb/query/opencypher/OpenCypherPatternPredicateTest.java +++ b/engine/src/test/java/com/arcadedb/query/opencypher/OpenCypherPatternPredicateTest.java @@ -27,9 +27,9 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import java.util.List; - +import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; @@ -440,4 +440,80 @@ WHERE NOT (p)-[:LIVING_IN]->(:Country {name: 'Germany'}) \ } } } + + @Nested + class InlineRelationshipPredicate { + private Database db; + + @BeforeEach + void setUp() { + db = new DatabaseFactory("./databases/test-inline-rel-predicate").create(); + db.getSchema().createVertexType("Person"); + db.getSchema().createEdgeType("KNOWS"); + db.command("opencypher", + """ + CREATE (a:Person {name: 'Alice'}), \ + (b:Person {name: 'Bob'}), \ + (c:Person {name: 'Charlie'}), \ + (a)-[:KNOWS {since: 2018}]->(b), \ + (a)-[:KNOWS {since: 2020}]->(c)"""); + } + + @AfterEach + void tearDown() { + if (db != null) + db.drop(); + } + + @Test + void inlineWhereOnRelationshipIsApplied() { + // Only the 2018 relationship must match in each direction + try (final ResultSet rs = db.query("opencypher", + """ + MATCH (a:Person)-[r:KNOWS WHERE r.since < 2019]-(b) \ + RETURN DISTINCT a.name AS person, b.name AS friend, r.since AS knowsSince \ + ORDER BY knowsSince, person, friend""")) { + final List rows = new ArrayList<>(); + while (rs.hasNext()) { + final Result row = rs.next(); + rows.add(row.getProperty("person") + "->" + row.getProperty("friend") + ":" + row.getProperty("knowsSince")); + } + assertThat(rows).containsExactlyInAnyOrder("Alice->Bob:2018", "Bob->Alice:2018"); + } + } + + @Test + void inlineWhereOnRelationshipDirected() { + // Directed pattern: only Alice->Bob (since=2018) must be returned + try (final ResultSet rs = db.query("opencypher", + """ + MATCH (a:Person)-[r:KNOWS WHERE r.since < 2019]->(b) \ + RETURN a.name AS person, b.name AS friend, r.since AS knowsSince""")) { + assertThat(rs.hasNext()).isTrue(); + final Result row = rs.next(); + assertThat((String) row.getProperty("person")).isEqualTo("Alice"); + assertThat((String) row.getProperty("friend")).isEqualTo("Bob"); + assertThat(((Number) row.getProperty("knowsSince")).intValue()).isEqualTo(2018); + assertThat(rs.hasNext()).isFalse(); + } + } + + @Test + void inlineWhereWithExternalWhereClause() { + // Combined: inline relationship predicate AND an outer WHERE clause + try (final ResultSet rs = db.query("opencypher", + """ + MATCH (a:Person)-[r:KNOWS WHERE r.since < 2019]-(b) \ + WHERE NOT (b:NonexistentLabel) \ + RETURN DISTINCT a.name AS person, b.name AS friend, r.since AS knowsSince \ + ORDER BY knowsSince""")) { + final List rows = new ArrayList<>(); + while (rs.hasNext()) { + final Result row = rs.next(); + rows.add(row.getProperty("person") + "->" + row.getProperty("friend") + ":" + row.getProperty("knowsSince")); + } + assertThat(rows).containsExactlyInAnyOrder("Alice->Bob:2018", "Bob->Alice:2018"); + } + } + } }