Skip to content

Commit bf85f19

Browse files
DarthMaxs1ck
DarthMax
and
s1ck
committed
Expose property state parameter for addProperty method on RelationshipSchema
Co-authored-by: Martin Junghanns <martin.junghanns@neotechnology.com>
1 parent 325b1be commit bf85f19

File tree

10 files changed

+120
-74
lines changed

10 files changed

+120
-74
lines changed

‎core/src/main/java/org/neo4j/gds/beta/generator/RandomGraphGenerator.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.neo4j.gds.annotation.ValueClass;
2626
import org.neo4j.gds.api.DefaultValue;
2727
import org.neo4j.gds.api.IdMap;
28+
import org.neo4j.gds.api.PropertyState;
2829
import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
2930
import org.neo4j.gds.api.schema.Direction;
3031
import org.neo4j.gds.api.schema.GraphSchema;
@@ -147,7 +148,8 @@ private RelationshipSchema relationshipSchema() {
147148
var entry = relationshipSchema.getOrCreateRelationshipType(relationshipType, direction);
148149
maybeRelationshipPropertyProducer.ifPresent(pp -> entry.addProperty(
149150
pp.getPropertyName(),
150-
pp.propertyType()
151+
pp.propertyType(),
152+
PropertyState.PERSISTENT
151153
));
152154
return relationshipSchema;
153155
}

‎core/src/main/java/org/neo4j/gds/core/loading/CSRGraphStoreUtil.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.neo4j.gds.RelationshipType;
2525
import org.neo4j.gds.api.DatabaseId;
2626
import org.neo4j.gds.api.Graph;
27+
import org.neo4j.gds.api.PropertyState;
2728
import org.neo4j.gds.api.RelationshipProperty;
2829
import org.neo4j.gds.api.RelationshipPropertyStore;
2930
import org.neo4j.gds.api.Relationships;
@@ -80,7 +81,8 @@ public static CSRGraphStore createFromGraph(
8081

8182
entry.addProperty(
8283
property,
83-
ValueType.DOUBLE
84+
ValueType.DOUBLE,
85+
PropertyState.PERSISTENT
8486
);
8587
});
8688

‎core/src/main/java/org/neo4j/gds/core/loading/construction/GraphFactory.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.neo4j.gds.api.DefaultValue;
3333
import org.neo4j.gds.api.IdMap;
3434
import org.neo4j.gds.api.PartialIdMap;
35+
import org.neo4j.gds.api.PropertyState;
3536
import org.neo4j.gds.api.Relationships;
3637
import org.neo4j.gds.api.nodeproperties.ValueType;
3738
import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
@@ -307,7 +308,7 @@ public static HugeGraph create(IdMap idMap, RelationshipsAndDirection relationsh
307308
);
308309

309310
if (relationshipsAndDirection.relationships().properties().isPresent()) {
310-
entry.addProperty("property", ValueType.DOUBLE);
311+
entry.addProperty("property", ValueType.DOUBLE, PropertyState.PERSISTENT);
311312
}
312313

313314
return create(

‎core/src/test/java/org/neo4j/gds/beta/generator/RandomGraphGeneratorTest.java‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.neo4j.gds.NodeLabel;
2828
import org.neo4j.gds.RelationshipType;
2929
import org.neo4j.gds.TestSupport;
30+
import org.neo4j.gds.api.PropertyState;
3031
import org.neo4j.gds.api.nodeproperties.ValueType;
3132
import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
3233
import org.neo4j.gds.api.schema.Direction;
@@ -559,10 +560,7 @@ static Stream<Arguments> expectedSchemas() {
559560
.nodePropertyProducer(PropertyProducer.randomLong("nodeProp", 0, 42))
560561
.relationshipDistribution(RelationshipDistribution.RANDOM)
561562
.build(),
562-
NodeSchema
563-
.empty()
564-
.addLabel(NodeLabel.of("A"))
565-
.addProperty(NodeLabel.of("A"), "nodeProp", ValueType.LONG),
563+
NodeSchema.empty().addLabel(NodeLabel.of("A")).addProperty(NodeLabel.of("A"), "nodeProp", ValueType.LONG),
566564
RelationshipSchema.empty().addRelationshipType(RelationshipType.of("REL"), Direction.DIRECTED)
567565
), Arguments.of("relationship type and node property",
568566
RandomGraphGenerator
@@ -574,9 +572,12 @@ static Stream<Arguments> expectedSchemas() {
574572
.relationshipDistribution(RelationshipDistribution.RANDOM)
575573
.build(),
576574
NodeSchema.empty().addLabel(NodeLabel.ALL_NODES),
577-
RelationshipSchema
578-
.empty()
579-
.addProperty(RelationshipType.of("FOOBAR"), Direction.DIRECTED, "relProperty", ValueType.DOUBLE)
575+
RelationshipSchema.empty().addProperty(RelationshipType.of("FOOBAR"),
576+
Direction.DIRECTED,
577+
"relProperty",
578+
ValueType.DOUBLE,
579+
PropertyState.PERSISTENT
580+
)
580581
), Arguments.of("node label, node property, relationship type and relationship property",
581582
RandomGraphGenerator
582583
.builder()
@@ -588,13 +589,13 @@ static Stream<Arguments> expectedSchemas() {
588589
.relationshipPropertyProducer(PropertyProducer.randomDouble("relProp", 0, 42))
589590
.relationshipDistribution(RelationshipDistribution.RANDOM)
590591
.build(),
591-
NodeSchema
592-
.empty()
593-
.addLabel(NodeLabel.of("A"))
594-
.addProperty(NodeLabel.of("A"), "nodeProp", ValueType.LONG),
595-
RelationshipSchema
596-
.empty()
597-
.addProperty(RelationshipType.of("FOOBAR"), Direction.DIRECTED, "relProp", ValueType.DOUBLE)
592+
NodeSchema.empty().addLabel(NodeLabel.of("A")).addProperty(NodeLabel.of("A"), "nodeProp", ValueType.LONG),
593+
RelationshipSchema.empty().addProperty(RelationshipType.of("FOOBAR"),
594+
Direction.DIRECTED,
595+
"relProp",
596+
ValueType.DOUBLE,
597+
PropertyState.PERSISTENT
598+
)
598599
), Arguments.of("relationship type, relationship property, and undirected orientation",
599600
RandomGraphGenerator
600601
.builder()
@@ -609,7 +610,12 @@ static Stream<Arguments> expectedSchemas() {
609610
NodeSchema.empty().addLabel(NodeLabel.of("A")),
610611
RelationshipSchema
611612
.empty()
612-
.addProperty(RelationshipType.of("FOOBAR"), UNDIRECTED, "relProp", ValueType.DOUBLE)
613+
.addProperty(RelationshipType.of("FOOBAR"),
614+
UNDIRECTED,
615+
"relProp",
616+
ValueType.DOUBLE,
617+
PropertyState.PERSISTENT
618+
)
613619
));
614620
}
615621
}

‎graph-schema-api/src/main/java/org/neo4j/gds/api/schema/RelationshipSchema.java‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
package org.neo4j.gds.api.schema;
2121

2222
import org.neo4j.gds.RelationshipType;
23+
import org.neo4j.gds.api.PropertyState;
2324
import org.neo4j.gds.api.nodeproperties.ValueType;
24-
import org.neo4j.gds.core.Aggregation;
2525

2626
import java.util.LinkedHashMap;
2727
import java.util.Map;
@@ -98,21 +98,14 @@ public RelationshipSchema addProperty(
9898
return this;
9999
}
100100

101-
public RelationshipSchema addProperty(
102-
RelationshipType relationshipType, Direction direction, String propertyKey, ValueType valueType
103-
) {
104-
getOrCreateRelationshipType(relationshipType, direction).addProperty(propertyKey, valueType);
105-
return this;
106-
}
107-
108101
public RelationshipSchema addProperty(
109102
RelationshipType relationshipType,
110103
Direction direction,
111104
String propertyKey,
112105
ValueType valueType,
113-
Aggregation aggregation
106+
PropertyState propertyState
114107
) {
115-
getOrCreateRelationshipType(relationshipType, direction).addProperty(propertyKey, valueType, aggregation);
108+
getOrCreateRelationshipType(relationshipType, direction).addProperty(propertyKey, valueType, propertyState);
116109
return this;
117110
}
118111

‎graph-schema-api/src/main/java/org/neo4j/gds/api/schema/RelationshipSchemaEntry.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,15 @@ RelationshipSchemaEntry union(RelationshipSchemaEntry other) {
8484
return new RelationshipSchemaEntry(this.identifier(), this.direction, unionProperties(other.properties));
8585
}
8686

87-
public RelationshipSchemaEntry addProperty(String propertyKey, ValueType valueType) {
88-
return addProperty(propertyKey, valueType, Aggregation.DEFAULT);
89-
}
90-
91-
public RelationshipSchemaEntry addProperty(String propertyKey, ValueType valueType, Aggregation aggregation) {
87+
public RelationshipSchemaEntry addProperty(String propertyKey, ValueType valueType, PropertyState propertyState) {
9288
return addProperty(
9389
propertyKey,
9490
RelationshipPropertySchema.of(
9591
propertyKey,
9692
valueType,
9793
valueType.fallbackValue(),
98-
PropertyState.PERSISTENT,
99-
aggregation
94+
propertyState,
95+
Aggregation.DEFAULT
10096
)
10197
);
10298
}

‎graph-schema-api/src/test/java/org/neo4j/gds/api/schema/RelationshipSchemaTest.java‎

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -116,15 +116,15 @@ void testFiltering(Direction orientation, boolean isUndirected) {
116116
var label2 = RelationshipType.of("Bar");
117117

118118
var relationshipSchema = RelationshipSchema.empty()
119-
.addProperty(label1, orientation, "bar", ValueType.DOUBLE)
120-
.addProperty(label1, orientation, "baz", ValueType.DOUBLE)
121-
.addProperty(label2, orientation, "baz", ValueType.DOUBLE);
119+
.addProperty(label1, orientation, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT)
120+
.addProperty(label1, orientation, "baz", ValueType.DOUBLE, PropertyState.PERSISTENT)
121+
.addProperty(label2, orientation, "baz", ValueType.DOUBLE, PropertyState.PERSISTENT);
122122

123123
assertThat(relationshipSchema.filter(Set.of(label1, label2))).isEqualTo(relationshipSchema);
124124

125125
var expected = RelationshipSchema.empty()
126-
.addProperty(label1, orientation, "bar", ValueType.DOUBLE)
127-
.addProperty(label1, orientation, "baz", ValueType.DOUBLE);
126+
.addProperty(label1, orientation, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT)
127+
.addProperty(label1, orientation, "baz", ValueType.DOUBLE, PropertyState.PERSISTENT);
128128

129129
assertThat(relationshipSchema.filter(Set.of(label1))).isEqualTo(expected);
130130
assertThat(relationshipSchema.isUndirected()).isEqualTo(isUndirected);
@@ -136,9 +136,9 @@ void testFilteringMixed() {
136136
var undirectedType = RelationshipType.of("U");
137137

138138
var directed = RelationshipSchema.empty()
139-
.addProperty(directedType, Direction.DIRECTED, "bar", ValueType.DOUBLE);
139+
.addProperty(directedType, Direction.DIRECTED, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT);
140140
var undirected = RelationshipSchema.empty()
141-
.addProperty(undirectedType, Direction.UNDIRECTED, "flob", ValueType.DOUBLE);
141+
.addProperty(undirectedType, Direction.UNDIRECTED, "flob", ValueType.DOUBLE, PropertyState.PERSISTENT);
142142
var mixed = directed.union(undirected);
143143

144144
assertThat(mixed.isUndirected()).isFalse();
@@ -165,14 +165,14 @@ void testUnion(Direction direction1, Direction direction2, Boolean isUndirectedE
165165
var type2 = RelationshipType.of("Bar");
166166

167167
var relationshipSchema1 = RelationshipSchema.empty()
168-
.addProperty(type1, direction1, "bar", ValueType.DOUBLE);
168+
.addProperty(type1, direction1, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT);
169169

170170
var relationshipSchema2 = RelationshipSchema.empty()
171-
.addProperty(type2, direction2, "bar", ValueType.DOUBLE);
171+
.addProperty(type2, direction2, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT);
172172

173173
var expected = RelationshipSchema.empty()
174-
.addProperty(type1, direction1, "bar", ValueType.DOUBLE)
175-
.addProperty(type2, direction2, "bar", ValueType.DOUBLE);
174+
.addProperty(type1, direction1, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT)
175+
.addProperty(type2, direction2, "bar", ValueType.DOUBLE, PropertyState.PERSISTENT);
176176

177177
var actual = relationshipSchema1.union(relationshipSchema2);
178178
assertThat(actual).isEqualTo(expected);
@@ -199,13 +199,25 @@ void unionOnSameTypesFailsOnDirectionMismatch() {
199199

200200
@Test
201201
void unionOnSameTypeSamePropertyDifferentValueTypeFails() {
202-
var schema1 = RelationshipSchema.empty()
203-
.addProperty(RelationshipType.of("X"), Direction.DIRECTED, "x", ValueType.DOUBLE)
204-
.addProperty(RelationshipType.of("Y"), Direction.UNDIRECTED, "unlikelypartofmessage", ValueType.DOUBLE);
202+
var schema1 = RelationshipSchema
203+
.empty()
204+
.addProperty(RelationshipType.of("X"), Direction.DIRECTED, "x", ValueType.DOUBLE, PropertyState.PERSISTENT)
205+
.addProperty(RelationshipType.of("Y"),
206+
Direction.UNDIRECTED,
207+
"unlikelypartofmessage",
208+
ValueType.DOUBLE,
209+
PropertyState.PERSISTENT
210+
);
205211

206-
var schema2 = RelationshipSchema.empty()
207-
.addProperty(RelationshipType.of("X"), Direction.DIRECTED, "x", ValueType.LONG)
208-
.addProperty(RelationshipType.of("Y"), Direction.UNDIRECTED, "unlikelypartofmessage", ValueType.DOUBLE);
212+
var schema2 = RelationshipSchema
213+
.empty()
214+
.addProperty(RelationshipType.of("X"), Direction.DIRECTED, "x", ValueType.LONG, PropertyState.PERSISTENT)
215+
.addProperty(RelationshipType.of("Y"),
216+
Direction.UNDIRECTED,
217+
"unlikelypartofmessage",
218+
ValueType.DOUBLE,
219+
PropertyState.PERSISTENT
220+
);
209221

210222
assertThatThrownBy(() -> schema1.union(schema2))
211223
.hasMessageContaining("Combining schema entries with value type")
@@ -217,14 +229,28 @@ void unionOnSameTypeSamePropertyDifferentValueTypeFails() {
217229

218230
static Stream<Arguments> schemaAndHasProperties() {
219231
return Stream.of(
220-
Arguments.of(RelationshipSchema.empty().addRelationshipType(RelationshipType.of("A"), Direction.DIRECTED), false),
221-
Arguments.of(RelationshipSchema.empty().addRelationshipType(RelationshipType.of("A"), Direction.UNDIRECTED), false),
232+
Arguments.of(RelationshipSchema.empty().addRelationshipType(RelationshipType.of("A"), Direction.DIRECTED),
233+
false
234+
),
235+
Arguments.of(RelationshipSchema.empty().addRelationshipType(RelationshipType.of("A"), Direction.UNDIRECTED),
236+
false
237+
),
222238
Arguments.of(RelationshipSchema
223239
.empty()
224-
.addProperty(RelationshipType.of("A"), Direction.DIRECTED, "foo", ValueType.LONG), true),
240+
.addProperty(RelationshipType.of("A"),
241+
Direction.DIRECTED,
242+
"foo",
243+
ValueType.LONG,
244+
PropertyState.PERSISTENT
245+
), true),
225246
Arguments.of(RelationshipSchema
226247
.empty()
227-
.addProperty(RelationshipType.of("A"), Direction.UNDIRECTED, "foo", ValueType.LONG), true)
248+
.addProperty(RelationshipType.of("A"),
249+
Direction.UNDIRECTED,
250+
"foo",
251+
ValueType.LONG,
252+
PropertyState.PERSISTENT
253+
), true)
228254
);
229255
}
230256

@@ -250,9 +276,23 @@ void testBuildRelTypes() {
250276

251277
@Test
252278
void testBuildProperties() {
253-
var schema = RelationshipSchema.empty()
254-
.addProperty(RelationshipType.of("X"), Direction.UNDIRECTED, "x", ValueType.DOUBLE)
255-
.addProperty(RelationshipType.of("Y"), Direction.DIRECTED, "y", ValueType.DOUBLE, Aggregation.MIN)
279+
RelationshipSchema relationshipSchema = RelationshipSchema.empty()
280+
.addProperty(RelationshipType.of("X"), Direction.UNDIRECTED, "x", ValueType.DOUBLE,
281+
PropertyState.PERSISTENT
282+
);
283+
relationshipSchema
284+
.getOrCreateRelationshipType(RelationshipType.of("Y"), Direction.DIRECTED)
285+
.addProperty(
286+
"y",
287+
RelationshipPropertySchema.of(
288+
"y",
289+
ValueType.DOUBLE,
290+
DefaultValue.forDouble(),
291+
PropertyState.PERSISTENT,
292+
Aggregation.MIN
293+
)
294+
);
295+
var schema = relationshipSchema
256296
.addProperty(
257297
RelationshipType.of("Z"),
258298
Direction.UNDIRECTED,
@@ -277,10 +317,14 @@ void testBuildProperties() {
277317
void shouldCreateDeepCopiesWhenFiltering() {
278318
var relType = RelationshipType.of("A");
279319
var relationshipSchema = RelationshipSchema.empty();
280-
relationshipSchema.getOrCreateRelationshipType(relType, Direction.DIRECTED).addProperty("prop", ValueType.LONG);
320+
relationshipSchema.getOrCreateRelationshipType(relType, Direction.DIRECTED).addProperty("prop", ValueType.LONG,
321+
PropertyState.PERSISTENT
322+
);
281323
var filteredSchema = relationshipSchema.filter(Set.of(relType));
282324
filteredSchema
283-
.getOrCreateRelationshipType(relType, Direction.DIRECTED).addProperty("shouldNotExistInOriginalSchema", ValueType.LONG);
325+
.getOrCreateRelationshipType(relType, Direction.DIRECTED).addProperty("shouldNotExistInOriginalSchema", ValueType.LONG,
326+
PropertyState.PERSISTENT
327+
);
284328

285329
assertThat(relationshipSchema.get(relType).properties())
286330
.doesNotContainKey("shouldNotExistInOriginalSchema")

‎io/csv/src/test/java/org/neo4j/gds/core/io/file/csv/CsvRelationshipVisitorTest.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.junit.jupiter.api.Test;
2323
import org.neo4j.gds.RelationshipType;
24+
import org.neo4j.gds.api.PropertyState;
2425
import org.neo4j.gds.api.nodeproperties.ValueType;
2526
import org.neo4j.gds.api.schema.Direction;
2627
import org.neo4j.gds.api.schema.RelationshipSchema;
@@ -81,12 +82,12 @@ void visitRelationshipsWithTypesAndProperties() {
8182

8283
var relationshipSchema = RelationshipSchema.empty();
8384
relationshipSchema.getOrCreateRelationshipType(aType, Direction.DIRECTED)
84-
.addProperty("foo", ValueType.LONG)
85-
.addProperty("bar", ValueType.LONG);
85+
.addProperty("foo", ValueType.LONG, PropertyState.PERSISTENT)
86+
.addProperty("bar", ValueType.LONG, PropertyState.PERSISTENT);
8687

8788
relationshipSchema.getOrCreateRelationshipType(bType, Direction.DIRECTED)
88-
.addProperty("bar", ValueType.LONG)
89-
.addProperty("baz", ValueType.DOUBLE);
89+
.addProperty("bar", ValueType.LONG, PropertyState.PERSISTENT)
90+
.addProperty("baz", ValueType.DOUBLE, PropertyState.PERSISTENT);
9091

9192
var relationshipVisitor= new CsvRelationshipVisitor(tempDir, relationshipSchema);
9293

‎test-utils/src/main/java/org/neo4j/gds/gdl/GdlFactory.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ private HashMap<RelationshipType, List<String>> propertyKeysByRelType() {
412412
var relType = RelationshipType.of(edge.getLabel());
413413
var entry = relationshipSchema.getOrCreateRelationshipType(relType, orientation);
414414
edge.getProperties().keySet().forEach(propertyKey ->
415-
entry.addProperty(propertyKey, ValueType.DOUBLE)
415+
entry.addProperty(propertyKey, ValueType.DOUBLE, PropertyState.PERSISTENT)
416416
);
417417
});
418418

0 commit comments

Comments
 (0)