Skip to content

Commit

Permalink
Cleaned up geometry range queries to use correct CRS
Browse files Browse the repository at this point in the history
  • Loading branch information
craigtaverner authored and OliviaYtterbrink committed Feb 14, 2018
1 parent 9c7bbc0 commit 99bc3ac
Show file tree
Hide file tree
Showing 10 changed files with 131 additions and 44 deletions.
Expand Up @@ -24,15 +24,14 @@
import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.commons.lang3.builder.ToStringStyle; import org.apache.commons.lang3.builder.ToStringStyle;


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


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

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


@Override @Override
Expand Down Expand Up @@ -309,15 +308,15 @@ public boolean acceptsValue( Value value )
} }
if ( Values.isNumberValue( value ) ) if ( Values.isNumberValue( value ) )
{ {
if ( from != NO_VALUE ) if ( from != Values.NO_VALUE )
{ {
int compare = Values.COMPARATOR.compare( value, from ); int compare = Values.COMPARATOR.compare( value, from );
if ( compare < 0 || !fromInclusive && compare == 0 ) if ( compare < 0 || !fromInclusive && compare == 0 )
{ {
return false; return false;
} }
} }
if ( to != NO_VALUE ) if ( to != Values.NO_VALUE )
{ {
int compare = Values.COMPARATOR.compare( value, to ); int compare = Values.COMPARATOR.compare( value, to );
if ( compare > 0 || !toInclusive && compare == 0 ) if ( compare > 0 || !toInclusive && compare == 0 )
Expand Down Expand Up @@ -369,18 +368,24 @@ public boolean toInclusive()


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


GeometryRangePredicate( int propertyKeyId, PointValue from, boolean fromInclusive, PointValue to, boolean toInclusive ) GeometryRangePredicate( int propertyKeyId, PointValue from, boolean fromInclusive, PointValue to, boolean toInclusive )
{ {
super( propertyKeyId ); super( propertyKeyId );
this.from = from != null ? from : NO_VALUE; if ( from == null && to == null )
{
throw new IllegalArgumentException( "Cannot create GeometryRangePredicate without at least one bound" );
}
this.from = from;
this.fromInclusive = fromInclusive; this.fromInclusive = fromInclusive;
this.to = to != null ? to : NO_VALUE; this.to = to;
this.toInclusive = toInclusive; this.toInclusive = toInclusive;
this.crs = from != null ? from.getCoordinateReferenceSystem() : to.getCoordinateReferenceSystem();
} }


@Override @Override
Expand All @@ -400,8 +405,7 @@ public boolean acceptsValue( Value value )
if ( value instanceof PointValue ) if ( value instanceof PointValue )
{ {
PointValue point = (PointValue) value; PointValue point = (PointValue) value;
//TODO Use Hilbert Space Filling Curves for comparison return point.withinRange( from, fromInclusive, to, toInclusive );
return point.withinRange( from(), fromInclusive, to(), toInclusive );
} }
return false; return false;
} }
Expand All @@ -414,22 +418,17 @@ public ValueGroup valueGroup()


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


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

public Value fromAsValue()
{
return from;
} }


public Value toAsValue() public CoordinateReferenceSystem crs()
{ {
return to; return crs;
} }


public boolean fromInclusive() public boolean fromInclusive()
Expand Down Expand Up @@ -490,15 +489,15 @@ public boolean acceptsValue( Value value )
{ {
return false; return false;
} }
if ( from != NO_VALUE ) if ( from != Values.NO_VALUE )
{ {
int compare = Values.COMPARATOR.compare( value, from ); int compare = Values.COMPARATOR.compare( value, from );
if ( compare < 0 || !fromInclusive && compare == 0 ) if ( compare < 0 || !fromInclusive && compare == 0 )
{ {
return false; return false;
} }
} }
if ( to != NO_VALUE ) if ( to != Values.NO_VALUE )
{ {
int compare = Values.COMPARATOR.compare( value, to ); int compare = Values.COMPARATOR.compare( value, to );
if ( compare > 0 || !toInclusive && compare == 0 ) if ( compare > 0 || !toInclusive && compare == 0 )
Expand Down
Expand Up @@ -125,10 +125,10 @@ private NativeSchemaIndexPopulator makeIndexPopulator( IndexDescriptor descripto
switch ( descriptor.type() ) switch ( descriptor.type() )
{ {
case GENERAL: case GENERAL:
return new NativeNonUniqueSchemaIndexPopulator<>( pageCache, fs, indexFile, new SpatialLayoutNonUnique(), samplingConfig, monitor, descriptor, return new NativeNonUniqueSchemaIndexPopulator<>( pageCache, fs, indexFile, new SpatialLayoutNonUnique(crs), samplingConfig, monitor, descriptor,
indexId ); indexId );
case UNIQUE: case UNIQUE:
return new NativeUniqueSchemaIndexPopulator<>( pageCache, fs, indexFile, new SpatialLayoutUnique(), monitor, descriptor, indexId ); return new NativeUniqueSchemaIndexPopulator<>( pageCache, fs, indexFile, new SpatialLayoutUnique(crs), monitor, descriptor, indexId );
default: default:
throw new UnsupportedOperationException( "Can not create index populator of type " + descriptor.type() ); throw new UnsupportedOperationException( "Can not create index populator of type " + descriptor.type() );
} }
Expand Down Expand Up @@ -165,10 +165,10 @@ private SpatialSchemaIndexAccessor makeOnlineAccessor( IndexDescriptor descripto
switch ( descriptor.type() ) switch ( descriptor.type() )
{ {
case GENERAL: case GENERAL:
layout = new SpatialLayoutNonUnique(); layout = new SpatialLayoutNonUnique(crs);
break; break;
case UNIQUE: case UNIQUE:
layout = new SpatialLayoutUnique(); layout = new SpatialLayoutUnique(crs);
break; break;
default: default:
throw new UnsupportedOperationException( "Can not create index accessor of type " + descriptor.type() ); throw new UnsupportedOperationException( "Can not create index accessor of type " + descriptor.type() );
Expand Down
Expand Up @@ -21,16 +21,24 @@


import org.neo4j.index.internal.gbptree.Layout; import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.io.pagecache.PageCursor; import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.values.storable.CoordinateReferenceSystem;


/** /**
* {@link Layout} for numbers where numbers doesn't need to be unique. * {@link Layout} for numbers where numbers doesn't need to be unique.
*/ */
public abstract class SpatialLayout extends Layout.Adapter<SpatialSchemaKey,NativeSchemaValue> public abstract class SpatialLayout extends Layout.Adapter<SpatialSchemaKey,NativeSchemaValue>
{ {
private CoordinateReferenceSystem crs;

public SpatialLayout( CoordinateReferenceSystem crs )
{
this.crs = crs;
}

@Override @Override
public SpatialSchemaKey newKey() public SpatialSchemaKey newKey()
{ {
return new SpatialSchemaKey(); return new SpatialSchemaKey(crs);
} }


@Override @Override
Expand All @@ -41,6 +49,7 @@ public SpatialSchemaKey copyKey( SpatialSchemaKey key,
into.rawValueBits = key.rawValueBits; into.rawValueBits = key.rawValueBits;
into.setEntityId( key.getEntityId() ); into.setEntityId( key.getEntityId() );
into.setEntityIdIsSpecialTieBreaker( key.getEntityIdIsSpecialTieBreaker() ); into.setEntityIdIsSpecialTieBreaker( key.getEntityIdIsSpecialTieBreaker() );
into.crs = key.crs;
return into; return into;
} }


Expand Down
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.kernel.impl.index.schema; package org.neo4j.kernel.impl.index.schema;


import org.neo4j.index.internal.gbptree.Layout; import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.values.storable.CoordinateReferenceSystem;


public class SpatialLayoutNonUnique extends SpatialLayout public class SpatialLayoutNonUnique extends SpatialLayout
{ {
Expand All @@ -28,6 +29,11 @@ public class SpatialLayoutNonUnique extends SpatialLayout
public static final int MINOR_VERSION = 1; public static final int MINOR_VERSION = 1;
public static long IDENTIFIER = Layout.namedIdentifier( IDENTIFIER_NAME, NativeSchemaValue.SIZE ); public static long IDENTIFIER = Layout.namedIdentifier( IDENTIFIER_NAME, NativeSchemaValue.SIZE );


public SpatialLayoutNonUnique( CoordinateReferenceSystem crs )
{
super(crs);
}

@Override @Override
public long identifier() public long identifier()
{ {
Expand Down
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.kernel.impl.index.schema; package org.neo4j.kernel.impl.index.schema;


import org.neo4j.index.internal.gbptree.Layout; import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.values.storable.CoordinateReferenceSystem;


/** /**
* {@link Layout} for numbers where numbers need to be unique. * {@link Layout} for numbers where numbers need to be unique.
Expand All @@ -31,6 +32,11 @@ public class SpatialLayoutUnique extends SpatialLayout
public static final int MINOR_VERSION = 1; public static final int MINOR_VERSION = 1;
public static long IDENTIFIER = Layout.namedIdentifier( IDENTIFIER_NAME, NativeSchemaValue.SIZE ); public static long IDENTIFIER = Layout.namedIdentifier( IDENTIFIER_NAME, NativeSchemaValue.SIZE );


public SpatialLayoutUnique( CoordinateReferenceSystem crs )
{
super(crs);
}

@Override @Override
public long identifier() public long identifier()
{ {
Expand Down
Expand Up @@ -31,12 +31,11 @@
import org.neo4j.kernel.api.schema.index.IndexDescriptor; import org.neo4j.kernel.api.schema.index.IndexDescriptor;
import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig;
import org.neo4j.storageengine.api.schema.IndexProgressor; import org.neo4j.storageengine.api.schema.IndexProgressor;
import org.neo4j.values.storable.Value; import org.neo4j.values.storable.PointValue;
import org.neo4j.values.storable.ValueGroup;


import static java.lang.String.format; import static java.lang.String.format;


public class SpatialSchemaIndexReader<KEY extends NativeSchemaKey, VALUE extends NativeSchemaValue> extends NativeSchemaIndexReader<KEY, VALUE> public class SpatialSchemaIndexReader<KEY extends SpatialSchemaKey, VALUE extends NativeSchemaValue> extends NativeSchemaIndexReader<KEY, VALUE>
{ {
SpatialSchemaIndexReader( GBPTree<KEY,VALUE> tree, Layout<KEY,VALUE> layout, IndexSamplingConfig samplingConfig, IndexDescriptor descriptor ) SpatialSchemaIndexReader( GBPTree<KEY,VALUE> tree, Layout<KEY,VALUE> layout, IndexSamplingConfig samplingConfig, IndexDescriptor descriptor )
{ {
Expand Down Expand Up @@ -76,6 +75,11 @@ void initializeRangeForQuery( KEY treeKeyFrom, KEY treeKeyTo, IndexQuery[] predi
break; break;
case rangeGeometric: case rangeGeometric:
GeometryRangePredicate rangePredicate = (GeometryRangePredicate) predicate; GeometryRangePredicate rangePredicate = (GeometryRangePredicate) predicate;
if ( !rangePredicate.crs().equals( treeKeyTo.crs ) )
{
throw new IllegalArgumentException(
"IndexQuery on spatial index with mismatching CoordinateReferenceSystem: " + rangePredicate.crs() + " != " + treeKeyTo.crs );
}
initFromForRange( rangePredicate, treeKeyFrom ); initFromForRange( rangePredicate, treeKeyFrom );
initToForRange( rangePredicate, treeKeyTo ); initToForRange( rangePredicate, treeKeyTo );
break; break;
Expand All @@ -86,8 +90,8 @@ void initializeRangeForQuery( KEY treeKeyFrom, KEY treeKeyTo, IndexQuery[] predi


private void initToForRange( GeometryRangePredicate rangePredicate, KEY treeKeyTo ) private void initToForRange( GeometryRangePredicate rangePredicate, KEY treeKeyTo )
{ {
Value toValue = rangePredicate.toAsValue(); PointValue toValue = rangePredicate.to();
if ( toValue.valueGroup() == ValueGroup.NO_VALUE ) if ( toValue == null )
{ {
treeKeyTo.initAsHighest(); treeKeyTo.initAsHighest();
} }
Expand All @@ -100,8 +104,8 @@ private void initToForRange( GeometryRangePredicate rangePredicate, KEY treeKeyT


private void initFromForRange( GeometryRangePredicate rangePredicate, KEY treeKeyFrom ) private void initFromForRange( GeometryRangePredicate rangePredicate, KEY treeKeyFrom )
{ {
Value fromValue = rangePredicate.fromAsValue(); PointValue fromValue = rangePredicate.from();
if ( fromValue.valueGroup() == ValueGroup.NO_VALUE ) if ( fromValue == null )
{ {
treeKeyFrom.initAsLowest(); treeKeyFrom.initAsLowest();
} }
Expand Down
Expand Up @@ -48,6 +48,12 @@ class SpatialSchemaKey implements NativeSchemaKey


byte type; byte type;
long rawValueBits; long rawValueBits;
CoordinateReferenceSystem crs;

public SpatialSchemaKey( CoordinateReferenceSystem crs )
{
this.crs = crs;
}


@Override @Override
public void setEntityIdIsSpecialTieBreaker( boolean entityIdIsSpecialTieBreaker ) public void setEntityIdIsSpecialTieBreaker( boolean entityIdIsSpecialTieBreaker )
Expand Down Expand Up @@ -97,19 +103,21 @@ public NumberValue asValue()
@Override @Override
public void initAsLowest() public void initAsLowest()
{ {
//TODO: Get dimension
double[] limit = new double[2]; double[] limit = new double[2];
Arrays.fill(limit, Double.NEGATIVE_INFINITY); Arrays.fill(limit, Double.NEGATIVE_INFINITY);
writePoint( CoordinateReferenceSystem.WGS84, limit ); writePoint( crs, limit );
entityId = Long.MIN_VALUE; entityId = Long.MIN_VALUE;
entityIdIsSpecialTieBreaker = true; entityIdIsSpecialTieBreaker = true;
} }


@Override @Override
public void initAsHighest() public void initAsHighest()
{ {
//TODO: Get dimension
double[] limit = new double[2]; double[] limit = new double[2];
Arrays.fill(limit, Double.POSITIVE_INFINITY); Arrays.fill(limit, Double.POSITIVE_INFINITY);
writePoint( CoordinateReferenceSystem.WGS84, limit ); writePoint( crs, limit );
entityId = Long.MAX_VALUE; entityId = Long.MAX_VALUE;
entityIdIsSpecialTieBreaker = true; entityIdIsSpecialTieBreaker = true;
} }
Expand Down
Expand Up @@ -108,7 +108,7 @@ private IndexReader selectIf( IndexQuery... predicates ) throws IndexNotApplicab
} }
else if ( predicates[0] instanceof GeometryRangePredicate ) else if ( predicates[0] instanceof GeometryRangePredicate )
{ {
return readerMap.get( ((GeometryRangePredicate) predicates[0]).from().getCoordinateReferenceSystem() ); return readerMap.get( ((GeometryRangePredicate) predicates[0]).crs() );
} }
return null; return null;
} }
Expand Down
Expand Up @@ -443,6 +443,44 @@ class SpatialFunctionsAcceptanceTest extends ExecutionEngineFunSuite with Cypher
assert(result.isEmpty) assert(result.isEmpty)
} }


test("indexed point - range query 4 - multiple CRS") {
// Given
graph.createIndex("Place", "location")
graph.execute("CREATE (p:Place) SET p.location = point({y: 56.7, x: 12.78, crs: 'cartesian'})")
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 5 - multiple inequalities") {
// Given
graph.createIndex("Place", "location")
graph.execute("CREATE (p:Place) SET p.location = point({y: 55.7, x: 11.78, crs: 'cartesian'})")
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: 55.7, longitude: 11.78, crs: 'WGS-84'}) AND 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, 11.78, 55.7))))
}

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

0 comments on commit 99bc3ac

Please sign in to comment.