Skip to content

Commit

Permalink
Let TxStateIndexChanges perform index scan in order
Browse files Browse the repository at this point in the history
  • Loading branch information
@fickludd
fickludd committed Sep 17, 2018
1 parent da354b8 commit c1f84e3
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 18 deletions.
6 changes: 4 additions & 2 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/DefaultNodeValueIndexCursor.java
View file
Expand Up @@ -29,6 +29,7 @@
import java.util.Iterator;

import org.neo4j.graphdb.Resource;
import org.neo4j.internal.kernel.api.IndexOrder;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.kernel.api.NodeCursor;
import org.neo4j.internal.kernel.api.NodeValueIndexCursor;
Expand Down Expand Up @@ -58,6 +59,7 @@ final class DefaultNodeValueIndexCursor extends IndexCursor<IndexProgressor>
private Iterator<NodeWithPropertyValues> addedWithValues = Collections.emptyIterator();
private LongSet removed = LongSets.immutable.empty();
private boolean needsValues;
private IndexOrder indexOrder = IndexOrder.NONE;
private final DefaultCursors pool;

DefaultNodeValueIndexCursor( DefaultCursors pool )
Expand Down Expand Up @@ -308,13 +310,13 @@ private void scanQuery( IndexDescriptor descriptor )
if ( needsValues )
{
AddedWithValuesAndRemoved changes =
TxStateIndexChanges.indexUpdatesWithValuesForScan( txState, descriptor );
TxStateIndexChanges.indexUpdatesWithValuesForScan( txState, descriptor, indexOrder );
addedWithValues = changes.added.iterator();
removed = removed( txState, changes.removed );
}
else
{
AddedAndRemoved changes = TxStateIndexChanges.indexUpdatesForScan( txState, descriptor );
AddedAndRemoved changes = TxStateIndexChanges.indexUpdatesForScan( txState, descriptor, indexOrder );
added = changes.added.longIterator();
removed = removed( txState, changes.removed );
}
Expand Down
23 changes: 17 additions & 6 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/TxStateIndexChanges.java
View file
Expand Up @@ -33,6 +33,7 @@
import java.util.Map;
import java.util.NavigableMap;

import org.neo4j.internal.kernel.api.IndexOrder;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.storageengine.api.schema.IndexDescriptor;
import org.neo4j.storageengine.api.txstate.DiffSets;
Expand All @@ -57,9 +58,13 @@ class TxStateIndexChanges
new AddedAndRemoved( LongLists.immutable.empty(), LongSets.immutable.empty() );
private static final ValueTuple MAX_STRING_TUPLE = ValueTuple.of( Values.MAX_STRING );

static AddedAndRemoved indexUpdatesForScan( ReadableTransactionState txState, IndexDescriptor descriptor )
static AddedAndRemoved indexUpdatesForScan( ReadableTransactionState txState, IndexDescriptor descriptor, IndexOrder indexOrder )
{
UnmodifiableMap<ValueTuple,? extends LongDiffSets> updates = txState.getIndexUpdates( descriptor.schema() );
Map<ValueTuple,? extends LongDiffSets> updates =
indexOrder == IndexOrder.NONE ?
txState.getIndexUpdates( descriptor.schema() ) :
txState.getSortedIndexUpdates( descriptor.schema() );

if ( updates == null )
{
return EMPTY_ADDED_AND_REMOVED;
Expand All @@ -73,12 +78,18 @@ static AddedAndRemoved indexUpdatesForScan( ReadableTransactionState txState, In
added.addAll( diffSet.getAdded() );
removed.addAll( diffSet.getRemoved() );
}
return new AddedAndRemoved( added, removed );
return new AddedAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed );
}

static AddedWithValuesAndRemoved indexUpdatesWithValuesForScan( ReadableTransactionState txState, IndexDescriptor descriptor )
static AddedWithValuesAndRemoved indexUpdatesWithValuesForScan( ReadableTransactionState txState,
IndexDescriptor descriptor,
IndexOrder indexOrder )
{
UnmodifiableMap<ValueTuple,? extends LongDiffSets> updates = txState.getIndexUpdates( descriptor.schema() );
Map<ValueTuple,? extends LongDiffSets> updates =
indexOrder == IndexOrder.NONE ?
txState.getIndexUpdates( descriptor.schema() ) :
txState.getSortedIndexUpdates( descriptor.schema() );

if ( updates == null )
{
return EMPTY_ADDED_AND_REMOVED_WITH_VALUES;
Expand All @@ -95,7 +106,7 @@ static AddedWithValuesAndRemoved indexUpdatesWithValuesForScan( ReadableTransact
diffSets.getAdded().each( nodeId -> added.add( new NodeWithPropertyValues( nodeId, values ) ) );
removed.addAll( diffSets.getRemoved() );
}
return new AddedWithValuesAndRemoved( added, removed );
return new AddedWithValuesAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed );
}

static AddedAndRemoved indexUpdatesForSuffixOrContains( ReadableTransactionState txState, IndexDescriptor descriptor, IndexQuery query )
Expand Down
71 changes: 61 additions & 10 deletions community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/TxStateIndexChangesTest.java
View file
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.kernel.impl.newapi;

import org.apache.commons.lang3.ArrayUtils;
import org.eclipse.collections.api.LongIterable;
import org.eclipse.collections.impl.UnmodifiableMap;
import org.eclipse.collections.impl.factory.primitive.LongSets;
Expand All @@ -32,12 +33,11 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeMap;

import org.neo4j.helpers.collection.Pair;
import org.neo4j.internal.kernel.api.IndexOrder;
import org.neo4j.internal.kernel.api.IndexQuery;
import org.neo4j.internal.kernel.api.schema.SchemaDescriptor;
import org.neo4j.kernel.api.schema.index.TestIndexDescriptorFactory;
Expand Down Expand Up @@ -79,8 +79,8 @@ void shouldComputeIndexUpdatesForScanOnAnEmptyTxState()
final ReadableTransactionState state = Mockito.mock( ReadableTransactionState.class );

// WHEN
AddedAndRemoved changes = indexUpdatesForScan( state, index );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, index );
AddedAndRemoved changes = indexUpdatesForScan( state, index, IndexOrder.NONE );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, index, IndexOrder.NONE );

// THEN
assertTrue( changes.isEmpty() );
Expand All @@ -97,14 +97,65 @@ void shouldComputeIndexUpdatesForScanWhenThereAreNewNodes()
.build();

// WHEN
AddedAndRemoved changes = indexUpdatesForScan( state, index );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, index );
AddedAndRemoved changes = indexUpdatesForScan( state, index, IndexOrder.NONE );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, index, IndexOrder.NONE );

// THEN
assertContains( changes.added, 42L, 43L );
assertContains( changesWithValues.added, nodeWithPropertyValues( 42L, "foo" ), nodeWithPropertyValues( 43L, "bar" ) );
}

@Test
void shouldComputeIndexUpdatesForScanWithAscendingOrder()
{
assertScanWithOrder( IndexOrder.ASCENDING );
}

@Test
void shouldComputeIndexUpdatesForScanWithDescendingOrder()
{
assertScanWithOrder( IndexOrder.DESCENDING );
}

private void assertScanWithOrder( IndexOrder indexOrder )
{
// GIVEN
final ReadableTransactionState state = new TxStateBuilder()
.withAdded( 40L, "Aaron" )
.withAdded( 41L, "Agatha" )
.withAdded( 42L, "Andreas" )
.withAdded( 43L, "Barbarella" )
.withAdded( 44L, "Andrea" )
.withAdded( 45L, "Aristotle" )
.withAdded( 46L, "Barbara" )
.withAdded( 47L, "Cinderella" )
.build();

// WHEN
AddedAndRemoved changes = indexUpdatesForScan( state, index, indexOrder );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, index, indexOrder );

NodeWithPropertyValues[] expectedNodesWithValues = {nodeWithPropertyValues( 40L, "Aaron" ),
nodeWithPropertyValues( 41L, "Agatha" ),
nodeWithPropertyValues( 44L, "Andrea" ),
nodeWithPropertyValues( 42L, "Andreas" ),
nodeWithPropertyValues( 45L, "Aristotle" ),
nodeWithPropertyValues( 46L, "Barbara" ),
nodeWithPropertyValues( 43L, "Barbarella" ),
nodeWithPropertyValues( 47L, "Cinderella" )};

if ( indexOrder == IndexOrder.DESCENDING )
{
ArrayUtils.reverse( expectedNodesWithValues );
}

long[] expectedNodeIds = Arrays.stream( expectedNodesWithValues ).mapToLong( NodeWithPropertyValues::getNodeId ).toArray();

// THEN
assertContainsInOrder( changes.added, expectedNodeIds );
assertContainsInOrder( changesWithValues.added, expectedNodesWithValues );
}

@Test
void shouldComputeIndexUpdatesForSeekWhenThereAreNewNodes()
{
Expand Down Expand Up @@ -407,8 +458,8 @@ void shouldScanWhenThereAreNewNodes()
.build();

// WHEN
AddedAndRemoved changes = indexUpdatesForScan( state, compositeIndex );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, compositeIndex );
AddedAndRemoved changes = indexUpdatesForScan( state, compositeIndex, IndexOrder.NONE );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, compositeIndex, IndexOrder.NONE );

// THEN
assertContains( changes.added, 42L, 43L );
Expand Down Expand Up @@ -461,8 +512,8 @@ void shouldHandleMixedAddsAndRemovesEntry()
.build();

// WHEN
AddedAndRemoved changes = indexUpdatesForScan( state, compositeIndex );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, compositeIndex );
AddedAndRemoved changes = indexUpdatesForScan( state, compositeIndex, IndexOrder.NONE );
AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForScan( state, compositeIndex, IndexOrder.NONE );

// THEN
assertContains( changes.added, 42L );
Expand Down

0 comments on commit c1f84e3

Please sign in to comment.