diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/DefaultNodeValueIndexCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/DefaultNodeValueIndexCursor.java index 6537db42dd13e..3b058c58a6f28 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/DefaultNodeValueIndexCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/DefaultNodeValueIndexCursor.java @@ -385,13 +385,13 @@ private void suffixOrContainsQuery( IndexDescriptor descriptor, IndexQuery query if ( needsValues ) { - AddedWithValuesAndRemoved changes = indexUpdatesWithValuesForSuffixOrContains( txState, descriptor, query ); + AddedWithValuesAndRemoved changes = indexUpdatesWithValuesForSuffixOrContains( txState, descriptor, query, indexOrder ); addedWithValues = changes.getAdded().iterator(); removed = removed( txState, changes.getRemoved() ); } else { - AddedAndRemoved changes = indexUpdatesForSuffixOrContains( txState, descriptor, query ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( txState, descriptor, query, indexOrder ); added = changes.getAdded().longIterator(); removed = removed( txState, changes.getRemoved() ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/TxStateIndexChanges.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/TxStateIndexChanges.java index 48a684edd4d7c..3a00225ddfabb 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/TxStateIndexChanges.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/TxStateIndexChanges.java @@ -36,7 +36,6 @@ 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; import org.neo4j.storageengine.api.txstate.LongDiffSets; import org.neo4j.storageengine.api.txstate.ReadableTransactionState; import org.neo4j.values.storable.TextValue; @@ -62,118 +61,47 @@ class TxStateIndexChanges static AddedAndRemoved indexUpdatesForScan( ReadableTransactionState txState, IndexDescriptor descriptor, IndexOrder indexOrder ) { - Map updates = - indexOrder == IndexOrder.NONE ? - txState.getIndexUpdates( descriptor.schema() ) : - txState.getSortedIndexUpdates( descriptor.schema() ); - - if ( updates == null ) - { - return EMPTY_ADDED_AND_REMOVED; - } - - MutableLongList added = LongLists.mutable.empty(); - MutableLongSet removed = LongSets.mutable.empty(); - - for ( LongDiffSets diffSet : updates.values() ) - { - added.addAll( diffSet.getAdded() ); - removed.addAll( diffSet.getRemoved() ); - } - return new AddedAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed ); + return indexUpdatesForScanAndFilter( txState, descriptor, null, indexOrder ); } static AddedWithValuesAndRemoved indexUpdatesWithValuesForScan( ReadableTransactionState txState, IndexDescriptor descriptor, IndexOrder indexOrder ) { - Map updates = - indexOrder == IndexOrder.NONE ? - txState.getIndexUpdates( descriptor.schema() ) : - txState.getSortedIndexUpdates( descriptor.schema() ); - - if ( updates == null ) - { - return EMPTY_ADDED_AND_REMOVED_WITH_VALUES; - } - - MutableList added = Lists.mutable.empty(); - MutableLongSet removed = LongSets.mutable.empty(); - - for ( Map.Entry entry : updates.entrySet() ) - { - Value[] values = entry.getKey().getValues(); - LongDiffSets diffSets = entry.getValue(); - - diffSets.getAdded().each( nodeId -> added.add( new NodeWithPropertyValues( nodeId, values ) ) ); - removed.addAll( diffSets.getRemoved() ); - } - return new AddedWithValuesAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed ); + return indexUpdatesWithValuesScanAndFilter( txState, descriptor, null, indexOrder ); } // SUFFIX - static AddedAndRemoved indexUpdatesForSuffixOrContains( ReadableTransactionState txState, IndexDescriptor descriptor, IndexQuery query ) + static AddedAndRemoved indexUpdatesForSuffixOrContains( ReadableTransactionState txState, + IndexDescriptor descriptor, + IndexQuery query, + IndexOrder indexOrder ) { if ( descriptor.schema().getPropertyIds().length != 1 ) { throw new IllegalStateException( "Suffix and contains queries are only supported for single property queries" ); } - UnmodifiableMap updates = txState.getIndexUpdates( descriptor.schema() ); - if ( updates == null ) - { - return EMPTY_ADDED_AND_REMOVED; - } - - MutableLongList added = LongLists.mutable.empty(); - MutableLongSet removed = LongSets.mutable.empty(); - - for ( Map.Entry entry : updates.entrySet() ) - { - if ( query.acceptsValue( entry.getKey().getOnlyValue() ) ) - { - LongDiffSets diffSet = entry.getValue(); - added.addAll( diffSet.getAdded() ); - removed.addAll( diffSet.getRemoved() ); - } - } - return new AddedAndRemoved( added, removed ); + return indexUpdatesForScanAndFilter( txState, descriptor, query, indexOrder ); } - static AddedWithValuesAndRemoved indexUpdatesWithValuesForSuffixOrContains( ReadableTransactionState txState, IndexDescriptor descriptor, - IndexQuery query ) + static AddedWithValuesAndRemoved indexUpdatesWithValuesForSuffixOrContains( ReadableTransactionState txState, + IndexDescriptor descriptor, + IndexQuery query, + IndexOrder indexOrder ) { if ( descriptor.schema().getPropertyIds().length != 1 ) { throw new IllegalStateException( "Suffix and contains queries are only supported for single property queries" ); } - - UnmodifiableMap updates = txState.getIndexUpdates( descriptor.schema() ); - if ( updates == null ) - { - return EMPTY_ADDED_AND_REMOVED_WITH_VALUES; - } - - MutableList added = Lists.mutable.empty(); - MutableLongSet removed = LongSets.mutable.empty(); - - for ( Map.Entry entry : updates.entrySet() ) - { - ValueTuple key = entry.getKey(); - if ( query.acceptsValue( key.getOnlyValue() ) ) - { - Value[] values = key.getValues(); - LongDiffSets diffSets = entry.getValue(); - diffSets.getAdded().each( nodeId -> added.add( new NodeWithPropertyValues( nodeId, values ) ) ); - removed.addAll( diffSets.getRemoved() ); - } - } - return new AddedWithValuesAndRemoved( added, removed ); + return indexUpdatesWithValuesScanAndFilter( txState, descriptor, query, indexOrder ); } // SEEK - static AddedAndRemoved indexUpdatesForSeek( ReadableTransactionState txState, IndexDescriptor descriptor, ValueTuple values ) + static AddedAndRemoved indexUpdatesForSeek( ReadableTransactionState txState, + IndexDescriptor descriptor, + ValueTuple values ) { UnmodifiableMap updates = txState.getIndexUpdates( descriptor.schema() ); if ( updates != null ) @@ -388,6 +316,72 @@ static AddedWithValuesAndRemoved indexUpdatesWithValuesForRangeSeekByPrefix( Rea // HELPERS + private static AddedAndRemoved indexUpdatesForScanAndFilter( ReadableTransactionState txState, + IndexDescriptor descriptor, + IndexQuery filter, + IndexOrder indexOrder ) + { + Map updates = getUpdates( txState, descriptor, indexOrder ); + + if ( updates == null ) + { + return EMPTY_ADDED_AND_REMOVED; + } + + MutableLongList added = LongLists.mutable.empty(); + MutableLongSet removed = LongSets.mutable.empty(); + + for ( Map.Entry entry : updates.entrySet() ) + { + ValueTuple key = entry.getKey(); + if ( filter == null || filter.acceptsValue( key.getOnlyValue() ) ) + { + LongDiffSets diffSet = entry.getValue(); + added.addAll( diffSet.getAdded() ); + removed.addAll( diffSet.getRemoved() ); + } + } + return new AddedAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed ); + } + + private static AddedWithValuesAndRemoved indexUpdatesWithValuesScanAndFilter( ReadableTransactionState txState, + IndexDescriptor descriptor, + IndexQuery filter, + IndexOrder indexOrder ) + { + Map updates = getUpdates( txState, descriptor, indexOrder ); + + if ( updates == null ) + { + return EMPTY_ADDED_AND_REMOVED_WITH_VALUES; + } + + MutableList added = Lists.mutable.empty(); + MutableLongSet removed = LongSets.mutable.empty(); + + for ( Map.Entry entry : updates.entrySet() ) + { + ValueTuple key = entry.getKey(); + if ( filter == null || filter.acceptsValue( key.getOnlyValue() ) ) + { + Value[] values = key.getValues(); + LongDiffSets diffSet = entry.getValue(); + diffSet.getAdded().each( nodeId -> added.add( new NodeWithPropertyValues( nodeId, values ) ) ); + removed.addAll( diffSet.getRemoved() ); + } + } + return new AddedWithValuesAndRemoved( indexOrder == IndexOrder.DESCENDING ? added.asReversed() : added, removed ); + } + + private static Map getUpdates( ReadableTransactionState txState, + IndexDescriptor descriptor, + IndexOrder indexOrder ) + { + return indexOrder == IndexOrder.NONE ? + txState.getIndexUpdates( descriptor.schema() ) : + txState.getSortedIndexUpdates( descriptor.schema() ); + } + public static class AddedAndRemoved { private final LongIterable added; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/TxStateIndexChangesTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/TxStateIndexChangesTest.java index c3d845b80b93e..317d978bd692c 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/TxStateIndexChangesTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/newapi/TxStateIndexChangesTest.java @@ -303,10 +303,9 @@ void shouldComputeIndexUpdatesForRangeSeekByContainsWhenThereAreNoMatchingNodes( .build(); // WHEN - AddedAndRemoved changes = - indexUpdatesForSuffixOrContains( state, index, IndexQuery.stringContains( index.schema().getPropertyId(), "eulav" ) ); - AddedWithValuesAndRemoved changesWithValues = - indexUpdatesWithValuesForSuffixOrContains( state, index, IndexQuery.stringContains( index.schema().getPropertyId(), "eulav" ) ); + IndexQuery.StringContainsPredicate indexQuery = IndexQuery.stringContains( index.schema().getPropertyId(), "eulav" ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); + AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); // THEN assertTrue( changes.getAdded().isEmpty() ); @@ -329,10 +328,9 @@ void shouldComputeIndexUpdatesForRangeSeekBySuffixWhenThereArePartiallyMatchingN .build(); // WHEN - AddedAndRemoved changes = - indexUpdatesForSuffixOrContains( state, index, IndexQuery.stringSuffix( index.schema().getPropertyId(), "ella" ) ); - AddedWithValuesAndRemoved changesWithValues = - indexUpdatesWithValuesForSuffixOrContains( state, index, IndexQuery.stringSuffix( index.schema().getPropertyId(), "ella" ) ); + IndexQuery.StringSuffixPredicate indexQuery = IndexQuery.stringSuffix( index.schema().getPropertyId(), "ella" ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); + AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); // THEN assertContains( changes.getAdded(), 46L, 47L ); @@ -341,6 +339,49 @@ void shouldComputeIndexUpdatesForRangeSeekBySuffixWhenThereArePartiallyMatchingN nodeWithPropertyValues( 47L, "Cinderella" ) ); } + @Test + void shouldComputeIndexUpdatesForSuffixWithAscendingOrder() + { + assertRangeSeekBySuffixForOrder( IndexOrder.ASCENDING ); + } + + @Test + void shouldComputeIndexUpdatesForSuffixWithDescendingOrder() + { + assertRangeSeekBySuffixForOrder( IndexOrder.DESCENDING ); + } + + private void assertRangeSeekBySuffixForOrder( IndexOrder indexOrder ) + { + // GIVEN + ReadableTransactionState state = new TxStateBuilder() + .withAdded( 40L, "Aaron" ) + .withAdded( 41L, "Bonbon" ) + .withAdded( 42L, "Crayfish" ) + .withAdded( 43L, "Mayonnaise" ) + .withAdded( 44L, "Seashell" ) + .withAdded( 45L, "Ton" ) + .withAdded( 46L, "Macron" ) + .withAdded( 47L, "Tony" ) + .withAdded( 48L, "Evon" ) + .withAdded( 49L, "Andromeda" ) + .build(); + + // WHEN + IndexQuery indexQuery = IndexQuery.stringSuffix( index.schema().getPropertyId(), "on" ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( state, index, indexQuery, indexOrder ); + AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForSuffixOrContains( state, index, indexQuery, indexOrder ); + + NodeWithPropertyValues[] expected = {nodeWithPropertyValues( 40L, "Aaron" ), + nodeWithPropertyValues( 41L, "Bonbon" ), + nodeWithPropertyValues( 48L, "Evon" ), + nodeWithPropertyValues( 46L, "Macron" ), + nodeWithPropertyValues( 45L, "Ton" )}; + + // THEN + assertContains( indexOrder, changes, changesWithValues, expected ); + } + @Test void shouldComputeIndexUpdatesForRangeSeekByContainsWhenThereArePartiallyMatchingNewNodes() { @@ -357,10 +398,9 @@ void shouldComputeIndexUpdatesForRangeSeekByContainsWhenThereArePartiallyMatchin .build(); // WHEN - AddedAndRemoved changes = - indexUpdatesForSuffixOrContains( state, index, IndexQuery.stringContains( index.schema().getPropertyId(), "arbar" ) ); - AddedWithValuesAndRemoved changesWithValues = - indexUpdatesWithValuesForSuffixOrContains( state, index, IndexQuery.stringContains( index.schema().getPropertyId(), "arbar" ) ); + IndexQuery.StringContainsPredicate indexQuery = IndexQuery.stringContains( index.schema().getPropertyId(), "arbar" ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); + AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForSuffixOrContains( state, index, indexQuery, IndexOrder.NONE ); // THEN assertContains( changes.getAdded(), 45L, 46L ); @@ -368,6 +408,49 @@ void shouldComputeIndexUpdatesForRangeSeekByContainsWhenThereArePartiallyMatchin nodeWithPropertyValues( 45L, "Barbara" ), nodeWithPropertyValues( 46L, "Barbarella" ) ); } + + @Test + void shouldComputeIndexUpdatesForContainsWithAscendingOrder() + { + assertRangeSeekByContainsForOrder( IndexOrder.ASCENDING ); + } + + @Test + void shouldComputeIndexUpdatesForContainsWithDescendingOrder() + { + assertRangeSeekByContainsForOrder( IndexOrder.DESCENDING ); + } + + private void assertRangeSeekByContainsForOrder( IndexOrder indexOrder ) + { + // GIVEN + ReadableTransactionState state = new TxStateBuilder() + .withAdded( 40L, "Smashing" ) + .withAdded( 41L, "Bashley" ) + .withAdded( 42L, "Crasch" ) + .withAdded( 43L, "Mayonnaise" ) + .withAdded( 44L, "Seashell" ) + .withAdded( 45L, "Ton" ) + .withAdded( 46L, "The Flash" ) + .withAdded( 47L, "Strayhound" ) + .withAdded( 48L, "Trashy" ) + .withAdded( 49L, "Andromeda" ) + .build(); + + // WHEN + IndexQuery indexQuery = IndexQuery.stringContains( index.schema().getPropertyId(), "ash" ); + AddedAndRemoved changes = indexUpdatesForSuffixOrContains( state, index, indexQuery, indexOrder ); + AddedWithValuesAndRemoved changesWithValues = indexUpdatesWithValuesForSuffixOrContains( state, index, indexQuery, indexOrder ); + + NodeWithPropertyValues[] expected = {nodeWithPropertyValues( 41L, "Bashley" ), + nodeWithPropertyValues( 44L, "Seashell" ), + nodeWithPropertyValues( 40L, "Smashing" ), + nodeWithPropertyValues( 46L, "The Flash" ), + nodeWithPropertyValues( 48L, "Trashy" )}; + + // THEN + assertContains( indexOrder, changes, changesWithValues, expected ); + } } @Nested @@ -436,7 +519,7 @@ private void assertRangeSeekByPrefixForOrder( IndexOrder indexOrder ) } @Test - void shouldComputeIndexUpdatesForRangeSeekByPrefixWhenThereAreNonStringNodes() throws Exception + void shouldComputeIndexUpdatesForRangeSeekByPrefixWhenThereAreNonStringNodes() { // GIVEN final ReadableTransactionState state = new TxStateBuilder()