Skip to content

Commit

Permalink
Fixed spatial within to work with improver ternaryCompare
Browse files Browse the repository at this point in the history
  • Loading branch information
@craigtaverner
craigtaverner committed Aug 24, 2018
1 parent 0ccdd2c commit 2018e18
Show file tree
Hide file tree
Showing 4 changed files with 190 additions and 89 deletions.
21 changes: 7 additions & 14 deletions community/values/src/main/java/org/neo4j/values/Comparison.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public int value()
{ {
return 1; return 1;
} }

}, },
EQUAL EQUAL
{ {
Expand All @@ -43,7 +42,6 @@ public int value()
{ {
return 0; return 0;
} }

}, },
SMALLER_THAN SMALLER_THAN
{ {
Expand All @@ -52,18 +50,10 @@ public int value()
{ {
return -1; return -1;
} }

}, },
UNDEFINED GREATER_THAN_AND_EQUAL,
{ SMALLER_THAN_AND_EQUAL,
@Override UNDEFINED;
public int value()
{
throw new IllegalStateException(
"This value is undefined and can't handle primitive comparisons" );
}

};


/** /**
* Integer representation of comparison * Integer representation of comparison
Expand All @@ -76,7 +66,10 @@ public int value()
* if equal. * if equal.
* @throws IllegalStateException if the result is undefined. * @throws IllegalStateException if the result is undefined.
*/ */
public abstract int value(); public int value()
{
throw new IllegalStateException( "This value is undefined and can't handle primitive comparisons" );
}


/** /**
* Maps an integer value to comparison result. * Maps an integer value to comparison result.
Expand Down
88 changes: 56 additions & 32 deletions community/values/src/main/java/org/neo4j/values/storable/PointValue.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -167,26 +167,49 @@ Comparison unsafeTernaryCompareTo( Value otherValue )
return Comparison.UNDEFINED; return Comparison.UNDEFINED;
} }


int result = 0; int eq = 0;
int gt = 0;
int lt = 0;
for ( int i = 0; i < coordinate.length; i++ ) for ( int i = 0; i < coordinate.length; i++ )
{ {
int cmpVal = Double.compare( this.coordinate[i], other.coordinate[i] ); int cmpVal = Double.compare( this.coordinate[i], other.coordinate[i] );
if ( cmpVal == 0 && result != 0 ) if ( cmpVal > 0 )
{ {
// Equal on one dimension, but not others, is not defined gt++;
return Comparison.UNDEFINED;
} }
if ( cmpVal != result ) else if ( cmpVal < 0 )
{ {
if ( (cmpVal < 0 && result > 0) || (cmpVal > 0 && result < 0) || (i > 0 && result == 0) ) lt++;
{ }
return Comparison.UNDEFINED; else
} {
result = cmpVal; eq++;
} }
} }

if ( eq == coordinate.length )
return Comparison.from( result ); {
return Comparison.EQUAL;
}
else if ( gt == coordinate.length )
{
return Comparison.GREATER_THAN;
}
else if ( lt == coordinate.length )
{
return Comparison.SMALLER_THAN;
}
else if ( lt == 0 )
{
return Comparison.GREATER_THAN_AND_EQUAL;
}
else if ( gt == 0 )
{
return Comparison.SMALLER_THAN_AND_EQUAL;
}
else
{
return Comparison.UNDEFINED;
}
} }


@Override @Override
Expand Down Expand Up @@ -279,40 +302,41 @@ public CRS getCRS()
* @param includeUpper governs if the upper comparison should be inclusive * @param includeUpper governs if the upper comparison should be inclusive
* @return true if this value is within the described range * @return true if this value is within the described range
*/ */
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 && upper == null ) if ( lower == null && upper == null )
{ {
return true; return true;
} }
if ( (lower != null) && (this.crs.getCode() != lower.crs.getCode() || this.coordinate.length != lower.coordinate.length) )
{ if ( lower != null )
return false;
}
if ( (upper != null) && (this.crs.getCode() != upper.crs.getCode() || this.coordinate.length != upper.coordinate.length) )
{ {
return false; Comparison comparison = this.unsafeTernaryCompareTo( lower );
if ( comparison == Comparison.UNDEFINED )
{
return null;
}
else if ( comparison == Comparison.SMALLER_THAN || comparison == Comparison.SMALLER_THAN_AND_EQUAL ||
(comparison == Comparison.EQUAL || comparison == Comparison.GREATER_THAN_AND_EQUAL) && !includeLower )
{
return false;
}
} }


for ( int i = 0; i < coordinate.length; i++ ) if ( upper != null )
{ {
if ( lower != null ) Comparison comparison = this.unsafeTernaryCompareTo( upper );
if ( comparison == Comparison.UNDEFINED )
{ {
int cmpVal = Double.compare( this.coordinate[i], lower.coordinate[i] ); return null;
if ( !includeLower && cmpVal == 0 || cmpVal < 0 )
{
return false;
}
} }
if ( upper != null ) else if ( comparison == Comparison.GREATER_THAN || comparison == Comparison.GREATER_THAN_AND_EQUAL ||
(comparison == Comparison.EQUAL || comparison == Comparison.SMALLER_THAN_AND_EQUAL) && !includeUpper )
{ {
int cmpVal = Double.compare( this.coordinate[i], upper.coordinate[i] ); return false;
if ( !includeUpper && cmpVal == 0 || cmpVal > 0 )
{
return false;
}
} }
} }

return true; return true;
} }


Expand Down
85 changes: 77 additions & 8 deletions community/values/src/test/java/org/neo4j/values/storable/PointTest.java
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -114,22 +114,91 @@ public void shouldTernaryCompareTwoPoints()
equalTo( Comparison.UNDEFINED ) ); equalTo( Comparison.UNDEFINED ) );
assertThat( "Point greater on both dimensions is greater", pointValue( Cartesian, 2, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point greater on both dimensions is greater", pointValue( Cartesian, 2, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.GREATER_THAN ) ); equalTo( Comparison.GREATER_THAN ) );
assertThat( "Point greater on first dimensions is UNDEFINED", pointValue( Cartesian, 2, 2 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point greater on first dimensions is >=", pointValue( Cartesian, 2, 2 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.UNDEFINED ) ); equalTo( Comparison.GREATER_THAN_AND_EQUAL ) );
assertThat( "Point greater on second dimensions is UNDEFINED", pointValue( Cartesian, 1, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point greater on second dimensions is >=", pointValue( Cartesian, 1, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.UNDEFINED ) ); equalTo( Comparison.GREATER_THAN_AND_EQUAL ) );
assertThat( "Point smaller on both dimensions is smaller", pointValue( Cartesian, 0, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point smaller on both dimensions is smaller", pointValue( Cartesian, 0, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.SMALLER_THAN ) ); equalTo( Comparison.SMALLER_THAN ) );
assertThat( "Point smaller on first dimensions is UNDEFINED", pointValue( Cartesian, 0, 2 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point smaller on first dimensions is <=", pointValue( Cartesian, 0, 2 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.UNDEFINED ) ); equalTo( Comparison.SMALLER_THAN_AND_EQUAL ) );
assertThat( "Point smaller on second dimensions is UNDEFINED", pointValue( Cartesian, 1, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), assertThat( "Point smaller on second dimensions is <=", pointValue( Cartesian, 1, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ),
equalTo( Comparison.UNDEFINED ) ); equalTo( Comparison.SMALLER_THAN_AND_EQUAL ) );
assertThat( "Point greater on first and smaller on second dimensions is UNDEFINED", assertThat( "Point greater on first and smaller on second dimensions is UNDEFINED",
pointValue( Cartesian, 2, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), equalTo( Comparison.UNDEFINED ) ); pointValue( Cartesian, 2, 1 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), equalTo( Comparison.UNDEFINED ) );
assertThat( "Point smaller on first and greater on second dimensions is UNDEFINED", assertThat( "Point smaller on first and greater on second dimensions is UNDEFINED",
pointValue( Cartesian, 0, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), equalTo( Comparison.UNDEFINED ) ); pointValue( Cartesian, 0, 3 ).unsafeTernaryCompareTo( pointValue( Cartesian, 1, 2 ) ), equalTo( Comparison.UNDEFINED ) );
} }


@Test
public void shouldComparePointWithin()
{
// Edge cases
assertThat( "Always within no bounds", pointValue( Cartesian, 1, 2 ).withinRange( null, false, null, false ), equalTo( true ) );
assertThat( "Different CRS for lower bound should be undefined",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( WGS84, 1, 2 ), true, null, false ), equalTo( null ) );
assertThat( "Different CRS for upper bound should be undefined",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( WGS84, 1, 2 ), true ), equalTo( null ) );

// Lower bound
assertThat( "Within same lower bound if inclusive", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), true, null, false ),
equalTo( true ) );
assertThat( "Not within same lower bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), false, null, false ), equalTo( false ) );
assertThat( "Within smaller lower bound if inclusive", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), true, null, false ),
equalTo( true ) );
assertThat( "Within smaller lower bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), false, null, false ), equalTo( true ) );
assertThat( "Within partially smaller lower bound if inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 1 ), true, null, false ), equalTo( true ) );
assertThat( "Not within partially smaller lower bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 1 ), false, null, false ), equalTo( false ) );
assertThat( "Invalid if lower bound both greater and less than",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 2, 1 ), false, null, false ), equalTo( null ) );
assertThat( "Invalid if lower bound both greater and less than even when inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 2, 1 ), true, null, false ), equalTo( null ) );

// Upper bound
assertThat( "Within same upper bound if inclusive", pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 1, 2 ), true ),
equalTo( true ) );
assertThat( "Not within same upper bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 1, 2 ), false ), equalTo( false ) );
assertThat( "Within larger upper bound if inclusive", pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 3 ), true ),
equalTo( true ) );
assertThat( "Within larger upper bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 3 ), false ), equalTo( true ) );
assertThat( "Within partially larger upper bound if inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 2 ), true ), equalTo( true ) );
assertThat( "Not within partially larger upper bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 2 ), false ), equalTo( false ) );
assertThat( "Invalid if upper bound both greater and less than",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 1 ), false ), equalTo( null ) );
assertThat( "Invalid if upper bound both greater and less than even when inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( null, false, pointValue( Cartesian, 2, 1 ), true ), equalTo( null ) );

// Lower and upper bounds
assertThat( "Not within same bounds if inclusive on lower", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), true, pointValue( Cartesian, 1, 2 ), false ),
equalTo( false ) );
assertThat( "Not within same bounds if inclusive on upper", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), false, pointValue( Cartesian, 1, 2 ), true ),
equalTo( false ) );
assertThat( "Within same bounds if inclusive on both", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), true, pointValue( Cartesian, 1, 2 ), true ),
equalTo( true ) );
assertThat( "Not within same bounds if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 2 ), false, pointValue( Cartesian, 1, 2 ), false ), equalTo( false ) );
assertThat( "Within smaller lower bound if inclusive", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), true, pointValue( Cartesian, 1, 2 ), true ),
equalTo( true ) );
assertThat( "Within smaller lower bound if inclusive on upper", pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), false, pointValue( Cartesian, 1, 2 ), true ),
equalTo( true ) );
assertThat( "Not within smaller lower bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), false, pointValue( Cartesian, 1, 2 ), false ), equalTo( false ) );
assertThat( "Within partially smaller lower bound if inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 1 ), true, pointValue( Cartesian, 2, 3 ), false ), equalTo( true ) );
assertThat( "Not within partially smaller lower bound if not inclusive",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 1, 1 ), false, pointValue( Cartesian, 2, 3 ), false ), equalTo( false ) );
assertThat( "Within wider bounds",
pointValue( Cartesian, 1, 2 ).withinRange( pointValue( Cartesian, 0, 1 ), false, pointValue( Cartesian, 2, 3 ), false ), equalTo( true ) );
}

//------------------------------------------------------------- //-------------------------------------------------------------
// Parser tests // Parser tests


Expand Down

0 comments on commit 2018e18

Please sign in to comment.