Skip to content

Commit

Permalink
Use correct comparison for points to enable rangeSeek
Browse files Browse the repository at this point in the history
  • Loading branch information
@OliviaYtterbrink
OliviaYtterbrink committed Feb 14, 2018
1 parent d1d38d6 commit 1bb666a
Show file tree
Hide file tree
Showing 6 changed files with 113 additions and 34 deletions.
3 changes: 2 additions & 1 deletion ...-3.4/src/main/scala/org/neo4j/cypher/internal/v3_4/expressions/PredicateExpressions.scala
View file
Expand Up @@ -157,7 +157,8 @@ sealed trait InequalityExpression extends Expression with BinaryOperatorExpressi
TypeSignature(argumentTypes = Vector(CTInteger, CTFloat), outputType = CTBoolean),
TypeSignature(argumentTypes = Vector(CTInteger, CTInteger), outputType = CTBoolean),
TypeSignature(argumentTypes = Vector(CTFloat, CTFloat), outputType = CTBoolean),
TypeSignature(argumentTypes = Vector(CTString, CTString), outputType = CTBoolean)
TypeSignature(argumentTypes = Vector(CTString, CTString), outputType = CTBoolean),
TypeSignature(argumentTypes = Vector(CTPoint, CTPoint), outputType = CTBoolean)
)

def includeEquality: Boolean
Expand Down
1 change: 1 addition & 0 deletions ...g/neo4j/cypher/internal/runtime/interpreted/commands/predicates/ComparablePredicate.scala
View file
Expand Up @@ -40,6 +40,7 @@ abstract sealed class ComparablePredicate(val left: Expression, val right: Expre
case (n1: NumberValue, n2: NumberValue) => Some(compare(AnyValues.COMPARATOR.compare(n1, n2)))
case (n1: TextValue, n2: TextValue) => Some(compare(AnyValues.COMPARATOR.compare(n1, n2)))
case (n1: BooleanValue, n2: BooleanValue) => Some(compare(AnyValues.COMPARATOR.compare(n1, n2)))
case (n1: PointValue, n2: PointValue) => Some(compare(n1.compareTo(n2)))
case _ => None
}
res
Expand Down
68 changes: 45 additions & 23 deletions community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/IndexQuery.java
View file
Expand Up @@ -24,13 +24,17 @@
import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle;

import java.util.Arrays;

import org.neo4j.values.storable.PointValue;
import org.neo4j.values.storable.TextValue;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup;
import org.neo4j.values.storable.ValueTuple;
import org.neo4j.values.storable.Values;

import static org.neo4j.values.storable.Values.NO_VALUE;

public abstract class IndexQuery
{
/**
Expand Down Expand Up @@ -226,7 +230,7 @@ public IndexQueryType type()
@Override
public boolean acceptsValue( Value value )
{
return value != null && value != Values.NO_VALUE;
return value != null && value != NO_VALUE;
}

@Override
Expand Down Expand Up @@ -307,15 +311,15 @@ public boolean acceptsValue( Value value )
}
if ( Values.isNumberValue( value ) )
{
if ( from != Values.NO_VALUE )
if ( from != NO_VALUE )
{
int compare = Values.COMPARATOR.compare( value, from );
if ( compare < 0 || !fromInclusive && compare == 0 )
{
return false;
}
}
if ( to != Values.NO_VALUE )
if ( to != NO_VALUE )
{
int compare = Values.COMPARATOR.compare( value, to );
if ( compare > 0 || !toInclusive && compare == 0 )
Expand Down Expand Up @@ -367,17 +371,17 @@ public boolean toInclusive()

public static final class GeometryRangePredicate extends IndexQuery
{
private final PointValue from;
private final Value from;
private final boolean fromInclusive;
private final PointValue to;
private final Value to;
private final boolean toInclusive;

GeometryRangePredicate( int propertyKeyId, PointValue from, boolean fromInclusive, PointValue to, boolean toInclusive )
{
super( propertyKeyId );
this.from = from;
this.from = from != null ? from : NO_VALUE;
this.fromInclusive = fromInclusive;
this.to = to;
this.to = to != null ? to : NO_VALUE;
this.toInclusive = toInclusive;
}

Expand All @@ -399,23 +403,31 @@ public boolean acceptsValue( Value value )
{
PointValue point = (PointValue) value;
//TODO Use Hilbert Space Filling Curves for comparison
if ( from != Values.NO_VALUE )
PointValue lower, upper;
if ( from == NO_VALUE && to == NO_VALUE )
{
int compare = point.compareTo( from );
if ( compare < 0 || !fromInclusive && compare == 0 )
{
return false;
}
return true;
}
if ( to != Values.NO_VALUE )
if ( from != NO_VALUE && to != NO_VALUE )
{
int compare = point.compareTo( to );
if ( compare > 0 || !toInclusive && compare == 0 )
{
return false;
}
lower = from();
upper = to();
}
return true;
else if ( from != NO_VALUE )
{
lower = from();
double[] limit = new double[lower.coordinate().length];
Arrays.fill(limit, Double.POSITIVE_INFINITY);
upper = Values.pointValue(lower.getCoordinateReferenceSystem(), limit);
}
else
{
upper = to();
double[] limit = new double[upper.coordinate().length];
Arrays.fill(limit, Double.NEGATIVE_INFINITY);
lower = Values.pointValue(upper.getCoordinateReferenceSystem(), limit);
}
return point.withinRange( lower, fromInclusive, upper, toInclusive );
}
return false;
}
Expand All @@ -428,10 +440,20 @@ public ValueGroup valueGroup()

public PointValue from()
{
return from;
return (PointValue) from.asObject();
}

public PointValue to()
{
return (PointValue) to.asObject();
}

public Value fromAsValue()
{
return from;
}

public Value toAsValue()
{
return to;
}
Expand Down Expand Up @@ -494,15 +516,15 @@ public boolean acceptsValue( Value value )
{
return false;
}
if ( from != Values.NO_VALUE )
if ( from != NO_VALUE )
{
int compare = Values.COMPARATOR.compare( value, from );
if ( compare < 0 || !fromInclusive && compare == 0 )
{
return false;
}
}
if ( to != Values.NO_VALUE )
if ( to != NO_VALUE )
{
int compare = Values.COMPARATOR.compare( value, to );
if ( compare > 0 || !toInclusive && compare == 0 )
Expand Down
10 changes: 5 additions & 5 deletions ...el/src/main/java/org/neo4j/kernel/impl/index/schema/spatial/SpatialSchemaIndexReader.java
View file
Expand Up @@ -33,7 +33,7 @@
import org.neo4j.internal.kernel.api.IndexOrder;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.kernel.api.IndexQuery.ExactPredicate;
import org.neo4j.internal.kernel.api.IndexQuery.NumberRangePredicate;
import org.neo4j.internal.kernel.api.IndexQuery.GeometryRangePredicate;
import org.neo4j.io.IOUtils;
import org.neo4j.kernel.api.exceptions.index.IndexNotApplicableKernelException;
import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig;
Expand Down Expand Up @@ -139,8 +139,8 @@ public void query( IndexProgressor.NodeValueClient cursor, IndexOrder indexOrder
treeKeyTo.from( Long.MAX_VALUE, exactPredicate.value() );
startSeekForInitializedRange( cursor, treeKeyFrom, treeKeyTo );
break;
case rangeNumeric:
NumberRangePredicate rangePredicate = (NumberRangePredicate) predicate;
case rangeGeometric:
GeometryRangePredicate rangePredicate = (GeometryRangePredicate) predicate;
initFromForRange( rangePredicate, treeKeyFrom );
initToForRange( rangePredicate, treeKeyTo );
startSeekForInitializedRange( cursor, treeKeyFrom, treeKeyTo );
Expand All @@ -165,7 +165,7 @@ public static void validateQuery( IndexOrder indexOrder, IndexQuery[] predicates
}
}

private void initToForRange( NumberRangePredicate rangePredicate, KEY treeKeyTo )
private void initToForRange( GeometryRangePredicate rangePredicate, KEY treeKeyTo )
{
Value toValue = rangePredicate.toAsValue();
if ( toValue.valueGroup() == ValueGroup.NO_VALUE )
Expand All @@ -179,7 +179,7 @@ private void initToForRange( NumberRangePredicate rangePredicate, KEY treeKeyTo
}
}

private void initFromForRange( NumberRangePredicate rangePredicate, KEY treeKeyFrom )
private void initFromForRange( GeometryRangePredicate rangePredicate, KEY treeKeyFrom )
{
Value fromValue = rangePredicate.fromAsValue();
if ( fromValue.valueGroup() == ValueGroup.NO_VALUE )
Expand Down
4 changes: 2 additions & 2 deletions ...ity/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/spatial/SpatialSchemaKey.java
View file
Expand Up @@ -94,14 +94,14 @@ NumberValue asValue()

void initAsLowest()
{
writePoint( Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY ) );
writePoint( Values.pointValue( CoordinateReferenceSystem.WGS84, Double.NEGATIVE_INFINITY, Double.NEGATIVE_INFINITY ) );
entityId = Long.MIN_VALUE;
entityIdIsSpecialTieBreaker = true;
}

void initAsHighest()
{
writePoint( Values.pointValue( CoordinateReferenceSystem.Cartesian, Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY ) );
writePoint( Values.pointValue( CoordinateReferenceSystem.WGS84, Double.POSITIVE_INFINITY, Double.POSITIVE_INFINITY ) );
entityId = Long.MAX_VALUE;
entityIdIsSpecialTieBreaker = true;
}
Expand Down
61 changes: 58 additions & 3 deletions .../src/test/scala/org/neo4j/internal/cypher/acceptance/SpatialFunctionsAcceptanceTest.scala
View file
Expand Up @@ -355,7 +355,7 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
planComparisonStrategy = ComparePlansWithAssertion({ plan =>
plan should useOperatorWithText("Projection", "point")
plan should useOperatorWithText("NodeIndexSeek", ":Place(location)")
}, expectPlansToFail = Configs.AbsolutelyAll - Configs.Version3_4))
}, expectPlansToFail = Configs.AbsolutelyAll - Configs.Version3_4 - Configs.Version3_3))

// Then
val point = result.columnAs("point").toList.head.asInstanceOf[Point]
Expand All @@ -376,18 +376,73 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
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: 50.7, longitude: 12.78, crs: 'WGS-84'})")

val configuration = TestConfiguration(Versions(Versions.V3_3, Versions.V3_4, Versions.Default), Planners(Planners.Cost, Planners.Default), Runtimes(Runtimes.Interpreted, Runtimes.Slotted, Runtimes.Default))
// When
val result = executeWith(Configs.Version3_4 - Configs.Compiled - Configs.AllRulePlanners,
val result = executeWith(configuration,
"MATCH (p:Place) WHERE p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'}) RETURN p.location as point",
planComparisonStrategy = ComparePlansWithAssertion({ plan =>
plan should useOperatorWithText("Projection", "point")
plan should useOperatorWithText("NodeIndexSeek", ":Place(location)")
}, expectPlansToFail = Configs.AbsolutelyAll - Configs.Version3_4))
}, expectPlansToFail = Configs.AbsolutelyAll - configuration))

// Then
result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.WGS84, 12.78, 56.7))))
}

test("indexed point - range query") {
// Given
graph.createIndex("Place", "location")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 55.7, longitude: 11.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 50.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 10.78, crs: 'WGS-84'})")

// When
val result = innerExecuteDeprecated("CYPHER runtime=slotted MATCH (p:Place) WHERE p.location > point({latitude: 56.0, longitude: 12.0, crs: 'WGS-84'}) RETURN p.location as point", Map.empty)

// Then
val plan = result.executionPlanDescription()
plan should useOperatorWithText("Projection", "point")
plan should useOperatorWithText("NodeIndexSeekByRange", ":Place(location)")
result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.WGS84, 12.78, 56.7))))
}

test("indexed point - range query 2") {
// Given
graph.createIndex("Place", "location")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 55.7, longitude: 11.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 50.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 10.78, crs: 'WGS-84'})")

// When
val result = innerExecuteDeprecated("CYPHER runtime=slotted MATCH (p:Place) WHERE p.location >= point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'}) RETURN p.location as point", Map.empty)

// Then
val plan = result.executionPlanDescription()
plan should useOperatorWithText("Projection", "point")
plan should useOperatorWithText("NodeIndexSeekByRange", ":Place(location)")
result.toList should equal(List(Map("point" -> Values.pointValue(CoordinateReferenceSystem.WGS84, 12.78, 56.7))))
}

test("indexed point - range query 3") {
// Given
graph.createIndex("Place", "location")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 55.7, longitude: 11.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 50.7, longitude: 12.78, crs: 'WGS-84'})")
graph.execute("CREATE (p:Place) SET p.location = point({latitude: 56.7, longitude: 10.78, crs: 'WGS-84'})")

// When
val result = innerExecuteDeprecated("CYPHER runtime=slotted MATCH (p:Place) WHERE p.location > point({latitude: 56.7, longitude: 12.78, crs: 'WGS-84'}) RETURN p.location as point", Map.empty)

// Then
val plan = result.executionPlanDescription()
plan should useOperatorWithText("Projection", "point")
plan should useOperatorWithText("NodeIndexSeekByRange", ":Place(location)")
assert(result.isEmpty)
}

test("array of points should be assignable to node property") {
// Given
createLabeledNode("Place")
Expand Down

0 comments on commit 1bb666a

Please sign in to comment.