Skip to content

Commit

Permalink
Fix minimalSplitter when left and right are equal
Browse files Browse the repository at this point in the history
Previously when creating minimal splitter from left and right that where equal
the desired length of right to be copied to minimal splitter would be
length of right +1. This could cause out of bounds exceptions and unsorted
indexes. `minimalLengthFromRightNeededToDifferentiateFromLeft` now take
right max length into consideration.

Also add tests for indexing lots of equal values.
  • Loading branch information
burqen committed Aug 30, 2018
1 parent e288224 commit 1c49c05
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 6 deletions.
Expand Up @@ -26,6 +26,7 @@


import java.time.ZoneId; import java.time.ZoneId;
import java.time.ZoneOffset; import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;


Expand Down Expand Up @@ -427,6 +428,48 @@ public void testIndexEndsWithWithDuplicated() throws Exception
assertThat( query( stringSuffix( 1, "apa*" ) ), equalTo( Collections.emptyList() ) ); assertThat( query( stringSuffix( 1, "apa*" ) ), equalTo( Collections.emptyList() ) );
assertThat( query( stringSuffix( 1, "" ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L, 6L ) ) ); assertThat( query( stringSuffix( 1, "" ) ), equalTo( asList( 1L, 2L, 3L, 4L, 5L, 6L ) ) );
} }

@Test
public void testIndexShouldHandleLargeAmountOfDuplicatesString() throws Exception
{
doTestShouldHandleLargeAmountOfDuplicates( "this is a semi-long string that will need to be split" );
}

@Test
public void testIndexShouldHandleLargeAmountOfDuplicatesStringArray() throws Exception
{
Value arrayValue = nextRandomValidArrayValue();
doTestShouldHandleLargeAmountOfDuplicates( arrayValue );
}

private void doTestShouldHandleLargeAmountOfDuplicates( Object value ) throws Exception
{
List<IndexEntryUpdate<?>> updates = new ArrayList<>();
List<Long> nodeIds = new ArrayList<>();
for ( long i = 0; i < 1000; i++ )
{
nodeIds.add( i );
updates.add( add( i, descriptor.schema(), value ) );
}
updateAndCommit( updates );

assertThat( query( exists( 1 ) ), equalTo( nodeIds ) );
}

private Value nextRandomValidArrayValue()
{
Value value;
while ( true )
{
value = random.randomValues().nextArray();
// todo remove when spatial is supported by all
if ( testSuite.supportsSpatial() || !Values.isGeometryArray( value ) )
{
break;
}
}
return value;
}
} }


// This behaviour is expected by Unique indexes // This behaviour is expected by Unique indexes
Expand Down
Expand Up @@ -473,15 +473,16 @@ private static void minimalSplitterArray( GenericKeyState left, GenericKeyState
// Convert from last equal index to first index to differ +1 // Convert from last equal index to first index to differ +1
// Convert from index to length +1 // Convert from index to length +1
// Total +2 // Total +2
arrayCopier.copyArray( into, right, lastEqualIndex + 2 ); int length = Math.min( right.arrayLength, lastEqualIndex + 2 );
arrayCopier.copyArray( into, right, length );
} }


private static void minimalSplitterText( GenericKeyState left, GenericKeyState right, GenericKeyState into ) private static void minimalSplitterText( GenericKeyState left, GenericKeyState right, GenericKeyState into )
{ {
int length = 0; int length = 0;
if ( left.type == Type.TEXT ) if ( left.type == Type.TEXT )
{ {
length = StringLayout.firstPosToDiffer( left.byteArray, (int) left.long0, right.byteArray, (int) right.long0 ); length = StringLayout.minimalLengthFromRightNeededToDifferentiateFromLeft( left.byteArray, (int) left.long0, right.byteArray, (int) right.long0 );
} }
into.writeUTF8( right.byteArray, 0, length ); into.writeUTF8( right.byteArray, 0, length );
} }
Expand Down
Expand Up @@ -22,7 +22,7 @@
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 static java.lang.Integer.min; import static java.lang.Math.min;
import static java.lang.String.format; import static java.lang.String.format;
import static org.neo4j.kernel.impl.index.schema.StringIndexKey.ENTITY_ID_SIZE; import static org.neo4j.kernel.impl.index.schema.StringIndexKey.ENTITY_ID_SIZE;


Expand Down Expand Up @@ -87,11 +87,11 @@ public boolean fixedSize()
@Override @Override
public void minimalSplitter( StringIndexKey left, StringIndexKey right, StringIndexKey into ) public void minimalSplitter( StringIndexKey left, StringIndexKey right, StringIndexKey into )
{ {
int targetLength = firstPosToDiffer( left.bytes, left.bytesLength, right.bytes, right.bytesLength ); int targetLength = minimalLengthFromRightNeededToDifferentiateFromLeft( left.bytes, left.bytesLength, right.bytes, right.bytesLength );
into.copyFrom( right, targetLength ); into.copyFrom( right, targetLength );
} }


static int firstPosToDiffer( byte[] leftBytes, int leftLength, byte[] rightBytes, int rightLength ) static int minimalLengthFromRightNeededToDifferentiateFromLeft( byte[] leftBytes, int leftLength, byte[] rightBytes, int rightLength )
{ {
int lastEqualIndex = -1; int lastEqualIndex = -1;
int maxLength = min( leftLength, rightLength ); int maxLength = min( leftLength, rightLength );
Expand All @@ -106,7 +106,7 @@ static int firstPosToDiffer( byte[] leftBytes, int leftLength, byte[] rightBytes
// Convert from last equal index to first index to differ +1 // Convert from last equal index to first index to differ +1
// Convert from index to length +1 // Convert from index to length +1
// Total +2 // Total +2
return lastEqualIndex + 2; return Math.min( rightLength, lastEqualIndex + 2 );
} }


@Override @Override
Expand Down
Expand Up @@ -259,6 +259,13 @@ void mustProduceValidMinimalSplitters( ValueGenerator valueGenerator )
assertValidMinimalSplitter( left, right ); assertValidMinimalSplitter( left, right );
} }


@ParameterizedTest
@MethodSource( "validValueGenerators" )
void mustProduceValidMinimalSplittersWhenValuesAreEqual( ValueGenerator valueGenerator )
{
assertValidMinimalSplitterForEqualValues( valueGenerator.next() );
}

@ParameterizedTest @ParameterizedTest
@MethodSource( "validValueGenerators" ) @MethodSource( "validValueGenerators" )
void mustReportCorrectSize( ValueGenerator valueGenerator ) void mustReportCorrectSize( ValueGenerator valueGenerator )
Expand Down Expand Up @@ -457,6 +464,22 @@ private void assertValidMinimalSplitter( Value left, Value right )
"right state not less than minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter ); "right state not less than minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter );
} }


private void assertValidMinimalSplitterForEqualValues( Value value )
{
GenericKeyState leftState = new GenericKeyState();
leftState.writeValue( value, NEUTRAL );
GenericKeyState rightState = new GenericKeyState();
rightState.writeValue( value, NEUTRAL );

GenericKeyState minimalSplitter = new GenericKeyState();
GenericKeyState.minimalSplitter( leftState, rightState, minimalSplitter );

assertTrue( leftState.compareValueTo( minimalSplitter ) == 0,
"left state not equal to minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter );
assertTrue( rightState.compareValueTo( minimalSplitter ) == 0,
"right state equal to minimal splitter, leftState=" + leftState + ", rightState=" + rightState + ", minimalSplitter=" + minimalSplitter );
}

private static Value nextValidValue() private static Value nextValidValue()
{ {
Value value; Value value;
Expand Down

0 comments on commit 1c49c05

Please sign in to comment.