diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/DistanceFunction.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/DistanceFunction.scala index 906eb28413fb8..e8555aa2820e0 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/DistanceFunction.scala +++ b/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) = { diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala index 87dfdbf576332..62c949eb61865 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/commands/expressions/PointFunction.scala +++ b/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") } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java index 3a36f6e318ddd..5e84efecd1424 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/util/ValueUtils.java @@ -254,30 +254,31 @@ public static MapValue asMapValue( Map 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" ) ) { diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialDistanceAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialDistanceAcceptanceTest.scala index 098c240afd80d..a9ebc681925c3 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialDistanceAcceptanceTest.scala +++ b/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'})") diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialFunctionsAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialFunctionsAcceptanceTest.scala index e916e4e37dd9f..d5fd7d940b228 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialFunctionsAcceptanceTest.scala +++ b/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 diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialIndexAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialIndexAcceptanceTest.scala index 1298fe2e35835..3373d35b87da9 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialIndexAcceptanceTest.scala +++ b/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'})")