Skip to content

Commit 118e557

Browse files
Mats-SXDarthMaxsoerenreichardt
committed
Throw GQL compliant exceptions in Cypher projection
The @procedure, @userfunction, and @UserAggregatingFunction annotations provide compiled try-catch clauses for exceptions. When we use the CallableUserAggregationFunction interface 'manually' we need to also implement the try-catch ourselves. Co-authored-by: Max Kießling <[email protected]> Co-authored-by: Sören Reichardt <[email protected]>
1 parent 35072a5 commit 118e557

File tree

5 files changed

+124
-54
lines changed

5 files changed

+124
-54
lines changed

‎cypher-aggregation/src/integrationTest/java/org/neo4j/gds/projection/CypherAggregationTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.neo4j.gds.wcc.WccStreamProc;
5252
import org.neo4j.graphdb.Direction;
5353
import org.neo4j.graphdb.GraphDatabaseService;
54+
import org.neo4j.graphdb.QueryExecutionException;
5455
import org.neo4j.graphdb.RelationshipType;
5556
import org.neo4j.graphdb.Result;
5657
import org.neo4j.kernel.api.QueryLanguage;
@@ -71,6 +72,7 @@
7172
import static org.assertj.core.api.Assertions.assertThatCode;
7273
import static org.assertj.core.api.Assertions.assertThatException;
7374
import static org.assertj.core.api.Assertions.assertThatThrownBy;
75+
import static org.assertj.core.api.InstanceOfAssertFactories.type;
7476
import static org.hamcrest.Matchers.any;
7577
import static org.hamcrest.Matchers.instanceOf;
7678
import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
@@ -122,6 +124,25 @@ static void registerUserAggregationFunction(
122124
}
123125
}
124126

127+
@Test
128+
void catchesExceptionsAndRethrowsGqlCompliantOnResult() {
129+
runQuery("RETURN gds.graph.project('g', 0, 1)");
130+
var exception = assertThatThrownBy(() -> runQuery("RETURN gds.graph.project('g', 0, 1)")).asInstanceOf(type(
131+
QueryExecutionException.class)).actual();
132+
var gqlCause = exception.gqlStatusObject().cause().orElse(exception.gqlStatusObject());
133+
assertThat(gqlCause.gqlStatus()).isEqualTo("53U00");
134+
assertThat(gqlCause.getMessage()).isEqualTo("53U00: Execution of the function gds.graph.project() failed due to java.lang.IllegalArgumentException: Graph g already exists.");
135+
}
136+
137+
@Test
138+
void catchesExceptionsAndRethrowsGqlCompliantOnUpdate() {
139+
var exception = assertThatThrownBy(() -> runQuery("RETURN gds.graph.project('g', null, null)")).asInstanceOf(type(
140+
QueryExecutionException.class)).actual();
141+
var gqlCause = exception.gqlStatusObject().cause().orElse(exception.gqlStatusObject());
142+
assertThat(gqlCause.gqlStatus()).isEqualTo("53U00");
143+
assertThat(gqlCause.getMessage()).isEqualTo("53U00: Execution of the function gds.graph.project() failed due to java.lang.IllegalArgumentException: The node has to be either a NODE or an INTEGER, but got NO_VALUE.");
144+
}
145+
125146
@Test
126147
void testMatch() {
127148
runQuery("MATCH (s:A)-[:REL]->(t:A) RETURN gds.graph.project('g', s, t)");

‎cypher-aggregation/src/main/java/org/neo4j/gds/projection/CypherAggregation.java‎

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -82,35 +82,42 @@ public ProjectionResult procedureSyntax(
8282

8383
@Override
8484
public UserAggregationReducer createReducer(Context ctx) throws ProcedureException {
85-
var databaseService = ctx.graphDatabaseAPI();
86-
var metrics = GraphDatabaseApiProxy.lookupComponentProvider(ctx, Metrics.class, true);
87-
var taskStore = GraphDatabaseApiProxy.lookupComponentProvider(ctx, TaskStore.class, true);
88-
var log = GraphDatabaseApiProxy.lookupComponentProvider(ctx, Log.class, true);
89-
var username = ctx.kernelTransaction().securityContext().subject().executingUser();
90-
var transaction = ctx.transaction();
85+
try {
86+
var databaseService = ctx.graphDatabaseAPI();
87+
var metrics = GraphDatabaseApiProxy.lookupComponentProvider(ctx, Metrics.class, true);
88+
var taskStore = GraphDatabaseApiProxy.lookupComponentProvider(ctx, TaskStore.class, true);
89+
var log = GraphDatabaseApiProxy.lookupComponentProvider(ctx, Log.class, true);
90+
var username = ctx.kernelTransaction().securityContext().subject().executingUser();
91+
var transaction = ctx.transaction();
9192

92-
var runsOnCompositeDatabase = AggregationInitializationHelper.runsOnCompositeDatabase(ctx);
93-
var queryProvider = AggregationInitializationHelper.getQueryProvider(ctx, runsOnCompositeDatabase);
93+
var runsOnCompositeDatabase = AggregationInitializationHelper.runsOnCompositeDatabase(ctx);
94+
var queryProvider = AggregationInitializationHelper.getQueryProvider(ctx, runsOnCompositeDatabase);
9495

95-
var writeMode = runsOnCompositeDatabase
96-
? WriteMode.NONE
97-
: WriteMode.LOCAL;
96+
var writeMode = runsOnCompositeDatabase
97+
? WriteMode.NONE
98+
: WriteMode.LOCAL;
9899

99-
var queryEstimator = QueryEstimator.fromTransaction(DatabaseTransactionContext.of(databaseService, transaction));
100+
var queryEstimator = QueryEstimator.fromTransaction(DatabaseTransactionContext.of(
101+
databaseService,
102+
transaction
103+
));
100104

101-
ProductGraphAggregator productGraphAggregator = new ProductGraphAggregator(
102-
DatabaseId.of(databaseService.databaseName()),
103-
username,
104-
writeMode,
105-
queryEstimator,
106-
queryProvider,
107-
metrics.projectionMetrics(),
108-
taskStore,
109-
new LogAdapter(log)
110-
);
105+
ProductGraphAggregator productGraphAggregator = new ProductGraphAggregator(
106+
DatabaseId.of(databaseService.databaseName()),
107+
username,
108+
writeMode,
109+
queryEstimator,
110+
queryProvider,
111+
metrics.projectionMetrics(),
112+
taskStore,
113+
new LogAdapter(log)
114+
);
111115

112-
ctx.internalTransaction().registerCloseableResource(productGraphAggregator);
116+
ctx.internalTransaction().registerCloseableResource(productGraphAggregator);
113117

114-
return productGraphAggregator;
118+
return productGraphAggregator;
119+
} catch (Throwable T) {
120+
throw ProcedureException.invocationFailed("function", FUNCTION_NAME.toString(), T);
121+
}
115122
}
116123
}

‎cypher-aggregation/src/main/java/org/neo4j/gds/projection/GraphAggregator.java‎

Lines changed: 27 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@
6666
import java.util.stream.Collectors;
6767
import java.util.stream.StreamSupport;
6868

69+
import static org.neo4j.gds.projection.CypherAggregation.FUNCTION_NAME;
6970
import static org.neo4j.gds.projection.GraphImporter.NO_TARGET_NODE;
7071
import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
7172

@@ -279,35 +280,36 @@ public void applyUpdates() {
279280

280281
@Override
281282
public AnyValue result() throws ProcedureException {
282-
var projectionMetric = projectionMetricsService.createCypherV2();
283-
ProjectionResult result;
284-
try (projectionMetric) {
285-
projectionMetric.start();
286-
result = buildGraph();
287-
} catch (Exception e) {
288-
projectionMetric.failed(e);
289-
throw ProcedureException.generalProcedureException(
290-
CypherAggregation.FUNCTION_NAME.name(),
291-
e
292-
);
293-
}
283+
try {
284+
var projectionMetric = projectionMetricsService.createCypherV2();
285+
ProjectionResult result;
286+
try (projectionMetric) {
287+
projectionMetric.start();
288+
result = buildGraph();
289+
} catch (Exception e) {
290+
projectionMetric.failed(e);
291+
throw e;
292+
}
294293

295-
if (result == null) {
296-
return Values.NO_VALUE;
297-
}
294+
if (result == null) {
295+
return Values.NO_VALUE;
296+
}
298297

299-
var builder = new MapValueBuilder(6);
300-
builder.add("graphName", Values.stringValue(result.graphName()));
301-
builder.add("nodeCount", Values.longValue(result.nodeCount()));
302-
builder.add("relationshipCount", Values.longValue(result.relationshipCount()));
303-
builder.add("projectMillis", Values.longValue(result.projectMillis()));
304-
builder.add("configuration", ValueUtils.asAnyValue(result.configuration()));
305-
builder.add("query", ValueUtils.asAnyValue(result.query()));
306-
MapValue projectResult = builder.build();
298+
var builder = new MapValueBuilder(6);
299+
builder.add("graphName", Values.stringValue(result.graphName()));
300+
builder.add("nodeCount", Values.longValue(result.nodeCount()));
301+
builder.add("relationshipCount", Values.longValue(result.relationshipCount()));
302+
builder.add("projectMillis", Values.longValue(result.projectMillis()));
303+
builder.add("configuration", ValueUtils.asAnyValue(result.configuration()));
304+
builder.add("query", ValueUtils.asAnyValue(result.query()));
305+
MapValue projectResult = builder.build();
307306

308-
this.completedSuccessfully.set(true);
307+
this.completedSuccessfully.set(true);
309308

310-
return projectResult;
309+
return projectResult;
310+
} catch (Throwable T) {
311+
throw ProcedureException.invocationFailed("function", FUNCTION_NAME.toString(), T);
312+
}
311313
}
312314

313315
public @Nullable ProjectionResult buildGraph() {

‎cypher-aggregation/src/main/java/org/neo4j/gds/projection/ProductGraphAggregator.java‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import org.neo4j.values.AnyValue;
2929
import org.neo4j.values.storable.TextValue;
3030

31+
import static org.neo4j.gds.projection.CypherAggregation.FUNCTION_NAME;
32+
3133
// public is required for the Cypher runtime
3234
@SuppressWarnings("WeakerAccess")
3335
public class ProductGraphAggregator extends GraphAggregator {
@@ -56,11 +58,8 @@ public void update(AnyValue[] input) throws ProcedureException {
5658
input[4],
5759
input[5]
5860
);
59-
} catch (Exception e) {
60-
throw ProcedureException.generalProcedureException(
61-
CypherAggregation.FUNCTION_NAME.name(),
62-
e
63-
);
61+
} catch (Throwable T) {
62+
throw ProcedureException.invocationFailed("function", FUNCTION_NAME.toString(), T);
6463
}
6564
}
6665
}

‎cypher-aggregation/src/test/java/org/neo4j/gds/projection/CypherAggregationTest.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Neo4j is free software: you can redistribute it and/or modify
8+
* it under the terms of the GNU General Public License as published by
9+
* the Free Software Foundation, either version 3 of the License, or
10+
* (at your option) any later version.
11+
*
12+
* This program is distributed in the hope that it will be useful,
13+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
* GNU General Public License for more details.
16+
*
17+
* You should have received a copy of the GNU General Public License
18+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
19+
*/
20+
package org.neo4j.gds.projection;
21+
22+
import org.junit.jupiter.api.Test;
23+
import org.neo4j.internal.kernel.api.exceptions.ProcedureException;
24+
25+
import static org.assertj.core.api.Assertions.assertThat;
26+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;
27+
import static org.assertj.core.api.InstanceOfAssertFactories.type;
28+
29+
class CypherAggregationTest {
30+
31+
@Test
32+
void catchesExceptionsAndRethrowsGqlCompliant() {
33+
var cypherAggregation = new CypherAggregation();
34+
var exception = assertThatThrownBy(() -> cypherAggregation.createReducer(null)).asInstanceOf(type(
35+
ProcedureException.class)).actual();
36+
var gqlCause = exception.gqlStatusObject().cause().orElse(exception.gqlStatusObject());
37+
assertThat(gqlCause.gqlStatus()).isEqualTo("53U00");
38+
assertThat(gqlCause.getMessage()).isEqualTo(
39+
"53U00: Execution of the function gds.graph.project() failed due to java.lang.NullPointerException: Cannot invoke \"org.neo4j.kernel.api.procedure.Context.graphDatabaseAPI()\" because \"ctx\" is null.");
40+
}
41+
}

0 commit comments

Comments
 (0)