Skip to content

Commit

Permalink
Fixed bug with spatial inequalities including points on the axes
Browse files Browse the repository at this point in the history
  • Loading branch information
@craigtaverner
craigtaverner committed Aug 24, 2018
1 parent 56e01d1 commit 330c8b4
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 14 deletions.
38 changes: 28 additions & 10 deletions community/values/src/main/java/org/neo4j/values/storable/PointValue.java
View file
Expand Up @@ -276,19 +276,37 @@ public CRS getCRS()
*/ */
public boolean withinRange( PointValue lower, boolean includeLower, PointValue upper, boolean includeUpper ) public boolean withinRange( PointValue lower, boolean includeLower, PointValue upper, boolean includeUpper )
{ {
if ( lower != null ) if ( lower == null && upper == null )
{ {
Comparison compareLower = this.unsafeTernaryCompareTo( lower ); return false;
if ( compareLower == Comparison.UNDEFINED || compareLower == Comparison.SMALLER_THAN || compareLower == Comparison.EQUAL && !includeLower ) }
{ if ( (lower != null) && (this.crs.getCode() != lower.crs.getCode() || this.coordinate.length != lower.coordinate.length) )
return false; {
} return false;
}
if ( (upper != null) && (this.crs.getCode() != upper.crs.getCode() || this.coordinate.length != upper.coordinate.length) )
{
return false;
} }
if ( upper != null )
for ( int i = 0; i < coordinate.length; i++ )
{ {
Comparison compareUpper = this.unsafeTernaryCompareTo( upper ); if ( lower != null )
return compareUpper != Comparison.UNDEFINED && compareUpper != Comparison.GREATER_THAN && {
(compareUpper != Comparison.EQUAL || includeUpper); int cmpVal = Double.compare( this.coordinate[i], lower.coordinate[i] );
if ( !includeLower && cmpVal == 0 || cmpVal < 0 )
{
return false;
}
}
if ( upper != null )
{
int cmpVal = Double.compare( this.coordinate[i], upper.coordinate[i] );
if ( !includeUpper && cmpVal == 0 || cmpVal > 0 )
{
return false;
}
}
} }
return true; return true;
} }
Expand Down
92 changes: 88 additions & 4 deletions ...c/test/scala/org/neo4j/internal/cypher/acceptance/SpatialIndexResultsAcceptanceTest.scala
View file
Expand Up @@ -48,8 +48,11 @@ class SpatialIndexResultsAcceptanceTest extends IndexingTestSupport {
assertRangeScanFor("<=", point, node) assertRangeScanFor("<=", point, node)
assertLabelRangeScanFor("<=", point, node) assertLabelRangeScanFor("<=", point, node)


assertRangeScanFor("<", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 0), node) assertRangeScanFor("<", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 1), node)
assertLabelRangeScanFor("<", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 0), node) assertLabelRangeScanFor("<", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 1), node)

assertRangeScanFor("<=", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 0), node)
assertLabelRangeScanFor("<=", Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 0), node)
} }


test("indexed point should be readable from node property") { test("indexed point should be readable from node property") {
Expand Down Expand Up @@ -513,7 +516,10 @@ class SpatialIndexResultsAcceptanceTest extends IndexingTestSupport {
assertRangeScanFor(">=", originPoint, origin, upRightTop) assertRangeScanFor(">=", originPoint, origin, upRightTop)
assertRangeScanFor("<", originPoint, downLeftBottom) assertRangeScanFor("<", originPoint, downLeftBottom)
assertRangeScanFor("<=", originPoint, origin, downLeftBottom) assertRangeScanFor("<=", originPoint, origin, downLeftBottom)
assertRangeScanFor(">", minPoint, "<", maxPoint, origin, downLeftTop, downRightBottom, downrightTop, upLeftBottom, upLeftTop, upRightBottom) assertRangeScanFor(">=", minPoint, "<=", maxPoint, origin, upRightTop, downLeftBottom, downLeftTop, downRightBottom, downrightTop, upLeftBottom, upLeftTop, upRightBottom)
assertRangeScanFor(">", minPoint, "<=", maxPoint, origin, upRightTop)
assertRangeScanFor(">=", minPoint, "<", maxPoint, origin, downLeftBottom)
assertRangeScanFor(">", minPoint, "<", maxPoint, origin)
} }


test("indexed points 3D WGS84 space - range queries") { test("indexed points 3D WGS84 space - range queries") {
Expand All @@ -540,6 +546,84 @@ class SpatialIndexResultsAcceptanceTest extends IndexingTestSupport {
assertRangeScanFor(">=", midPoint, n0, n1) assertRangeScanFor(">=", midPoint, n0, n1)
assertRangeScanFor("<", midPoint, n8) assertRangeScanFor("<", midPoint, n8)
assertRangeScanFor("<=", midPoint, n0, n8) assertRangeScanFor("<=", midPoint, n0, n8)
assertRangeScanFor(">", minPoint, "<", maxPoint, n0, n2, n3, n4, n5, n6, n7) assertRangeScanFor(">=", minPoint, "<=", maxPoint, n0, n1, n2, n3, n4, n5, n6, n7, n8)
assertRangeScanFor(">", minPoint, "<=", maxPoint, n0, n1)
assertRangeScanFor(">=", minPoint, "<", maxPoint, n0, n8)
assertRangeScanFor(">", minPoint, "<", maxPoint, n0)
}

test("Range query should return points greater on both axes or less on both axes") {
// Given nodes at the search point, above and below it, and on the axes intersecting it
createIndex()
val nodeAbove15 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1.2345, 5.4321))
val nodeBelow15 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 0.2345, 4.4321))
val nodeAt15 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 5))
val nodeAt25 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 2, 5))
val nodeAt16 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 6))
val nodeAt05 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 0, 5))
val nodeAt14 = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 4))
createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 0.2345, 5.4321))
createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1.2345, 4.4321))

Map(
">=" -> Set(nodeAbove15, nodeAt15, nodeAt16, nodeAt25),
"<=" -> Set(nodeBelow15, nodeAt15, nodeAt05, nodeAt14),
">" -> Set(nodeAbove15),
"<" -> Set(nodeBelow15)
).toList.foreach {
case (op, expected) =>
// When
val includingBorder = innerExecuteDeprecated(s"MATCH (n:Label) WHERE n.prop $op point({x: 1, y: 5}) RETURN n")

// Then
val expectAxes = op contains "="
withClue(s"Should ${if(expectAxes) "" else "NOT "}find nodes that are on the axes defined by the search point when using operator '$op'") {
includingBorder.toList.map(_ ("n")).toSet should be(expected)
}
}
// When running a spatial range query for points greater than or equal to the search point
val includingBorder = innerExecuteDeprecated("MATCH (n:Label) WHERE n.prop >= point({x: 1, y: 5}) RETURN n")

// Then expect to also find nodes on the intersecting axes
withClue("Should find nodes that are on the axes defined by the search point") {
includingBorder.toList.map(_ ("n")).toSet should be(Set(nodeAbove15, nodeAt15, nodeAt16, nodeAt25))
}

// When running a spatial range query for points only greater than the search point
val excludingBorder = innerExecuteDeprecated("MATCH (n:Label) WHERE n.prop > point({x: 1, y: 5}) RETURN n")

// Then expect to find nodes above the search point and not on the intersecting axes
withClue("Should NOT find nodes that are on the axes defined by the search point") {
excludingBorder.toList.map(_ ("n")).toSet should be(Set(nodeAbove15))
}
}

test("Bounding box query on regular grid should not return points on the edges") {
// Given
createIndex()
val grid = Range(-10, 10).map { x =>
Range(-10, 10).map { y =>
createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, x, y))
}
}
val nodeToFind = createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 1.2345, 5.4321))
createIndexedNode(Values.pointValue(CoordinateReferenceSystem.Cartesian, 5.4321, 1.2345))
val vertices = Seq(grid(11)(15), grid(11)(16), grid(12)(16), grid(12)(15))

// When running a bounding box query we expect to include all or none of the border points
val includingBorder = innerExecuteDeprecated("MATCH (n:Label) WHERE point({x: 1, y: 5}) <= n.prop <= point({x: 2, y: 6}) RETURN n")
withClue("Should find nodes that are on the axes defined by the search point") {
includingBorder.toList.size should be(5)
}
val excludingBorder = innerExecuteDeprecated("MATCH (n:Label) WHERE point({x: 1, y: 5}) < n.prop < point({x: 2, y: 6}) RETURN n")
withClue("Should find nodes that are on NOT the axes defined by the search point") {
excludingBorder.toList.size should be(1)
}

// And when using the range scan assertions we should find the same results
val minPoint = Values.pointValue(CoordinateReferenceSystem.Cartesian, 1, 5)
val maxPoint = Values.pointValue(CoordinateReferenceSystem.Cartesian, 2, 6)
assertRangeScanFor(">=", minPoint, "<=", maxPoint, vertices :+ nodeToFind: _*)
assertRangeScanFor(">", minPoint, "<", maxPoint, nodeToFind)
} }
} }

0 comments on commit 330c8b4

Please sign in to comment.