Merge pull request #9906 from neo-technology/empty-project-values

Move usages of Neo4j Values from nodes builder to native projection
neo4j · Dec 4, 2024 · 740dd63 · 740dd63
2 parents 4d4bd00 + 28d94b9
commit 740dd63
Show file tree

Hide file tree

Showing 8 changed files with 28 additions and 87 deletions.
diff --git a/...main/java/org/neo4j/gds/core/loading/nodeproperties/DoubleArrayNodePropertiesBuilder.java b/...main/java/org/neo4j/gds/core/loading/nodeproperties/DoubleArrayNodePropertiesBuilder.java
@@ -28,9 +28,7 @@
 import org.neo4j.gds.core.concurrency.DefaultPool;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
 import org.neo4j.gds.utils.GdsNeo4jValueConversion;
-import org.neo4j.gds.utils.Neo4jValueConversion;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 import java.util.Arrays;
 import java.util.stream.Collectors;
@@ -57,11 +55,6 @@ public void set(long neoNodeId, double[] value) {
         builder.set(neoNodeId, value);
     }
 
-    @Override
-    public void setValue(long neoNodeId, Value value) {
-        set(neoNodeId, Neo4jValueConversion.getDoubleArray(value));
-    }
-
     @Override
     public void setValue(long neoNodeId, GdsValue value) {
         set(neoNodeId, GdsNeo4jValueConversion.getDoubleArray(value));

diff --git a/.../src/main/java/org/neo4j/gds/core/loading/nodeproperties/DoubleNodePropertiesBuilder.java b/.../src/main/java/org/neo4j/gds/core/loading/nodeproperties/DoubleNodePropertiesBuilder.java
@@ -28,9 +28,7 @@
 import org.neo4j.gds.core.concurrency.DefaultPool;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
 import org.neo4j.gds.utils.GdsNeo4jValueConversion;
-import org.neo4j.gds.utils.Neo4jValueConversion;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
@@ -80,12 +78,6 @@ public void set(long neoNodeId, double value) {
         updateMaxValue(value);
     }
 
-    @Override
-    public void setValue(long neoNodeId, Value value) {
-        double doubleValue = Neo4jValueConversion.getDoubleValue(value);
-        set(neoNodeId, doubleValue);
-    }
-
     @Override
     public void setValue(long neoNodeId, GdsValue value) {
         double doubleValue = GdsNeo4jValueConversion.getDoubleValue(value);

diff --git a/.../main/java/org/neo4j/gds/core/loading/nodeproperties/FloatArrayNodePropertiesBuilder.java b/.../main/java/org/neo4j/gds/core/loading/nodeproperties/FloatArrayNodePropertiesBuilder.java
@@ -28,9 +28,7 @@
 import org.neo4j.gds.core.concurrency.DefaultPool;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
 import org.neo4j.gds.utils.GdsNeo4jValueConversion;
-import org.neo4j.gds.utils.Neo4jValueConversion;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 import java.util.Arrays;
 import java.util.stream.Collectors;
@@ -55,11 +53,6 @@ public void set(long neoNodeId, float[] value) {
         builder.set(neoNodeId, value);
     }
 
-    @Override
-    public void setValue(long neoNodeId, Value value) {
-        set(neoNodeId, Neo4jValueConversion.getFloatArray(value));
-    }
-
     @Override
     public void setValue(long neoNodeId, GdsValue value) {
         set(neoNodeId, GdsNeo4jValueConversion.getFloatArray(value));

diff --git a/core/src/main/java/org/neo4j/gds/core/loading/nodeproperties/InnerNodePropertiesBuilder.java b/core/src/main/java/org/neo4j/gds/core/loading/nodeproperties/InnerNodePropertiesBuilder.java
@@ -22,12 +22,9 @@
 import org.neo4j.gds.api.PartialIdMap;
 import org.neo4j.gds.api.properties.nodes.NodePropertyValues;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 public interface InnerNodePropertiesBuilder {
 
-    void setValue(long neoNodeId, Value value);
-
     void setValue(long neoNodeId, GdsValue value);
 
     /**

diff --git a/...c/main/java/org/neo4j/gds/core/loading/nodeproperties/LongArrayNodePropertiesBuilder.java b/...c/main/java/org/neo4j/gds/core/loading/nodeproperties/LongArrayNodePropertiesBuilder.java
@@ -28,9 +28,7 @@
 import org.neo4j.gds.core.concurrency.DefaultPool;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
 import org.neo4j.gds.utils.GdsNeo4jValueConversion;
-import org.neo4j.gds.utils.Neo4jValueConversion;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 import java.util.Arrays;
 import java.util.stream.Collectors;
@@ -55,11 +53,6 @@ public void set(long neoNodeId, long[] value) {
         builder.set(neoNodeId, value);
     }
 
-    @Override
-    public void setValue(long neoNodeId, Value value) {
-        set(neoNodeId, Neo4jValueConversion.getLongArray(value));
-    }
-
     @Override
     public void setValue(long neoNodeId, GdsValue value) {
         set(neoNodeId, GdsNeo4jValueConversion.getLongArray(value));

diff --git a/core/src/main/java/org/neo4j/gds/core/loading/nodeproperties/LongNodePropertiesBuilder.java b/core/src/main/java/org/neo4j/gds/core/loading/nodeproperties/LongNodePropertiesBuilder.java
@@ -29,9 +29,7 @@
 import org.neo4j.gds.core.concurrency.DefaultPool;
 import org.neo4j.gds.core.concurrency.ParallelUtil;
 import org.neo4j.gds.utils.GdsNeo4jValueConversion;
-import org.neo4j.gds.utils.Neo4jValueConversion;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.Value;
 
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.VarHandle;
@@ -88,12 +86,6 @@ public void set(long neoNodeId, long value) {
         updateMaxValue(value);
     }
 
-    @Override
-    public void setValue(long neoNodeId, Value value) {
-        var longValue = Neo4jValueConversion.getLongValue(value);
-        set(neoNodeId, longValue);
-    }
-
     @Override
     public void setValue(long neoNodeId, GdsValue value) {
         var longValue = GdsNeo4jValueConversion.getLongValue(value);

diff --git a/...c/main/java/org/neo4j/gds/core/loading/nodeproperties/NodePropertiesFromStoreBuilder.java b/...c/main/java/org/neo4j/gds/core/loading/nodeproperties/NodePropertiesFromStoreBuilder.java
@@ -30,18 +30,11 @@
 import org.neo4j.gds.mem.MemoryEstimations;
 import org.neo4j.gds.values.GdsNoValue;
 import org.neo4j.gds.values.GdsValue;
-import org.neo4j.values.storable.DoubleArray;
-import org.neo4j.values.storable.FloatArray;
-import org.neo4j.values.storable.FloatingPointValue;
-import org.neo4j.values.storable.IntegralValue;
-import org.neo4j.values.storable.LongArray;
-import org.neo4j.values.storable.Value;
-import org.neo4j.values.storable.Values;
+import org.neo4j.gds.values.primitive.PrimitiveValues;
 
 import java.util.concurrent.atomic.AtomicReference;
 
 import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
-import static org.neo4j.values.storable.Values.NO_VALUE;
 
 public final class NodePropertiesFromStoreBuilder {
 
@@ -80,15 +73,6 @@ private NodePropertiesFromStoreBuilder(
         this.innerBuilder = new AtomicReference<>();
     }
 
-    public void set(long neoNodeId, Value value) {
-        if (value != null && value != NO_VALUE) {
-            if (innerBuilder.get() == null) {
-                initializeWithType(value);
-            }
-            innerBuilder.get().setValue(neoNodeId, value);
-        }
-    }
-
     public void set(long neoNodeId, GdsValue value) {
         if (value != null && value != GdsNoValue.NO_VALUE) {
             if (innerBuilder.get() == null) {
@@ -101,7 +85,8 @@ public void set(long neoNodeId, GdsValue value) {
     public NodePropertyValues build(IdMap idMap) {
         if (innerBuilder.get() == null) {
             if (defaultValue.getObject() != null) {
-                initializeWithType(Values.of(defaultValue.getObject()));
+                var gdsValue = PrimitiveValues.create(defaultValue.getObject());
+                initializeWithType(gdsValue);
             } else {
                 throw new IllegalStateException("Cannot infer type of property");
             }
@@ -116,15 +101,6 @@ public NodePropertyValues build(IdMap idMap) {
         return innerBuilder.get().build(idMap.nodeCount(), actualIdMap, idMap.highestOriginalId());
     }
 
-    // This is synchronized as we want to prevent the creation of multiple InnerNodePropertiesBuilders of which only once survives.
-    private synchronized void initializeWithType(Value value) {
-        if (innerBuilder.get() == null) {
-            var valueType = valueType(value);
-            var newBuilder = newInnerBuilder(valueType);
-            innerBuilder.compareAndSet(null, newBuilder);
-        }
-    }
-
     // This is synchronized as we want to prevent the creation of multiple InnerNodePropertiesBuilders of which only once survives.
     private synchronized void initializeWithType(GdsValue value) {
         if (innerBuilder.get() == null) {
@@ -152,23 +128,4 @@ private InnerNodePropertiesBuilder newInnerBuilder(ValueType valueType) {
                 ));
         }
     }
-
-    private ValueType valueType(Value value) {
-        if (value instanceof IntegralValue) {
-            return ValueType.LONG;
-        } else if (value instanceof FloatingPointValue) {
-            return ValueType.DOUBLE;
-        } else if (value instanceof LongArray) {
-            return ValueType.LONG_ARRAY;
-        } else if (value instanceof DoubleArray) {
-            return ValueType.DOUBLE_ARRAY;
-        } else if (value instanceof FloatArray) {
-            return ValueType.FLOAT_ARRAY;
-        } else {
-            throw new UnsupportedOperationException(formatWithLocale(
-                "Loading of values of type %s is currently not supported",
-                value.getTypeName()
-            ));
-        }
-    }
 }
diff --git a/native-projection/src/main/java/org/neo4j/gds/projection/NativeNodePropertyImporter.java b/native-projection/src/main/java/org/neo4j/gds/projection/NativeNodePropertyImporter.java
@@ -32,12 +32,18 @@
 import org.neo4j.gds.config.ConcurrencyConfig;
 import org.neo4j.gds.core.GraphDimensions;
 import org.neo4j.gds.core.concurrency.Concurrency;
+import org.neo4j.gds.core.loading.GdsNeo4jValueConverter;
 import org.neo4j.gds.core.loading.NodeLabelTokenSet;
 import org.neo4j.gds.core.loading.nodeproperties.NodePropertiesFromStoreBuilder;
 import org.neo4j.internal.kernel.api.PropertyCursor;
 import org.neo4j.kernel.api.KernelTransaction;
 import org.neo4j.storageengine.api.PropertySelection;
 import org.neo4j.storageengine.api.Reference;
+import org.neo4j.values.storable.DoubleArray;
+import org.neo4j.values.storable.FloatArray;
+import org.neo4j.values.storable.FloatingPointValue;
+import org.neo4j.values.storable.IntegralValue;
+import org.neo4j.values.storable.LongArray;
 import org.neo4j.values.storable.Value;
 
 import java.util.ArrayList;
@@ -49,6 +55,7 @@
 import static java.util.stream.Collectors.toMap;
 import static org.neo4j.gds.core.GraphDimensions.ANY_LABEL;
 import static org.neo4j.gds.core.GraphDimensions.IGNORE;
+import static org.neo4j.gds.utils.StringFormatting.formatWithLocale;
 
 public final class NativeNodePropertyImporter {
 
@@ -142,14 +149,31 @@ private int setPropertyValue(
             Value value = propertyCursor.propertyValue();
 
             for (NodePropertiesFromStoreBuilder builder : builders) {
-                builder.set(neoNodeId, value);
+                verifyValueType(value);
+                var gdsValue = GdsNeo4jValueConverter.toValue(value);
+                builder.set(neoNodeId, gdsValue);
                 propertiesImported++;
             }
         }
 
         return propertiesImported;
     }
 
+    private void verifyValueType(Value value) {
+        if (!(
+            value instanceof IntegralValue ||
+                value instanceof FloatingPointValue ||
+                value instanceof LongArray ||
+                value instanceof DoubleArray ||
+                value instanceof FloatArray
+        )) {
+            throw new UnsupportedOperationException(formatWithLocale(
+                "Loading of values of type %s is currently not supported",
+                value.getTypeName()
+            ));
+        }
+    }
+
     public static final class Builder {
         private Concurrency concurrency = ConcurrencyConfig.TYPED_DEFAULT_CONCURRENCY;
         private Map<NodeLabel, PropertyMappings> propertyMappings;