Skip to content

Commit

Permalink
Adds proper geometry range support to GenericNativeIndexReader
Browse files Browse the repository at this point in the history 
Both for single-value geometry range queries and composite.

Also IndexAccessorCompatibility, when reading results from InedxRead#query
it will filter through the query predicates. This to get geometry range queries
return correct results.
  • Loading branch information
@tinwelint
tinwelint committed Sep 5, 2018
1 parent 9819da5 commit 2d43902
Show file tree
Hide file tree
Showing 14 changed files with 257 additions and 105 deletions.
55 changes: 23 additions & 32 deletions ...rnel-it/src/test/java/org/neo4j/kernel/api/index/CompositeIndexAccessorCompatibility.java
View file
Expand Up @@ -56,6 +56,7 @@
import static org.neo4j.values.storable.ValueGroup.GEOMETRY;
import static org.neo4j.values.storable.ValueGroup.GEOMETRY_ARRAY;
import static org.neo4j.values.storable.Values.booleanArray;
import static org.neo4j.values.storable.Values.intValue;
import static org.neo4j.values.storable.Values.longArray;
import static org.neo4j.values.storable.Values.pointArray;
import static org.neo4j.values.storable.Values.pointValue;
Expand Down Expand Up @@ -264,26 +265,12 @@ public void testIndexSeekExactWithRangeByTemporalArray() throws Exception
@Test
public void testIndexSeekExactWithRangeBySpatial() throws Exception
{
testIndexSeekExactWithRange( GEOMETRY, pointValue( WGS84, 100D, 100D ), pointValue( WGS84, 0D, 0D ),
testIndexSeekExactWithRange( GEOMETRY, intValue( 100 ), intValue( 10 ),
pointValue( WGS84, -10D, -10D ),
pointValue( WGS84, -1D, -1D ),
pointValue( WGS84, 0D, 0D ),
pointValue( WGS84, 1D, 1D ),
pointValue( WGS84, 10D, 10D ),
pointValue( WGS84, 110D, 110D ),
pointValue( WGS84, 200D, 200D ),
pointValue( WGS84, 300D, 300D ) );
}

@Test
public void testIndexSeekExactWithRangeBySpatialArray() throws Exception
{
testIndexSeekExactWithRange( GEOMETRY,
pointArray( new PointValue[] {pointValue( WGS84, 100D, 100D ), pointValue( WGS84, 100D, 101D )} ),
pointArray( new PointValue[] {pointValue( WGS84, 0D, 0D ), pointValue( WGS84, 0D, 1D )} ),

pointArray( new PointValue[] {pointValue( WGS84, 1D, 1D ), pointValue( WGS84, 1D, 2D )} ),
pointArray( new PointValue[] {pointValue( WGS84, 10D, 10D ), pointValue( WGS84, 10D, 11D )} ),
pointArray( new PointValue[] {pointValue( WGS84, 100D, 100D ), pointValue( WGS84, 100D, 101D )} ),
pointArray( new PointValue[] {pointValue( WGS84, 200D, 200D ), pointValue( WGS84, 200D, 201D )} ),
pointArray( new PointValue[] {pointValue( WGS84, 300D, 300D ), pointValue( WGS84, 300D, 301D )} ) );
pointValue( WGS84, 10D, 10D ) );
}

private void testIndexSeekExactWithRange( ValueGroup valueGroup, Value base1, Value base2, Value obj1, Value obj2, Value obj3, Value obj4, Value obj5 )
Expand All @@ -309,7 +296,6 @@ private void testIndexSeekExactWithRange( ValueGroup valueGroup, Value base1, Va
assertThat( query( exact( 0, base1 ), range( 1, obj5, false, obj2, true ) ), equalTo( EMPTY_LIST ) );
assertThat( query( exact( 0, base1 ), range( 1, null, false, obj3, false ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, null, true, obj3, true ) ), equalTo( asList( 1L, 2L, 3L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, valueGroup ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, obj1, false, obj2, true ) ), equalTo( singletonList( 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, obj1, false, obj3, false ) ), equalTo( singletonList( 2L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj2, true, obj4, false ) ), equalTo( asList( 7L, 8L ) ) );
Expand All @@ -318,9 +304,14 @@ private void testIndexSeekExactWithRange( ValueGroup valueGroup, Value base1, Va
assertThat( query( exact( 0, base2 ), range( 1, obj5, false, obj2, true ) ), equalTo( EMPTY_LIST ) );
assertThat( query( exact( 0, base2 ), range( 1, null, false, obj3, false ) ), equalTo( asList( 6L, 7L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, null, true, obj3, true ) ), equalTo( asList( 6L, 7L, 8L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, valueGroup ) ), equalTo( asList( 6L, 7L, 8L, 9L, 10L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, false, obj2, true ) ), equalTo( singletonList( 7L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, false, obj3, false ) ), equalTo( singletonList( 7L ) ) );

if ( valueGroup != GEOMETRY && valueGroup != GEOMETRY_ARRAY )
{
assertThat( query( exact( 0, base1 ), range( 1, obj1.valueGroup() ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1.valueGroup() ) ), equalTo( asList( 6L, 7L, 8L, 9L, 10L ) ) );
}
}

private void testIndexSeekExactWithRangeByBooleanType( ValueGroup valueGroup, Value base1, Value base2, Value obj1, Value obj2 ) throws Exception
Expand All @@ -337,15 +328,15 @@ private void testIndexSeekExactWithRangeByBooleanType( ValueGroup valueGroup, Va
assertThat( query( exact( 0, base1 ), range( 1, obj1, false, obj2, false ) ), equalTo( EMPTY_LIST ) );
assertThat( query( exact( 0, base1 ), range( 1, null, true, obj2, true ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, obj1, true, null, true ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, valueGroup ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, obj1.valueGroup() ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( exact( 0, base1 ), range( 1, obj2, true, obj1, true ) ), equalTo( EMPTY_LIST ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, true, obj2, true ) ), equalTo( asList( 3L, 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, false, obj2, true ) ), equalTo( singletonList( 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, true, obj2, false ) ), equalTo( singletonList( 3L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, false, obj2, false ) ), equalTo( EMPTY_LIST ) );
assertThat( query( exact( 0, base2 ), range( 1, null, true, obj2, true ) ), equalTo( asList( 3L, 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1, true, null, true ) ), equalTo( asList( 3L, 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, valueGroup ) ), equalTo( asList( 3L, 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj1.valueGroup() ) ), equalTo( asList( 3L, 4L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, obj2, true, obj1, true ) ), equalTo( EMPTY_LIST ) );
}

Expand Down Expand Up @@ -576,21 +567,21 @@ public void testIndexSeekRangeWithExistsBySpatial() throws Exception
{
testIndexSeekRangeWithExists( GEOMETRY,
pointValue( Cartesian, 0D, 0D ),
pointValue( Cartesian, 100D, 100D ),
pointValue( Cartesian, 200D, 200D ),
pointValue( Cartesian, 300D, 300D ),
pointValue( Cartesian, 400D, 400D ) );
pointValue( Cartesian, 1D, 1D ),
pointValue( Cartesian, 2D, 2D ),
pointValue( Cartesian, 3D, 3D ),
pointValue( Cartesian, 4D, 4D ) );
}

@Test
public void testIndexSeekRangeWithExistsBySpatialArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.GEOMETRY_ARRAY,
pointArray( new PointValue[] {pointValue( Cartesian, 0D, 0D ), pointValue( Cartesian, 0D, 1D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 100D, 100D ), pointValue( Cartesian, 100D, 100D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 200D, 200D ), pointValue( Cartesian, 200D, 200D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 300D, 300D ), pointValue( Cartesian, 300D, 300D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 400D, 400D ), pointValue( Cartesian, 400D, 400D )} ) );
pointArray( new PointValue[] {pointValue( Cartesian, 10D, 1D ), pointValue( Cartesian, 10D, 2D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 20D, 2D ), pointValue( Cartesian, 20D, 3D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 30D, 3D ), pointValue( Cartesian, 30D, 4D )} ),
pointArray( new PointValue[] {pointValue( Cartesian, 40D, 4D ), pointValue( Cartesian, 40D, 5D )} ) );
}

private void testIndexSeekRangeWithExists( ValueGroup valueGroup, Object obj1, Object obj2, Object obj3, Object obj4, Object obj5 ) throws Exception
Expand Down Expand Up @@ -618,7 +609,7 @@ private void testIndexSeekRangeWithExists( ValueGroup valueGroup, Value obj1, Va
{
// This cannot be done for spatial values because each bound in a spatial query needs a coordinate reference system,
// and those are provided by Value instances, e.g. PointValue
assertThat( query( range( 0, valueGroup ), exists( 1 ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
assertThat( query( range( 0, obj1.valueGroup() ), exists( 1 ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
}
assertThat( query( range( 0, obj1, false, obj2, true ), exists( 1 ) ), equalTo( singletonList( 2L ) ) );
assertThat( query( range( 0, obj1, false, obj3, false ), exists( 1 ) ), equalTo( singletonList( 2L ) ) );
Expand Down
59 changes: 48 additions & 11 deletions ...ity-it/kernel-it/src/test/java/org/neo4j/kernel/api/index/IndexAccessorCompatibility.java
View file
Expand Up @@ -19,25 +19,30 @@
*/
package org.neo4j.kernel.api.index;

import org.eclipse.collections.api.iterator.LongIterator;
import org.junit.After;
import org.junit.Before;

import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;

import org.neo4j.collection.PrimitiveLongResourceIterator;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.api.index.IndexUpdateMode;
import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig;
import org.neo4j.storageengine.api.schema.IndexDescriptor;
import org.neo4j.storageengine.api.schema.IndexReader;
import org.neo4j.values.storable.Value;

public abstract class IndexAccessorCompatibility extends IndexProviderCompatibilityTestSuite.Compatibility
{
protected IndexAccessor accessor;
// This map is for spatial values, so that the #query method can lookup the values for the results and filter properly
private Map<Long,Value[]> committedValues = new HashMap<>();

public IndexAccessorCompatibility( IndexProviderCompatibilityTestSuite testSuite, IndexDescriptor descriptor )
{
Expand All @@ -63,35 +68,67 @@ public void after()

protected List<Long> query( IndexQuery... predicates ) throws Exception
{
return metaGet( reader -> reader.query( predicates ) );
}

private List<Long> metaGet( ReaderInteraction interaction ) throws Exception
{
try ( IndexReader reader = accessor.newReader() )
try ( IndexReader reader = accessor.newReader();
PrimitiveLongResourceIterator unfilteredResults = reader.query( predicates ))
{
List<Long> list = new LinkedList<>();
for ( LongIterator iterator = interaction.results( reader ); iterator.hasNext(); )
while ( unfilteredResults.hasNext() )
{
list.add( iterator.next() );
long entityId = unfilteredResults.next();
if ( passesFilter( entityId, predicates ) )
{
list.add( entityId );
}
}
Collections.sort( list );
return list;
}
}

interface ReaderInteraction
/**
* Run the Value[] from a particular entityId through the list of IndexQuery[] predicates to see if they all accept the value.
*/
private boolean passesFilter( long entityId, IndexQuery[] predicates )
{
LongIterator results( IndexReader reader ) throws Exception;
if ( predicates.length == 1 && predicates[0] instanceof IndexQuery.ExistsPredicate )
{
return true;
}

Value[] values = committedValues.get( entityId );
for ( int i = 0; i < values.length; i++ )
{
if ( !predicates[i].acceptsValue( values[i] ) )
{
return false;
}
}
return true;
}

/**
* Commit these updates to the index. Also store the values, which currently are stored for all types except geometry,
* so therefore it's done explicitly here so that we can filter on them later.
*/
void updateAndCommit( List<IndexEntryUpdate<?>> updates ) throws IndexEntryConflictException
{
try ( IndexUpdater updater = accessor.newUpdater( IndexUpdateMode.ONLINE ) )
{
for ( IndexEntryUpdate<?> update : updates )
{
updater.process( update );
switch ( update.updateMode() )
{
case ADDED:
case CHANGED:
committedValues.put( update.getEntityId(), update.values() );
break;
case REMOVED:
committedValues.remove( update.getEntityId() );
break;
default:
throw new IllegalArgumentException( "Unknown update mode of " + update );
}
}
}
}
Expand Down
15 changes: 15 additions & 0 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/CompositeGenericKey.java
View file
Expand Up @@ -22,9 +22,13 @@
import java.util.Arrays;
import java.util.StringJoiner;

import org.neo4j.gis.spatial.index.curves.SpaceFillingCurve;
import org.neo4j.gis.spatial.index.curves.SpaceFillingCurveConfiguration;
import org.neo4j.io.pagecache.PageCursor;
import org.neo4j.kernel.impl.index.schema.config.IndexSpecificSpaceFillingCurveSettingsCache;
import org.neo4j.util.Preconditions;
import org.neo4j.values.storable.CoordinateReferenceSystem;
import org.neo4j.values.storable.PointValue;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup;

Expand All @@ -43,6 +47,17 @@ class CompositeGenericKey extends NativeIndexKey<CompositeGenericKey>
}
}

/**
* Special init method for when doing sub-queries for geometry range. This given slot will not be initialized with a {@link PointValue},
* but a 1D value which is derived from that point, calculated based on intersecting tiles.
*
* @see SpaceFillingCurve#getTilesIntersectingEnvelope(double[], double[], SpaceFillingCurveConfiguration)
*/
void initFromDerivedSpatialValue( int stateSlot, CoordinateReferenceSystem crs, long derivedValue, Inclusion inclusion )
{
states[stateSlot].writePointDerived( crs, derivedValue, inclusion );
}

@Override
void writeValue( int stateSlot, Value value, Inclusion inclusion )
{
Expand Down
12 changes: 12 additions & 0 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericKeyState.java
View file
Expand Up @@ -1783,6 +1783,18 @@ else if ( this.crs != crs )
}
}

void writePointDerived( CoordinateReferenceSystem crs, long derivedValue, NativeIndexKey.Inclusion inclusion )
{
if ( isArray )
{
throw new IllegalStateException( "Not sure we're doing arrays like this, are we?" );
}
type = Type.GEOMETRY;
this.inclusion = inclusion;
long0 = derivedValue;
updateCurve( crs );
}

private void updateCurve( CoordinateReferenceSystem crs )
{
if ( this.crs == null || this.crs != crs )
Expand Down
10 changes: 8 additions & 2 deletions ...y/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexAccessor.java
View file
Expand Up @@ -22,6 +22,7 @@
import java.io.File;
import java.io.IOException;

import org.neo4j.gis.spatial.index.curves.SpaceFillingCurveConfiguration;
import org.neo4j.index.internal.gbptree.GBPTree;
import org.neo4j.index.internal.gbptree.RecoveryCleanupWorkCollector;
import org.neo4j.io.fs.FileSystemAbstraction;
Expand All @@ -38,14 +39,19 @@

class GenericNativeIndexAccessor extends NativeIndexAccessor<CompositeGenericKey,NativeIndexValue>
{
private final IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings;
private final SpaceFillingCurveConfiguration configuration;
private Validator<Value[]> validator;

GenericNativeIndexAccessor( PageCache pageCache, FileSystemAbstraction fs, File storeFile, IndexLayout<CompositeGenericKey,NativeIndexValue> layout,
RecoveryCleanupWorkCollector recoveryCleanupWorkCollector, IndexProvider.Monitor monitor, StoreIndexDescriptor descriptor,
IndexSamplingConfig samplingConfig, IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings ) throws IOException
IndexSamplingConfig samplingConfig, IndexSpecificSpaceFillingCurveSettingsCache spaceFillingCurveSettings,
SpaceFillingCurveConfiguration configuration ) throws IOException
{
super( pageCache, fs, storeFile, layout, recoveryCleanupWorkCollector, monitor, descriptor, samplingConfig,
new SpaceFillingCurveSettingsWriter( spaceFillingCurveSettings ) );
this.spaceFillingCurveSettings = spaceFillingCurveSettings;
this.configuration = configuration;
}

@Override
Expand All @@ -57,7 +63,7 @@ protected void afterTreeInstantiation( GBPTree<CompositeGenericKey,NativeIndexVa
@Override
public IndexReader newReader()
{
return new GenericNativeIndexReader( tree, layout, samplingConfig, descriptor );
return new GenericNativeIndexReader( tree, layout, samplingConfig, descriptor, spaceFillingCurveSettings, configuration );
}

@Override
Expand Down
11 changes: 9 additions & 2 deletions .../kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexPopulator.java
View file
Expand Up @@ -21,6 +21,8 @@

import java.io.File;

import org.neo4j.gis.spatial.index.curves.SpaceFillingCurveConfiguration;
import org.neo4j.index.internal.gbptree.Layout;
import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.io.pagecache.PageCache;
import org.neo4j.kernel.api.index.IndexProvider;
Expand All @@ -32,16 +34,21 @@

class GenericNativeIndexPopulator extends NativeIndexPopulator<CompositeGenericKey,NativeIndexValue>
{
private final IndexSpecificSpaceFillingCurveSettingsCache spatialSettings;
private final SpaceFillingCurveConfiguration configuration;

GenericNativeIndexPopulator( PageCache pageCache, FileSystemAbstraction fs, File storeFile, IndexLayout<CompositeGenericKey,NativeIndexValue> layout,
IndexProvider.Monitor monitor, StoreIndexDescriptor descriptor, IndexSamplingConfig samplingConfig,
IndexSpecificSpaceFillingCurveSettingsCache spatialSettings )
IndexSpecificSpaceFillingCurveSettingsCache spatialSettings, SpaceFillingCurveConfiguration configuration )
{
super( pageCache, fs, storeFile, layout, monitor, descriptor, samplingConfig, new SpaceFillingCurveSettingsWriter( spatialSettings ) );
this.spatialSettings = spatialSettings;
this.configuration = configuration;
}

@Override
IndexReader newReader()
{
return new GenericNativeIndexReader( tree, layout, samplingConfig, descriptor );
return new GenericNativeIndexReader( tree, layout, samplingConfig, descriptor, spatialSettings, configuration );
}
}
5 changes: 3 additions & 2 deletions ...y/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/GenericNativeIndexProvider.java
View file
Expand Up @@ -46,6 +46,7 @@
import org.neo4j.kernel.impl.index.schema.config.SpaceFillingCurveSettings;
import org.neo4j.storageengine.api.schema.StoreIndexDescriptor;
import org.neo4j.kernel.impl.index.schema.config.SpaceFillingCurveSettingsReader;
import org.neo4j.storageengine.api.schema.StoreIndexDescriptor;
import org.neo4j.values.storable.CoordinateReferenceSystem;
import org.neo4j.values.storable.ValueCategory;

Expand Down Expand Up @@ -173,7 +174,7 @@ protected IndexPopulator newIndexPopulator( File storeFile, IndexLayout<Composit
{
// TODO this layout cast isn't nice, try and get rid of it
return new GenericNativeIndexPopulator( pageCache, fs, storeFile, layout, monitor, descriptor, samplingConfig,
((GenericLayout) layout).getSpaceFillingCurveSettings() );
((GenericLayout) layout).getSpaceFillingCurveSettings(), configuration );
}

@Override
Expand All @@ -182,7 +183,7 @@ protected IndexAccessor newIndexAccessor( File storeFile, IndexLayout<CompositeG
{
// TODO this layout cast isn't nice, try and get rid of it
return new GenericNativeIndexAccessor( pageCache, fs, storeFile, layout, recoveryCleanupWorkCollector, monitor, descriptor, samplingConfig,
((GenericLayout) layout).getSpaceFillingCurveSettings() );
((GenericLayout) layout).getSpaceFillingCurveSettings(), configuration );
}

@Override
Expand Down

0 comments on commit 2d43902

Please sign in to comment.