Merge pull request #5373 from FlorentinD/slim-graphstore-interface

FlorentinD · web-flow · commit f9b548693999 · 2022-05-18T16:25:07.000+02:00
Inline node-property related delegation methods
diff --git a/core/src/main/java/org/neo4j/gds/api/GraphStore.java b/core/src/main/java/org/neo4j/gds/api/GraphStore.java
@@ -110,12 +110,6 @@ default Collection<String> nodePropertyKeys(Collection<NodeLabel> labels) {
 
     NodeProperty nodeProperty(String propertyKey);
 
-    ValueType nodePropertyType(String propertyKey);
-
-    PropertyState nodePropertyState(String propertyKey);
-
-    NodePropertyValues nodePropertyValues(String propertyKey);
-
     void addNodeProperty(
         Set<NodeLabel> nodeLabels,
         String propertyKey,
diff --git a/core/src/main/java/org/neo4j/gds/api/GraphStoreAdapter.java b/core/src/main/java/org/neo4j/gds/api/GraphStoreAdapter.java
@@ -157,21 +157,6 @@ public NodeProperty nodeProperty(String propertyKey) {
         return graphStore.nodeProperty(propertyKey);
     }
 
-    @Override
-    public ValueType nodePropertyType(String propertyKey) {
-        return graphStore.nodePropertyType(propertyKey);
-    }
-
-    @Override
-    public PropertyState nodePropertyState(String propertyKey) {
-        return graphStore.nodePropertyState(propertyKey);
-    }
-
-    @Override
-    public NodePropertyValues nodePropertyValues(String propertyKey) {
-        return graphStore.nodePropertyValues(propertyKey);
-    }
-
     @Override
     public void addNodeProperty(
        Set<NodeLabel> nodeLabels, String propertyKey, NodePropertyValues propertyValues
diff --git a/core/src/main/java/org/neo4j/gds/core/loading/CSRGraphStore.java b/core/src/main/java/org/neo4j/gds/core/loading/CSRGraphStore.java
@@ -332,21 +332,6 @@ public NodeProperty nodeProperty(String propertyKey) {
         return this.nodeProperties.get(propertyKey);
     }
 
-    @Override
-    public ValueType nodePropertyType(String propertyKey) {
-        return nodeProperty(propertyKey).valueType();
-    }
-
-    @Override
-    public PropertyState nodePropertyState(String propertyKey) {
-        return nodeProperty(propertyKey).propertyState();
-    }
-
-    @Override
-    public NodePropertyValues nodePropertyValues(String propertyKey) {
-        return nodeProperty(propertyKey).values();
-    }
-
     @Override
     public Set<RelationshipType> relationshipTypes() {
         return relationships.keySet();
@@ -753,7 +738,7 @@ private Map<String, NodePropertyValues> filterNodeProperties(Collection<NodeLabe
             .stream()
             .collect(toMap(
                 Function.identity(),
-                this::nodePropertyValues
+                propertyKey -> nodeProperty(propertyKey).values()
             ));
     }
 
diff --git a/core/src/main/java/org/neo4j/gds/core/utils/io/NodeStore.java b/core/src/main/java/org/neo4j/gds/core/utils/io/NodeStore.java
@@ -127,7 +127,7 @@ static NodeStore of(
         graphStore.nodeLabels().forEach(label -> {
             var properties = nodeProperties.computeIfAbsent(label.name, k -> new HashMap<>());
             graphStore.schema().nodeSchema().propertySchemasFor(label).forEach(propertySchema -> {
-                properties.put(propertySchema.key(), graphStore.nodePropertyValues(propertySchema.key()));
+                properties.put(propertySchema.key(), graphStore.nodeProperty(propertySchema.key()).values());
             });
         });
 
diff --git a/core/src/test/java/org/neo4j/gds/core/GraphLoaderMultipleRelTypesAndPropertiesTest.java b/core/src/test/java/org/neo4j/gds/core/GraphLoaderMultipleRelTypesAndPropertiesTest.java
@@ -116,11 +116,11 @@ void nodeProjectionsWithExclusiveProperties() {
         assertEquals(Collections.emptySet(), graphStore.nodePropertyKeys(NodeLabel.of("N2")));
         assertEquals(Collections.singleton("prop2"), graphStore.nodePropertyKeys(NodeLabel.of("N3")));
 
-        NodePropertyValues prop1 = graphStore.nodePropertyValues("prop1");
+        NodePropertyValues prop1 = graphStore.nodeProperty("prop1").values();
         assertEquals(1L, prop1.longValue(0));
         assertEquals(0L, prop1.longValue(1));
 
-        NodePropertyValues prop2 = graphStore.nodePropertyValues("prop2");
+        NodePropertyValues prop2 = graphStore.nodeProperty("prop2").values();
         assertEquals(1L, prop2.longValue(0));
         assertEquals(2L, prop2.longValue(1));
     }
@@ -155,9 +155,9 @@ void nodeProjectionsWithAndWithoutLabel() {
         assertEquals(Set.of("prop1", "prop2"), graphStore.nodePropertyKeys(allIdentifier));
         assertEquals(Set.of("prop2"), graphStore.nodePropertyKeys(node2Identifier));
 
-        NodePropertyValues allProp1 = graphStore.nodePropertyValues("prop1");
-        NodePropertyValues allProp2 = graphStore.nodePropertyValues("prop2");
-        NodePropertyValues node2Prop2 = graphStore.nodePropertyValues("prop2");
+        var allProp1 = graphStore.nodeProperty("prop1").values();
+        var allProp2 = graphStore.nodeProperty("prop2").values();
+        var node2Prop2 = graphStore.nodeProperty("prop2").values();
 
         LongStream.range(0, 3).forEach(nodeId -> {
             double allProp1Value = allProp1.doubleValue(nodeId);
diff --git a/csv/src/main/java/org/neo4j/gds/core/utils/io/file/csv/estimation/NodePropertySampler.java b/csv/src/main/java/org/neo4j/gds/core/utils/io/file/csv/estimation/NodePropertySampler.java
@@ -115,7 +115,8 @@ void sample(long nodeId) {
 
             propertyCharactersSamples.forEach((propertyKey, sampleList) -> {
                 var propertyValue = graphStore
-                    .nodePropertyValues(propertyKey)
+                    .nodeProperty(propertyKey)
+                    .values()
                     .getObject(nodeId);
 
                 int characterCount = 0;
diff --git a/cypher/api/storage-engine-adapter/src/main/java/org/neo4j/gds/compat/AbstractInMemoryNodePropertyCursor.java b/cypher/api/storage-engine-adapter/src/main/java/org/neo4j/gds/compat/AbstractInMemoryNodePropertyCursor.java
@@ -78,7 +78,8 @@ protected void setPropertySelection(InMemoryPropertySelection propertySelection)
 
                     if (!propertySelection.isKeysOnly()) {
                         nodePropertyValues[propertyIndex] = graphStore
-                            .nodePropertyValues(nodePropertyKey)
+                            .nodeProperty(nodePropertyKey)
+                            .values()
                             .value(nodeId);
                     }
 
diff --git a/cypher/cypher-test/src/test/java/org/neo4j/gds/storageengine/InMemoryNodeCursorTest.java b/cypher/cypher-test/src/test/java/org/neo4j/gds/storageengine/InMemoryNodeCursorTest.java
@@ -134,7 +134,7 @@ void shouldTraverseProperties() throws TokenNotFoundException {
 
         var propertyName = tokenHolders.propertyKeyTokens().getTokenById(propertyCursor.propertyKey()).name();
         var propertyValue = ((LongValue) propertyCursor.propertyValue()).longValue();
-        assertThat(propertyValue).isEqualTo(graphStore.nodePropertyValues(propertyName).longValue(nodeCursor.getId()));
+        assertThat(propertyValue).isEqualTo(graphStore.nodeProperty(propertyName).values().longValue(nodeCursor.getId()));
 
         assertThat(propertyCursor.next()).isFalse();
     }
diff --git a/proc/beta/src/test/java/org/neo4j/gds/beta/modularity/ModularityOptimizationMutateProcTest.java b/proc/beta/src/test/java/org/neo4j/gds/beta/modularity/ModularityOptimizationMutateProcTest.java
@@ -32,7 +32,6 @@
 import org.neo4j.gds.RelationshipProjections;
 import org.neo4j.gds.api.GraphStore;
 import org.neo4j.gds.api.nodeproperties.ValueType;
-import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
 import org.neo4j.gds.config.GraphProjectFromStoreConfig;
 import org.neo4j.gds.config.ImmutableGraphProjectFromStoreConfig;
 import org.neo4j.gds.core.CypherMapWrapper;
@@ -111,8 +110,8 @@ void testMutateSeeded() {
         runQuery(query);
 
         GraphStore mutatedGraph = GraphStoreCatalog.get(TEST_USERNAME, namedDatabaseId(), TEST_GRAPH_NAME).graphStore();
-        NodePropertyValues communities = mutatedGraph.nodePropertyValues(mutateProperty());
-        NodePropertyValues seeds = mutatedGraph.nodePropertyValues("seed1");
+        var communities = mutatedGraph.nodeProperty(mutateProperty()).values();
+        var seeds = mutatedGraph.nodeProperty("seed1").values();
         for (int i = 0; i < mutatedGraph.nodeCount(); i++) {
             assertEquals(communities.longValue(i), seeds.longValue(i));
         }
diff --git a/proc/beta/src/test/java/org/neo4j/gds/beta/node2vec/Node2VecMutateProcTest.java b/proc/beta/src/test/java/org/neo4j/gds/beta/node2vec/Node2VecMutateProcTest.java
@@ -108,7 +108,7 @@ public void testMutateFailsOnExistingToken() {
     }
 
     private void assertMutatedGraph(GraphStore mutatedGraphStore) {
-        var mutatedProperties = mutatedGraphStore.nodePropertyValues(mutateProperty());
+        var mutatedProperties = mutatedGraphStore.nodeProperty(mutateProperty()).values();
         mutatedGraphStore.nodes().forEachNode(nodeId -> {
             var embedding = mutatedProperties.floatArrayValue(nodeId);
             assertThat(embedding)
diff --git a/proc/catalog/src/main/java/org/neo4j/gds/catalog/GraphRemoveNodePropertiesProc.java b/proc/catalog/src/main/java/org/neo4j/gds/catalog/GraphRemoveNodePropertiesProc.java
@@ -73,7 +73,7 @@ private Long removeNodeProperties(GraphStore graphStore, GraphRemoveNodeProperti
         var removedPropertiesCount = new MutableLong(0);
 
         config.nodeProperties().forEach(propertyKey -> {
-            removedPropertiesCount.add(graphStore.nodePropertyValues(propertyKey).size());
+            removedPropertiesCount.add(graphStore.nodeProperty(propertyKey).values().size());
             graphStore.removeNodeProperty(propertyKey);
         });
 
diff --git a/proc/common/src/main/java/org/neo4j/gds/functions/NodePropertyFunc.java b/proc/common/src/main/java/org/neo4j/gds/functions/NodePropertyFunc.java
@@ -96,7 +96,7 @@ public Object nodeProperty(
             return null;
         }
 
-        var propertyValues = graphStore.nodePropertyValues(propertyKey);
+        var propertyValues = graphStore.nodeProperty(propertyKey).values();
 
         switch (propertyValues.valueType()) {
             case LONG:
diff --git a/proc/embeddings/src/test/java/org/neo4j/gds/embeddings/graphsage/GraphSageMutateProcTest.java b/proc/embeddings/src/test/java/org/neo4j/gds/embeddings/graphsage/GraphSageMutateProcTest.java
@@ -65,7 +65,7 @@ void testWriting(int embeddingSize, String aggregator, ActivationFunction activa
             assertThat(row.get("configuration")).isInstanceOf(Map.class);
         });
 
-        NodePropertyValues embeddingNodePropertyValues = graphStore.nodePropertyValues(mutatePropertyKey);
+        NodePropertyValues embeddingNodePropertyValues = graphStore.nodeProperty(mutatePropertyKey).values();
         graphStore.nodes().forEachNode(nodeId -> {
             assertEquals(embeddingSize, embeddingNodePropertyValues.doubleArrayValue(nodeId).length);
             return true;
diff --git a/subgraph-filtering/src/main/java/org/neo4j/gds/beta/filter/NodesFilter.java b/subgraph-filtering/src/main/java/org/neo4j/gds/beta/filter/NodesFilter.java
@@ -121,12 +121,11 @@ private static NodePropertyStore filterNodeProperties(
         var inputMapping = inputGraphStore.nodes();
 
         propertyKeys.forEach(propertyKey -> {
-            var nodeProperties = inputGraphStore.nodePropertyValues(propertyKey);
-            var propertyState = inputGraphStore.nodePropertyState(propertyKey);
+            var nodeProperty = inputGraphStore.nodeProperty(propertyKey);
 
             NodePropertiesBuilder<?> nodePropertiesBuilder = getPropertiesBuilder(
                 inputMapping,
-                nodeProperties,
+                nodeProperty.values(),
                 concurrency
             );
 
@@ -142,7 +141,11 @@ private static NodePropertyStore filterNodeProperties(
 
             builder.putProperty(
                 propertyKey,
-                NodeProperty.of(propertyKey, propertyState, nodePropertiesBuilder.build(filteredNodeCount, filteredIdMap))
+                NodeProperty.of(
+                    propertyKey,
+                    nodeProperty.propertyState(),
+                    nodePropertiesBuilder.build(filteredNodeCount, filteredIdMap)
+                )
             );
         });
         progressTracker.endSubTask();
diff --git a/subgraph-filtering/src/main/java/org/neo4j/gds/beta/filter/expression/EvaluationContext.java b/subgraph-filtering/src/main/java/org/neo4j/gds/beta/filter/expression/EvaluationContext.java
@@ -51,7 +51,7 @@ public NodeEvaluationContext(GraphStore graphStore) {
             if (!graphStore.hasNodeProperty(propertyKey)) {
                 return DefaultValue.DOUBLE_DEFAULT_FALLBACK;
             } else {
-                NodePropertyValues nodePropertyValues = graphStore.nodePropertyValues(propertyKey);
+                NodePropertyValues nodePropertyValues = graphStore.nodeProperty(propertyKey).values();
                 return propertyType == ValueType.LONG
                     ? Double.longBitsToDouble(nodePropertyValues.longValue(nodeId))
                     : nodePropertyValues.doubleValue(nodeId);

Original file line number	Diff line number	Diff line change
`@@ -78,7 +78,8 @@ protected void setPropertySelection(InMemoryPropertySelection propertySelection)`
`78`	`78`
`79`	`79`	`if (!propertySelection.isKeysOnly()) {`
`80`	`80`	`nodePropertyValues[propertyIndex] = graphStore`
`81`		`- .nodePropertyValues(nodePropertyKey)`
	`81`	`+ .nodeProperty(nodePropertyKey)`
	`82`	`+ .values()`
`82`	`83`	`.value(nodeId);`
`83`	`84`	`}`
`84`	`85`
Original file line number	Diff line number	Diff line change
`@@ -134,7 +134,7 @@ void shouldTraverseProperties() throws TokenNotFoundException {`
`134`	`134`
`135`	`135`	`var propertyName = tokenHolders.propertyKeyTokens().getTokenById(propertyCursor.propertyKey()).name();`
`136`	`136`	`var propertyValue = ((LongValue) propertyCursor.propertyValue()).longValue();`
`137`		`- assertThat(propertyValue).isEqualTo(graphStore.nodePropertyValues(propertyName).longValue(nodeCursor.getId()));`
	`137`	`+ assertThat(propertyValue).isEqualTo(graphStore.nodeProperty(propertyName).values().longValue(nodeCursor.getId()));`
`138`	`138`
`139`	`139`	`assertThat(propertyCursor.next()).isFalse();`
`140`	`140`	`}`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ public void testMutateFailsOnExistingToken() {`
`108`	`108`	`}`
`109`	`109`
`110`	`110`	`private void assertMutatedGraph(GraphStore mutatedGraphStore) {`
`111`		`- var mutatedProperties = mutatedGraphStore.nodePropertyValues(mutateProperty());`
	`111`	`+ var mutatedProperties = mutatedGraphStore.nodeProperty(mutateProperty()).values();`
`112`	`112`	`mutatedGraphStore.nodes().forEachNode(nodeId -> {`
`113`	`113`	`var embedding = mutatedProperties.floatArrayValue(nodeId);`
`114`	`114`	`assertThat(embedding)`