Skip to content

Commit c04d12f

Browse files
committed
fix: cypher traversing considering unidirectional edges
Fixed issue ArcadeData#3711 The OpenCypher optimizer's anchor selection picks the node with lowest cardinality as the starting point. For MATCH (p:Person)-[:WORKS_FOR]->(c:Company), if Company has fewer vertices, the optimizer picks Company as anchor and reverses the traversal direction to IN (incoming). However, WORKS_FOR was created as UNIDIRECTIONAL, meaning edges are only stored on the source vertex's outgoing edge list (Person), not on the target's incoming list (Company). So traversing IN from Company finds nothing → 0 results. OPTIONAL MATCH worked because it uses a different code path: it first scans all Person vertices (mandatory MATCH), then for each person expands outgoing WORKS_FOR edges (always in the correct OUT direction).
1 parent d23d8f3 commit c04d12f

2 files changed

Lines changed: 161 additions & 1 deletion

File tree

engine/src/main/java/com/arcadedb/query/opencypher/optimizer/CypherOptimizer.java

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import com.arcadedb.query.opencypher.optimizer.rules.*;
4141
import com.arcadedb.query.opencypher.optimizer.statistics.CostModel;
4242
import com.arcadedb.query.opencypher.optimizer.statistics.StatisticsProvider;
43+
import com.arcadedb.schema.EdgeType;
4344
import com.arcadedb.query.sql.executor.CommandContext;
4445
import com.arcadedb.query.sql.executor.Result;
4546

@@ -131,7 +132,14 @@ public PhysicalPlan optimize() {
131132
}
132133

133134
// 3. Select anchor node (best starting point)
134-
final AnchorSelection anchor = anchorSelector.selectAnchor(logicalPlan);
135+
AnchorSelection anchor = anchorSelector.selectAnchor(logicalPlan);
136+
137+
// 3a. Validate anchor against UNIDIRECTIONAL edge constraints.
138+
// UNIDIRECTIONAL edges only store outgoing links on the source vertex,
139+
// so reverse traversal (incoming from target) finds nothing. If the selected
140+
// anchor would force reverse traversal of a UNIDIRECTIONAL edge, re-select
141+
// the source side as anchor instead.
142+
anchor = validateAnchorForUnidirectionalEdges(anchor, logicalPlan);
135143

136144
// 4. Create anchor operator (index seek or scan)
137145
final PhysicalOperator anchorOperator = createAnchorOperator(anchor);
@@ -244,6 +252,40 @@ private List<String> extractTypeNames(final LogicalPlan plan) {
244252
return typeNames;
245253
}
246254

255+
/**
256+
* Validates the selected anchor against UNIDIRECTIONAL edge constraints.
257+
* If the anchor is on the target side of a directed UNIDIRECTIONAL edge,
258+
* the expand operator would need to traverse incoming edges which don't exist.
259+
* In that case, force the source side as the anchor instead.
260+
*/
261+
private AnchorSelection validateAnchorForUnidirectionalEdges(final AnchorSelection anchor,
262+
final LogicalPlan logicalPlan) {
263+
final String anchorVar = anchor.getVariable();
264+
final var schema = database.getSchema();
265+
266+
for (final LogicalRelationship rel : logicalPlan.getRelationships()) {
267+
// Check if the anchor is the target of an OUT relationship (would need IN traversal)
268+
// or the source of an IN relationship (would need OUT traversal from the other side)
269+
final boolean anchorIsTarget = anchorVar.equals(rel.getTargetVariable()) && rel.getDirection() == Direction.OUT;
270+
final boolean anchorIsSource = anchorVar.equals(rel.getSourceVariable()) && rel.getDirection() == Direction.IN;
271+
272+
if (anchorIsTarget || anchorIsSource) {
273+
// Check if any edge type in this relationship is UNIDIRECTIONAL
274+
for (final String edgeTypeName : rel.getTypes()) {
275+
if (schema.existsType(edgeTypeName) && schema.getType(edgeTypeName) instanceof EdgeType et && !et.isBidirectional()) {
276+
// Force the other side as the anchor
277+
final String correctAnchorVar = anchorIsTarget ? rel.getSourceVariable() : rel.getTargetVariable();
278+
final LogicalNode correctNode = logicalPlan.getNodes().get(correctAnchorVar);
279+
if (correctNode != null)
280+
return anchorSelector.evaluateNodeDirect(correctNode, logicalPlan);
281+
break;
282+
}
283+
}
284+
}
285+
}
286+
return anchor;
287+
}
288+
247289
/**
248290
* Creates the appropriate physical operator for the anchor node.
249291
*
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*
2+
* Copyright © 2021-present Arcade Data Ltd (info@arcadedata.com)
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*
16+
* SPDX-FileCopyrightText: 2021-present Arcade Data Ltd (info@arcadedata.com)
17+
* SPDX-License-Identifier: Apache-2.0
18+
*/
19+
package com.arcadedb.query.opencypher;
20+
21+
import com.arcadedb.TestHelper;
22+
import com.arcadedb.query.sql.executor.Result;
23+
import com.arcadedb.query.sql.executor.ResultSet;
24+
import org.junit.jupiter.api.Test;
25+
26+
import java.util.ArrayList;
27+
import java.util.List;
28+
29+
import static org.assertj.core.api.Assertions.assertThat;
30+
31+
/**
32+
* Regression test for GitHub issue #3711:
33+
* Mandatory MATCH over existing relationship returns 0 rows,
34+
* while OPTIONAL MATCH and SQL can see the same edges.
35+
*
36+
* @author Luca Garulli (l.garulli@arcadedata.com)
37+
*/
38+
class OpenCypherMandatoryMatchRelationshipTest extends TestHelper {
39+
40+
@Override
41+
protected void beginTest() {
42+
database.command("sql", "CREATE VERTEX TYPE Person");
43+
database.command("sql", "CREATE VERTEX TYPE Company");
44+
database.command("sql", "CREATE EDGE TYPE KNOWS UNIDIRECTIONAL");
45+
database.command("sql", "CREATE EDGE TYPE WORKS_FOR UNIDIRECTIONAL");
46+
47+
database.transaction(() -> {
48+
database.command("opencypher",
49+
"CREATE (a:Person {name: 'Alice', age: 30})" +
50+
"-[:KNOWS {since: 2020}]->" +
51+
"(b:Person {name: 'Bob', age: 35})" +
52+
"-[:KNOWS {since: 2021}]->" +
53+
"(c:Person {name: 'Charlie', age: 25})" +
54+
"-[:KNOWS {since: 2022}]->" +
55+
"(d:Person {name: 'David', age: 28})");
56+
57+
database.command("opencypher", "CREATE (:Company {name: 'Acme'})");
58+
59+
database.command("opencypher",
60+
"MATCH (a:Person {name: 'Alice'}), (b:Person {name: 'Bob'}), (c:Company {name: 'Acme'}) " +
61+
"CREATE (a)-[:WORKS_FOR]->(c), (b)-[:WORKS_FOR]->(c)");
62+
});
63+
}
64+
65+
@Test
66+
void mandatoryMatchRelationshipShouldReturnResults() {
67+
// Issue #3711: mandatory MATCH with relationship should return results when edges exist
68+
try (final ResultSet rs = database.query("opencypher",
69+
"MATCH (p:Person)-[:WORKS_FOR]->(c:Company) " +
70+
"RETURN p.name as person, c.name as company ORDER BY person")) {
71+
final List<Result> results = new ArrayList<>();
72+
while (rs.hasNext())
73+
results.add(rs.next());
74+
75+
assertThat(results).hasSize(2);
76+
assertThat(results.get(0).<String>getProperty("person")).isEqualTo("Alice");
77+
assertThat(results.get(0).<String>getProperty("company")).isEqualTo("Acme");
78+
assertThat(results.get(1).<String>getProperty("person")).isEqualTo("Bob");
79+
assertThat(results.get(1).<String>getProperty("company")).isEqualTo("Acme");
80+
}
81+
}
82+
83+
@Test
84+
void sqlCanSeeEdges() {
85+
// Verify SQL can see the WORKS_FOR edges
86+
try (final ResultSet rs = database.query("sql", "SELECT count(*) as c FROM WORKS_FOR")) {
87+
assertThat(rs.hasNext()).isTrue();
88+
assertThat(((Number) rs.next().getProperty("c")).intValue()).isEqualTo(2);
89+
}
90+
}
91+
92+
@Test
93+
void optionalMatchCanSeeEdges() {
94+
// Verify OPTIONAL MATCH can see the WORKS_FOR edges
95+
try (final ResultSet rs = database.query("opencypher",
96+
"MATCH (p:Person) " +
97+
"OPTIONAL MATCH (p)-[:WORKS_FOR]->(c:Company) " +
98+
"RETURN p.name as name, c.name as company ORDER BY name")) {
99+
final List<Result> results = new ArrayList<>();
100+
while (rs.hasNext())
101+
results.add(rs.next());
102+
103+
assertThat(results).hasSize(4);
104+
// Alice -> Acme
105+
assertThat(results.get(0).<String>getProperty("name")).isEqualTo("Alice");
106+
assertThat(results.get(0).<String>getProperty("company")).isEqualTo("Acme");
107+
// Bob -> Acme
108+
assertThat(results.get(1).<String>getProperty("name")).isEqualTo("Bob");
109+
assertThat(results.get(1).<String>getProperty("company")).isEqualTo("Acme");
110+
// Charlie -> null
111+
assertThat(results.get(2).<String>getProperty("name")).isEqualTo("Charlie");
112+
assertThat((Object) results.get(2).getProperty("company")).isNull();
113+
// David -> null
114+
assertThat(results.get(3).<String>getProperty("name")).isEqualTo("David");
115+
assertThat((Object) results.get(3).getProperty("company")).isNull();
116+
}
117+
}
118+
}

0 commit comments

Comments
 (0)