Skip to content

Commit

Permalink
Generic native index updates from PR comments
Browse files Browse the repository at this point in the history 
Most of it fixing inconsistencies in GenericKeyState
  • Loading branch information
@tinwelint
tinwelint committed Sep 5, 2018
1 parent cfdc664 commit 3e397e1
Show file tree
Hide file tree
Showing 12 changed files with 173 additions and 134 deletions.
69 changes: 46 additions & 23 deletions ...rnel-it/src/test/java/org/neo4j/kernel/api/index/CompositeIndexAccessorCompatibility.java
View file
Expand Up @@ -35,6 +35,7 @@
import org.neo4j.values.storable.BooleanValue;
import org.neo4j.values.storable.CoordinateReferenceSystem;
import org.neo4j.values.storable.DateTimeValue;
import org.neo4j.values.storable.PointArray;
import org.neo4j.values.storable.PointValue;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup;
Expand Down Expand Up @@ -175,7 +176,7 @@ public void testIndexScanAndSeekExactWithExactByPoint() throws Exception
@Test
public void testIndexSeekExactWithRangeByString() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.TEXT, Values.of( "a" ), Values.of( "b" ),
testIndexSeekExactWithRange( Values.of( "a" ), Values.of( "b" ),
Values.of( "Anabelle" ),
Values.of( "Anna" ),
Values.of( "Bob" ),
Expand All @@ -186,7 +187,7 @@ public void testIndexSeekExactWithRangeByString() throws Exception
@Test
public void testIndexSeekExactWithRangeByNumber() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.NUMBER, Values.of( 303 ), Values.of( 101 ),
testIndexSeekExactWithRange( Values.of( 303 ), Values.of( 101 ),
Values.of( 111 ),
Values.of( 222 ),
Values.of( 333 ),
Expand All @@ -197,7 +198,7 @@ public void testIndexSeekExactWithRangeByNumber() throws Exception
@Test
public void testIndexSeekExactWithRangeByTemporal() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.DATE, epochDate( 303 ), epochDate( 101 ),
testIndexSeekExactWithRange( epochDate( 303 ), epochDate( 101 ),
epochDate( 111 ),
epochDate( 222 ),
epochDate( 333 ),
Expand All @@ -218,7 +219,7 @@ public void testIndexSeekExactWithRangeByBoolean() throws Exception
@Test
public void testIndexSeekExactWithRangeByStringArray() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.TEXT_ARRAY, stringArray( "a", "c" ), stringArray( "b", "c" ),
testIndexSeekExactWithRange( stringArray( "a", "c" ), stringArray( "b", "c" ),
stringArray( "Anabelle", "c" ),
stringArray( "Anna", "c" ),
stringArray( "Bob", "c" ),
Expand All @@ -230,7 +231,7 @@ public void testIndexSeekExactWithRangeByStringArray() throws Exception
@Test
public void testIndexSeekExactWithRangeByNumberArray() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.NUMBER_ARRAY, longArray( new long[]{333, 9000} ), longArray( new long[]{101, 900} ),
testIndexSeekExactWithRange( longArray( new long[]{333, 9000} ), longArray( new long[]{101, 900} ),
longArray( new long[]{111, 900} ),
longArray( new long[]{222, 900} ),
longArray( new long[]{333, 900} ),
Expand All @@ -242,7 +243,7 @@ public void testIndexSeekExactWithRangeByNumberArray() throws Exception
@Test
public void testIndexSeekExactWithRangeByBooleanArray() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.BOOLEAN_ARRAY, booleanArray( new boolean[]{true, true} ), booleanArray( new boolean[]{false, false} ),
testIndexSeekExactWithRange( booleanArray( new boolean[]{true, true} ), booleanArray( new boolean[]{false, false} ),
booleanArray( new boolean[]{false, false} ),
booleanArray( new boolean[]{false, true} ),
booleanArray( new boolean[]{true, false} ),
Expand All @@ -254,7 +255,7 @@ public void testIndexSeekExactWithRangeByBooleanArray() throws Exception
@Test
public void testIndexSeekExactWithRangeByTemporalArray() throws Exception
{
testIndexSeekExactWithRange( ValueGroup.DATE_ARRAY, dateArray( 303, 900 ), dateArray( 101, 900 ),
testIndexSeekExactWithRange( dateArray( 303, 900 ), dateArray( 101, 900 ),
dateArray( 111, 900 ),
dateArray( 222, 900 ),
dateArray( 333, 900 ),
Expand All @@ -265,15 +266,15 @@ public void testIndexSeekExactWithRangeByTemporalArray() throws Exception
@Test
public void testIndexSeekExactWithRangeBySpatial() throws Exception
{
testIndexSeekExactWithRange( GEOMETRY, intValue( 100 ), intValue( 10 ),
testIndexSeekExactWithRange( intValue( 100 ), intValue( 10 ),
pointValue( WGS84, -10D, -10D ),
pointValue( WGS84, -1D, -1D ),
pointValue( WGS84, 0D, 0D ),
pointValue( WGS84, 1D, 1D ),
pointValue( WGS84, 10D, 10D ) );
}

private void testIndexSeekExactWithRange( ValueGroup valueGroup, Value base1, Value base2, Value obj1, Value obj2, Value obj3, Value obj4, Value obj5 )
private void testIndexSeekExactWithRange( Value base1, Value base2, Value obj1, Value obj2, Value obj3, Value obj4, Value obj5 )
throws Exception
{
Assume.assumeTrue( "Assume support for granular composite queries", testSuite.supportsGranularCompositeQueries() );
Expand Down Expand Up @@ -307,11 +308,32 @@ private void testIndexSeekExactWithRange( ValueGroup valueGroup, Value base1, Va
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 ) ) );

ValueGroup valueGroup = obj1.valueGroup();
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 ) ) );
assertThat( query( exact( 0, base1 ), range( 1, valueGroup ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, valueGroup ) ), equalTo( asList( 6L, 7L, 8L, 9L, 10L ) ) );
}
else
{
CoordinateReferenceSystem crs = getCrs( obj1 );
assertThat( query( exact( 0, base1 ), range( 1, crs ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L ) ) );
assertThat( query( exact( 0, base2 ), range( 1, crs ) ), equalTo( asList( 6L, 7L, 8L, 9L, 10L ) ) );
}
}

private CoordinateReferenceSystem getCrs( Value value )
{
if ( Values.isGeometryValue( value ) )
{
return ((PointValue) value).getCoordinateReferenceSystem();
}
else if ( Values.isGeometryArray( value ) )
{
PointArray array = (PointArray) value;
return array.pointValue( 0 ).getCoordinateReferenceSystem();
}
throw new IllegalArgumentException( "Expected some geometry value to get CRS from, but got " + value );
}

private void testIndexSeekExactWithRangeByBooleanType( ValueGroup valueGroup, Value base1, Value base2, Value obj1, Value obj2 ) throws Exception
Expand Down Expand Up @@ -479,13 +501,13 @@ private void testIndexSeekExactWithExists( Value a, Value b ) throws Exception
@Test
public void testIndexSeekRangeWithExistsByString() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.TEXT, "Anabelle", "Anna", "Bob", "Harriet", "William" );
testIndexSeekRangeWithExists( "Anabelle", "Anna", "Bob", "Harriet", "William" );
}

@Test
public void testIndexSeekRangeWithExistsByNumber() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.NUMBER, -5, 0, 5.5, 10.0, 100.0 );
testIndexSeekRangeWithExists( -5, 0, 5.5, 10.0, 100.0 );
}

@Test
Expand All @@ -496,7 +518,7 @@ public void testIndexSeekRangeWithExistsByTemporal() throws Exception
DateTimeValue d3 = datetime( 10000, 100, ZoneId.of( "+01:00" ) );
DateTimeValue d4 = datetime( 10000, 100, ZoneId.of( "Europe/Stockholm" ) );
DateTimeValue d5 = datetime( 10000, 100, ZoneId.of( "+03:00" ) );
testIndexSeekRangeWithExists( ValueGroup.DATE, d1, d2, d3, d4, d5 );
testIndexSeekRangeWithExists( d1, d2, d3, d4, d5 );
}

@Test
Expand All @@ -521,7 +543,7 @@ public void testIndexSeekRangeWithExistsByBoolean() throws Exception
@Test
public void testIndexSeekRangeWithExistsByStringArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.TEXT_ARRAY,
testIndexSeekRangeWithExists(
new String[]{"Anabelle", "Anabelle"},
new String[]{"Anabelle", "Anablo"},
new String[]{"Anna", "Anabelle"},
Expand All @@ -532,7 +554,7 @@ public void testIndexSeekRangeWithExistsByStringArray() throws Exception
@Test
public void testIndexSeekRangeWithExistsByNumberArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.NUMBER_ARRAY,
testIndexSeekRangeWithExists(
new long[]{303, 303},
new long[]{303, 404},
new long[]{600, 303},
Expand All @@ -543,7 +565,7 @@ public void testIndexSeekRangeWithExistsByNumberArray() throws Exception
@Test
public void testIndexSeekRangeWithExistsByBooleanArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.NUMBER_ARRAY,
testIndexSeekRangeWithExists(
new boolean[]{false, false},
new boolean[]{false, true},
new boolean[]{true, false},
Expand All @@ -554,7 +576,7 @@ public void testIndexSeekRangeWithExistsByBooleanArray() throws Exception
@Test
public void testIndexSeekRangeWithExistsByTemporalArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.NUMBER_ARRAY,
testIndexSeekRangeWithExists(
dateArray( 303, 303 ),
dateArray( 303, 404 ),
dateArray( 404, 303 ),
Expand All @@ -565,7 +587,7 @@ public void testIndexSeekRangeWithExistsByTemporalArray() throws Exception
@Test
public void testIndexSeekRangeWithExistsBySpatial() throws Exception
{
testIndexSeekRangeWithExists( GEOMETRY,
testIndexSeekRangeWithExists(
pointValue( Cartesian, 0D, 0D ),
pointValue( Cartesian, 1D, 1D ),
pointValue( Cartesian, 2D, 2D ),
Expand All @@ -576,20 +598,20 @@ public void testIndexSeekRangeWithExistsBySpatial() throws Exception
@Test
public void testIndexSeekRangeWithExistsBySpatialArray() throws Exception
{
testIndexSeekRangeWithExists( ValueGroup.GEOMETRY_ARRAY,
testIndexSeekRangeWithExists(
pointArray( new PointValue[] {pointValue( Cartesian, 0D, 0D ), pointValue( Cartesian, 0D, 1D )} ),
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
private void testIndexSeekRangeWithExists( Object obj1, Object obj2, Object obj3, Object obj4, Object obj5 ) throws Exception
{
testIndexSeekRangeWithExists( valueGroup, Values.of( obj1 ), Values.of( obj2 ), Values.of( obj3 ), Values.of( obj4 ), Values.of( obj5 ) );
testIndexSeekRangeWithExists( Values.of( obj1 ), Values.of( obj2 ), Values.of( obj3 ), Values.of( obj4 ), Values.of( obj5 ) );
}

private void testIndexSeekRangeWithExists( ValueGroup valueGroup, Value obj1, Value obj2, Value obj3, Value obj4, Value obj5 ) throws Exception
private void testIndexSeekRangeWithExists( Value obj1, Value obj2, Value obj3, Value obj4, Value obj5 ) throws Exception
{
Assume.assumeTrue( "Assume support for granular composite queries", testSuite.supportsGranularCompositeQueries() );
updateAndCommit( asList(
Expand All @@ -605,6 +627,7 @@ private void testIndexSeekRangeWithExists( ValueGroup valueGroup, Value obj1, Va
assertThat( query( range( 0, obj5, false, obj2, true ), exists( 1 ) ), equalTo( EMPTY_LIST ) );
assertThat( query( range( 0, null, false, obj3, false ), exists( 1 ) ), equalTo( asList( 1L, 2L ) ) );
assertThat( query( range( 0, null, true, obj3, true ), exists( 1 ) ), equalTo( asList( 1L, 2L, 3L ) ) );
ValueGroup valueGroup = obj1.valueGroup();
if ( valueGroup != GEOMETRY && valueGroup != GEOMETRY_ARRAY )
{
// This cannot be done for spatial values because each bound in a spatial query needs a coordinate reference system,
Expand Down
11 changes: 9 additions & 2 deletions ...ity-it/kernel-it/src/test/java/org/neo4j/kernel/api/index/IndexAccessorCompatibility.java
View file
Expand Up @@ -37,6 +37,7 @@
import org.neo4j.storageengine.api.schema.IndexDescriptor;
import org.neo4j.storageengine.api.schema.IndexReader;
import org.neo4j.values.storable.Value;
import org.neo4j.values.storable.ValueGroup;

public abstract class IndexAccessorCompatibility extends IndexProviderCompatibilityTestSuite.Compatibility
{
Expand Down Expand Up @@ -98,10 +99,16 @@ private boolean passesFilter( long entityId, IndexQuery[] predicates )
Value[] values = committedValues.get( entityId );
for ( int i = 0; i < values.length; i++ )
{
if ( !predicates[i].acceptsValue( values[i] ) )
IndexQuery predicate = predicates[i];
if ( predicate.valueGroup() == ValueGroup.GEOMETRY || predicate.valueGroup() == ValueGroup.GEOMETRY_ARRAY )
{
return false;
if ( !predicates[i].acceptsValue( values[i] ) )
{
return false;
}
}
// else there's no functional need to let values, other than those of GEOMETRY type, to pass through the IndexQuery filtering
// avoiding this filtering will have testing be more strict in what index readers returns.
}
return true;
}
Expand Down
14 changes: 1 addition & 13 deletions .../src/test/java/org/neo4j/kernel/api/index/SimpleRandomizedIndexAccessorCompatibility.java
View file
Expand Up @@ -39,7 +39,6 @@

import static org.hamcrest.Matchers.equalTo;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

@Ignore( "Not a test. This is a compatibility suite that provides test cases for verifying" +
" IndexProvider implementations. Each index provider that is to be tested by this suite" +
Expand Down Expand Up @@ -69,18 +68,7 @@ public void testRandomValues() throws Exception
.collect( Collectors.toList() );
IndexQuery.ExactPredicate exact = IndexQuery.exact( 100, value );
List<Long> result = query( exact );
if ( isGeometryValue( value ) )
{
// Because spatial index can have false positives we can only assert on contains
for ( Long expectedEntityId : expectedEntityIds )
{
assertTrue( "query: " + exact, result.contains( expectedEntityId ) );
}
}
else
{
assertThat( "query: " + exact, result, equalTo( expectedEntityIds ) );
}
assertThat( "query: " + exact, result, equalTo( expectedEntityIds ) );
}
}

Expand Down
42 changes: 21 additions & 21 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/CompositeGenericKey.java
View file
Expand Up @@ -47,17 +47,6 @@ 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 Expand Up @@ -110,6 +99,27 @@ void initValueAsHighest( int stateSlot, ValueGroup valueGroup )
states[stateSlot].initValueAsHighest( valueGroup );
}

/**
* 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 );
}

void initAsPrefixLow( int stateSlot, String prefix )
{
states[stateSlot].initAsPrefixLow( prefix );
}

void initAsPrefixHigh( int stateSlot, String prefix )
{
states[stateSlot].initAsPrefixHigh( prefix );
}

@Override
int compareValueTo( CompositeGenericKey other )
{
Expand All @@ -124,16 +134,6 @@ int compareValueTo( CompositeGenericKey other )
return 0;
}

void initAsPrefixLow( int stateSlot, String prefix )
{
states[stateSlot].initAsPrefixLow( prefix );
}

void initAsPrefixHigh( int stateSlot, String prefix )
{
states[stateSlot].initAsPrefixHigh( prefix );
}

void copyValuesFrom( CompositeGenericKey key )
{
if ( key.states.length != states.length )
Expand Down

0 comments on commit 3e397e1

Please sign in to comment.