Support 3D Cartesian points in point() and distance() functions

community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/DistanceFunction.scala

@@ -64,14 +64,17 @@ trait DistanceCalculator extends PartialFunction[(PointValue, PointValue), Doubl
 object CartesianCalculator extends DistanceCalculator {
   override def isDefinedAt(points: (PointValue, PointValue)): Boolean =
     points._1.getCoordinateReferenceSystem.getCode() == CoordinateReferenceSystem.Cartesian.getCode() &&
-      points._2.getCoordinateReferenceSystem.getCode() == CoordinateReferenceSystem.Cartesian.getCode()
+      points._2.getCoordinateReferenceSystem.getCode() == CoordinateReferenceSystem.Cartesian.getCode() &&
+      points._1.coordinate().length == points._2.coordinate().length
 
   override def apply(points: (PointValue, PointValue)): Double = {
     val p1Coordinates = points._1.coordinate()
     val p2Coordinates = points._2.coordinate()
-
-    sqrt((p2Coordinates(0) - p1Coordinates(0)) * (p2Coordinates(0) - p1Coordinates(0)) +
-      (p2Coordinates(1) - p1Coordinates(1)) * (p2Coordinates(1) - p1Coordinates(1)))
+    val sqrSum = p1Coordinates.zip(p2Coordinates).foldLeft(0.0) { (sum,pair) =>
+      val diff = pair._1 - pair._2
+      sum + diff * diff
+    }
+    sqrt(sqrSum)
   }
 
   override def boundingBox(p: PointValue, distance: Double): (PointValue, PointValue) = {

community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala

@@ -35,7 +35,7 @@ case class PointFunction(data: Expression) extends NullInNullOutExpression(data)
     case IsMap(mapCreator) =>
       val map = mapCreator(state.query)
       if (containsNull(map)) Values.NO_VALUE
-      else ValueUtils.fromMap(map)
+      else ValueUtils.pointFromMap(map)
     case x => throw new CypherTypeException(s"Expected a map but got $x")
   }
 

community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java

@@ -254,30 +254,31 @@ public static MapValue asMapValue( Map<String,Object> map )
         return map( mapValues( map ) );
     }
 
-    public static PointValue fromMap( MapValue map )
+    public static PointValue pointFromMap( MapValue map )
     {
         if ( map.containsKey( "x" ) && map.containsKey( "y" ) )
         {
             double x = ((NumberValue) map.get( "x" )).doubleValue();
             double y = ((NumberValue) map.get( "y" )).doubleValue();
-            if ( !map.containsKey( "crs" ) )
-            {
-                return Values.pointValue( CoordinateReferenceSystem.Cartesian, x, y );
-            }
-
-            TextValue crs = (TextValue) map.get( "crs" );
-            if ( crs.stringValue().equals( CoordinateReferenceSystem.Cartesian.getName() ) )
+            double[] coordinates = map.containsKey( "z" ) ? new double[]{x, y, ((NumberValue) map.get( "z" )).doubleValue()} : new double[]{x, y};
+            CoordinateReferenceSystem crs = CoordinateReferenceSystem.Cartesian;
+            if ( map.containsKey( "crs" ) )
             {
-                return Values.pointValue( CoordinateReferenceSystem.Cartesian, x, y );
-            }
-            else if ( crs.stringValue().equals( CoordinateReferenceSystem.WGS84.getName() ) )
-            {
-                return Values.pointValue( CoordinateReferenceSystem.WGS84, x, y );
-            }
-            else
-            {
-                throw new IllegalArgumentException( "Unknown coordinate reference system: " + crs.stringValue() );
+                TextValue crsName = (TextValue) map.get( "crs" );
+                if ( crsName.stringValue().equals( CoordinateReferenceSystem.Cartesian.getName() ) )
+                {
+                    crs = CoordinateReferenceSystem.Cartesian;
+                }
+                else if ( crsName.stringValue().equals( CoordinateReferenceSystem.WGS84.getName() ) )
+                {
+                    crs = CoordinateReferenceSystem.WGS84;
+                }
+                else
+                {
+                    throw new IllegalArgumentException( "Unknown coordinate reference system: " + crsName.stringValue() );
+                }
             }
+            return Values.pointValue( crs, coordinates );
         }
         else if ( map.containsKey( "latitude" ) && map.containsKey( "longitude" ) )
         {

enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialDistanceAcceptanceTest.scala

@@ -72,6 +72,19 @@ class SpatialDistanceAcceptanceTest extends ExecutionEngineFunSuite with CypherC
     Math.round(result.columnAs("dist").next().asInstanceOf[Double]) should equal(10116214)
   }
 
+  test("distance function should work on 3D points") {
+    val result = executeWith(pointConfig,
+      """
+        |WITH point({x: 1.2, y: 3.4, z: 5.6}) as p1, point({x: 1.2, y: 3.4, z: 6.6}) as p2
+        |RETURN distance(p1,p2) as dist
+      """.stripMargin,
+      expectedDifferentResults = Configs.Version3_1 + Configs.AllRulePlanners,  // TODO should rather throw error
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "p1", "p2", "dist"),
+        expectPlansToFail = Configs.AllRulePlanners))
+
+    Math.round(result.columnAs("dist").next().asInstanceOf[Double]) should equal(1)
+  }
+
   test("distance function should not fail if provided with points from different CRS") {
     val localConfig = pointConfig - Configs.OldAndRule
       val res = executeWith(localConfig,
@@ -80,6 +93,16 @@ class SpatialDistanceAcceptanceTest extends ExecutionEngineFunSuite with CypherC
     res.columnAs[AnyRef]("dist").next() should be (null)
   }
 
+  test("distance function should return null if provided with points with different dimensions") {
+    val result = executeWith(pointConfig,
+      """WITH point({x: 2.3, y: 4.5}) as p1, point({x: 1.2, y: 3.4, z: 5.6}) as p2
+        |RETURN distance(p1,p2) as dist""".stripMargin,
+      expectedDifferentResults = Configs.Version3_1 + Configs.AllRulePlanners // TODO should rather throw error
+    )
+    val dist = result.columnAs[Any]("dist").next()
+    assert(dist == null)
+  }
+
   test("distance function should measure distance from Copenhagen train station to Neo4j in Malmö") {
     val result = executeWith(pointConfig,
       """
@@ -133,6 +156,11 @@ class SpatialDistanceAcceptanceTest extends ExecutionEngineFunSuite with CypherC
     result.toList should equal(List(Map("dist" -> null)))
   }
 
+  test("distance function should fail on wrong type") {
+    val config = Configs.AbsolutelyAll + TestConfiguration(Versions.Default, Planners.Default, Runtimes.Default) - Configs.Version2_3
+    failWithError(config, "RETURN distance(1, 2) as dist", List("Type mismatch: expected Point or Geometry but was Integer"))
+  }
+
   test("points with distance query and mixed crs") {
     // Given
     graph.execute("CREATE (p:Place) SET p.location = point({y: 56.7, x: 12.78, crs: 'cartesian'})")

enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialFunctionsAcceptanceTest.scala

@@ -48,6 +48,15 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
     result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.Cartesian, 2.3, 4.5))))
   }
 
+  test("point function should work with literal map and 3D cartesian coordinates") {
+    val result = executeWith(pointConfig, "RETURN point({x: 2.3, y: 4.5, z: 6.7, crs: 'cartesian'}) as point",
+      expectedDifferentResults = Configs.Version3_1 + Configs.AllRulePlanners,
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "point"),
+        expectPlansToFail = Configs.AllRulePlanners))
+
+    result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.Cartesian, 2.3, 4.5, 6.7))))
+  }
+
   test("point function should work with literal map and geographic coordinates") {
     val result = executeWith(pointConfig, "RETURN point({longitude: 2.3, latitude: 4.5, crs: 'WGS-84'}) as point",
       planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "point"),
@@ -226,6 +235,37 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
     point.getCRS.getHref should equal("http://spatialreference.org/ref/epsg/4326/")
   }
 
+  test("3D point should be assignable to node property") {
+    // Given
+    createLabeledNode("Place")
+
+    // When
+    val config = pointConfig - Configs.Cost3_1 - Configs.AllRulePlanners - Configs.Morsel
+    val result = executeWith(config, "MATCH (p:Place) SET p.location = point({x: 1.2, y: 3.4, z: 5.6}) RETURN p.location as point",
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "point"),
+        expectPlansToFail = Configs.AllRulePlanners))
+
+    // Then
+    result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.Cartesian, 1.2, 3.4, 5.6))))
+  }
+
+  test("3D point should be readable from node property") {
+    // Given
+    createLabeledNode("Place")
+    graph.execute("MATCH (p:Place) SET p.location = point({x: 1.2, y: 3.4, z: 5.6}) RETURN p.location as point")
+
+    // When
+    val result = executeWith(Configs.All, "MATCH (p:Place) RETURN p.location as point",
+      planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "point"),
+        expectPlansToFail = Configs.AllRulePlanners))
+
+    // Then
+    val point = result.columnAs("point").toList.head.asInstanceOf[Point]
+    point should equal(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1.2, 3.4, 5.6))
+    // And CRS names should equal
+    point.getCRS.getHref should equal("http://spatialreference.org/ref/sr-org/7203/")
+  }
+
   // TODO add 3D here too
   test("inequality on cartesian points") {
     // case same point
@@ -281,7 +321,7 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
   private def shouldCompareLike(a: String, b: String, aBiggerB: Any, aSmallerB: Any) = {
     val query =
       s"""WITH $a as a, $b as b
-        |RETURN a > b, a < b
+         |RETURN a > b, a < b
       """.stripMargin
 
     val pointConfig = Configs.Interpreted - Configs.BackwardsCompatibility - Configs.AllRulePlanners

enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialIndexAcceptanceTest.scala

@@ -201,12 +201,7 @@ class SpatialIndexResultsAcceptanceTest extends ExecutionEngineFunSuite with Cyp
 
   test("indexed point should be readable from node property") {
     // Given
-    graph.inTx {
-      graph.schema().indexFor(Label.label("Place")).on("location").create()
-    }
-    graph.inTx {
-      graph.schema().awaitIndexesOnline(5, TimeUnit.SECONDS)
-    }
+    graph.createIndex("Place", "location")
     createLabeledNode("Place")
     graph.execute("MATCH (p:Place) SET p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'}) RETURN p.location as point")
 
@@ -228,12 +223,7 @@ class SpatialIndexResultsAcceptanceTest extends ExecutionEngineFunSuite with Cyp
 
   test("with multiple indexed points only exact match should be returned") {
     // Given
-    graph.inTx {
-      graph.schema().indexFor(Label.label("Place")).on("location").create()
-    }
-    graph.inTx {
-      graph.schema().awaitIndexesOnline(5, TimeUnit.SECONDS)
-    }
+    graph.createIndex("Place", "location")
     createLabeledNode("Place")
     graph.execute("MATCH (p:Place) SET p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'}) RETURN p.location as point")
     graph.execute("CREATE (p:Place) SET p.location = point({latitude: 40.7, longitude: -35.78, crs: 'WGS-84'})")