diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/CastSupport.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/CastSupport.scala index ddc0d3575911..a995b062f602 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/CastSupport.scala +++ b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/CastSupport.scala @@ -91,7 +91,14 @@ object CastSupport { case (_: FloatValue, _: NumberValue) => a case (_: DoubleValue, _: NumberValue) => a - case (_: PointValue, _: PointValue) => a + case (p1: PointValue, p2: PointValue) => + if (p1.getCoordinateReferenceSystem != p2.getCoordinateReferenceSystem) { + throw new CypherTypeException("Collections containing point values with different CRS can not be stored in properties."); + } else if(p1.coordinate().length != p2.coordinate().length) { + throw new CypherTypeException("Collections containing point values with different dimensions can not be stored in properties."); + } else { + p1 + } case (a, b) if a == Values.NO_VALUE || b == Values.NO_VALUE => throw new CypherTypeException( "Collections containing null values can not be stored in properties.") diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/GeometryType.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/GeometryType.java index cdc88afbaa77..aafb9809601b 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/store/GeometryType.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/store/GeometryType.java @@ -256,13 +256,26 @@ public static long[] encodePoint( int keyId, CoordinateReferenceSystem crs, doub public static byte[] encodePointArray( PointValue[] points ) { int dimension = points[0].coordinate().length; + CoordinateReferenceSystem crs = points[0].getCoordinateReferenceSystem(); + for ( int i = 1; i < points.length; i++ ) + { + if ( dimension != points[i].coordinate().length ) + { + throw new IllegalArgumentException( + "Attempting to store array of points with inconsistent dimension. Point " + i + " has a different dimension." ); + } + if ( !crs.equals( points[i].getCoordinateReferenceSystem() ) ) + { + throw new IllegalArgumentException( "Attempting to store array of points with inconsistent CRS. Point " + i + " has a different CRS." ); + } + } + double[] data = new double[points.length * dimension]; for ( int i = 0; i < data.length; i++ ) { data[i] = points[i / dimension].coordinate()[i % dimension]; } - GeometryHeader geometryHeader = new GeometryHeader( GeometryType.GEOMETRY_POINT.gtype, dimension, - points[0].getCoordinateReferenceSystem() ); + GeometryHeader geometryHeader = new GeometryHeader( GeometryType.GEOMETRY_POINT.gtype, dimension, crs ); byte[] bytes = DynamicArrayStore.encodeFromNumbers( data, DynamicArrayStore.GEOMETRY_HEADER_SIZE ); geometryHeader.writeArrayHeaderTo( bytes ); return bytes; @@ -300,8 +313,8 @@ private void writeArrayHeaderTo( byte[] bytes ) bytes[1] = (byte) geometryType; bytes[2] = (byte) dimension; bytes[3] = (byte) crs.table.getTableId(); - bytes[4] = (byte) (crs.code & 0xFFL); - bytes[5] = (byte) (crs.code >> 8 & 0xFFL); + bytes[4] = (byte) (crs.code >> 8 & 0xFFL); + bytes[5] = (byte) (crs.code & 0xFFL); } static GeometryHeader fromArrayHeaderBytes( byte[] header ) @@ -309,7 +322,7 @@ static GeometryHeader fromArrayHeaderBytes( byte[] header ) int geometryType = Byte.toUnsignedInt( header[1] ); int dimension = Byte.toUnsignedInt( header[2] ); int crsTableId = Byte.toUnsignedInt( header[3] ); - int crsCode = Byte.toUnsignedInt( header[4] ) + (Byte.toUnsignedInt( header[5] ) << 8); + int crsCode = Byte.toUnsignedInt( header[5] ) + (Byte.toUnsignedInt( header[4] ) << 8); return new GeometryHeader( geometryType, dimension, crsTableId, crsCode ); } @@ -318,7 +331,7 @@ public static GeometryHeader fromArrayHeaderByteBuffer( ByteBuffer buffer ) int geometryType = Byte.toUnsignedInt( buffer.get() ); int dimension = Byte.toUnsignedInt( buffer.get() ); int crsTableId = Byte.toUnsignedInt( buffer.get() ); - int crsCode = Byte.toUnsignedInt( buffer.get() ) + (Byte.toUnsignedInt( buffer.get() ) << 8); + int crsCode = (Byte.toUnsignedInt( buffer.get() ) << 8) + Byte.toUnsignedInt( buffer.get() ); return new GeometryHeader( geometryType, dimension, crsTableId, crsCode ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestArrayStore.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestArrayStore.java index a5fdbcecda33..2c8af27a15b8 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestArrayStore.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/TestArrayStore.java @@ -44,6 +44,9 @@ import org.neo4j.test.rule.PageCacheRule; import org.neo4j.test.rule.TestDirectory; import org.neo4j.test.rule.fs.DefaultFileSystemRule; +import org.neo4j.values.storable.CoordinateReferenceSystem; +import org.neo4j.values.storable.PointValue; +import org.neo4j.values.storable.Values; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -99,6 +102,15 @@ public void longArrayPropertiesShouldBeBitPacked() throws Exception PropertyType.LONG, 34 ); } + @Test + public void doubleArrayPropertiesShouldBeBitPacked() throws Exception + { + assertBitPackedArrayGetsCorrectlySerializedAndDeserialized( new double[]{Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x7L )}, + PropertyType.DOUBLE, 3 ); + assertBitPackedArrayGetsCorrectlySerializedAndDeserialized( new double[]{Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x8L )}, + PropertyType.DOUBLE, 4 ); + } + @Test public void byteArrayPropertiesShouldNotBeBitPacked() throws Exception { @@ -128,23 +140,109 @@ public void stringArrayGetsStoredAsUtf8() throws Exception } } + @Test + public void pointArraysOfWgs84() throws Exception + { + PointValue[] array = + new PointValue[]{Values.pointValue( CoordinateReferenceSystem.WGS84, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x7L ) ), + Values.pointValue( CoordinateReferenceSystem.WGS84, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x1L ) )}; + int numberOfBitsUsedForDoubles = 3; + + assertPointArrayHasCorrectFormat( array, numberOfBitsUsedForDoubles ); + } + + @Test + public void pointArraysOfCartesian() throws Exception + { + PointValue[] array = + new PointValue[]{Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x7L ) ), + Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x1L ) )}; + int numberOfBitsUsedForDoubles = 3; + + assertPointArrayHasCorrectFormat( array, numberOfBitsUsedForDoubles ); + } + + @Test( expected = IllegalArgumentException.class ) + public void pointArraysOfMixedCRS() throws Exception + { + PointValue[] array = + new PointValue[]{Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x7L ) ), + Values.pointValue( CoordinateReferenceSystem.WGS84, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x1L ) )}; + + Collection records = new ArrayList<>(); + arrayStore.allocateRecords( records, array ); + } + + @Test( expected = IllegalArgumentException.class ) + public void pointArraysOfMixedDimension() throws Exception + { + PointValue[] array = + new PointValue[]{Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x7L ) ), + Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.longBitsToDouble( 0x1L ), Double.longBitsToDouble( 0x1L ), + Double.longBitsToDouble( 0x4L ) )}; + + Collection records = new ArrayList<>(); + arrayStore.allocateRecords( records, array ); + } + + private void assertPointArrayHasCorrectFormat( PointValue[] array, int numberOfBitsUsedForDoubles ) + { + Collection records = new ArrayList<>(); + arrayStore.allocateRecords( records, array ); + Pair loaded = loadArray( records ); + assertGeometryHeader( loaded.first(), GeometryType.GEOMETRY_POINT.gtype, 2, array[0].getCoordinateReferenceSystem().table.getTableId(), + array[0].getCoordinateReferenceSystem().code ); + + final int dimension = array[0].coordinate().length; + double[] pointDoubles = new double[array.length * dimension]; + for ( int i = 0; i < pointDoubles.length; i++ ) + { + pointDoubles[i] = array[i / dimension].coordinate()[i % dimension]; + } + + byte[] doubleHeader = Arrays.copyOf( loaded.other(), DynamicArrayStore.NUMBER_HEADER_SIZE ); + byte[] doubleBody = Arrays.copyOfRange( loaded.other(), DynamicArrayStore.NUMBER_HEADER_SIZE, loaded.other().length ); + assertNumericArrayHeaderAndContent( pointDoubles, PropertyType.DOUBLE, numberOfBitsUsedForDoubles, Pair.of( doubleHeader, doubleBody ) ); + } + private void assertStringHeader( byte[] header, int itemCount ) { assertEquals( PropertyType.STRING.byteValue(), header[0] ); assertEquals( itemCount, ByteBuffer.wrap( header, 1, 4 ).getInt() ); } + private void assertGeometryHeader( byte[] header, int geometryTpe, int dimension, int crsTableId, int crsCode ) + { + assertEquals( PropertyType.GEOMETRY.byteValue(), header[0] ); + assertEquals( geometryTpe, header[1] ); + assertEquals( dimension, header[2] ); + assertEquals( crsTableId, header[3] ); + assertEquals( crsCode, ByteBuffer.wrap( header, 4, 2 ).getShort() ); + } + private void assertBitPackedArrayGetsCorrectlySerializedAndDeserialized( Object array, PropertyType type, int expectedBitsUsedPerItem ) { Collection records = storeArray( array ); Pair asBytes = loadArray( records ); - assertArrayHeader( asBytes.first(), type, expectedBitsUsedPerItem ); - Bits bits = Bits.bitsFromBytes( asBytes.other() ); + assertNumericArrayHeaderAndContent( array, type, expectedBitsUsedPerItem, asBytes ); + } + + private void assertNumericArrayHeaderAndContent( Object array, PropertyType type, int expectedBitsUsedPerItem, Pair loadedBytesFromStore ) + { + assertArrayHeader( loadedBytesFromStore.first(), type, expectedBitsUsedPerItem ); + Bits bits = Bits.bitsFromBytes( loadedBytesFromStore.other() ); int length = Array.getLength( array ); for ( int i = 0; i < length; i++ ) { - assertEquals( ((Number)Array.get( array, i )).longValue(), bits.getLong( expectedBitsUsedPerItem ) ); + if ( array instanceof double[] ) + { + assertEquals( Double.doubleToLongBits( Array.getDouble( array, i ) ), bits.getLong( expectedBitsUsedPerItem ) ); + } + else + { + assertEquals( Array.getLong( array, i ), bits.getLong( expectedBitsUsedPerItem ) ); + } } } 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 2233dc969bf1..5392e784bec4 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 @@ -361,7 +361,7 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher )))) } - test("array of points should be readable from node property") { + test("array of cartesian points should be readable from node property") { // Given createLabeledNode("Place") graph.execute( @@ -386,4 +386,47 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher CartesianPoint(3.0, 3.0, CRS.Cartesian) )) } + + test("array of wgs84 points should be readable from node property") { + // Given + createLabeledNode("Place") + graph.execute( + """ + |UNWIND [1,2,3] as num + |WITH point({latitude: num, longitude: num}) as p + |WITH collect(p) as points + |MATCH (place:Place) SET place.location = points + |RETURN place.location as points + """.stripMargin) + + // When + val result = executeWith(Configs.All, "MATCH (p:Place) RETURN p.location as points", + planComparisonStrategy = ComparePlansWithAssertion(_ should useOperatorWithText("Projection", "point"), + expectPlansToFail = Configs.AllRulePlanners)) + + // Then + val points = result.columnAs("points").toList.head.asInstanceOf[Array[_]] + points should equal(Array( + CartesianPoint(1.0, 1.0, CRS.WGS84), + CartesianPoint(2.0, 2.0, CRS.WGS84), + CartesianPoint(3.0, 3.0, CRS.WGS84) + )) + } + + test("array of mixed points should not be assignable to node property") { + // Given + createLabeledNode("Place") + + // When + val config = expectedToSucceed - Configs.SlottedInterpreted - Configs.Cost3_1 - Configs.AllRulePlanners + val query = + """ + |WITH [point({x: 1, y: 2}), point({latitude: 1, longitude: 2})] as points + |MATCH (place:Place) SET place.location = points + |RETURN points + """.stripMargin + + // Then + failWithError(config + Configs.Procs, query, Seq("Collections containing point values with different CRS can not be stored in properties.")) + } }