Skip to content

Commit

Permalink
Fixes from code review
Browse files Browse the repository at this point in the history
- Disable intersection scan for now
- Some stuff forgot to remove
- Some clean up
  • Loading branch information
pontusmelke committed Jan 15, 2018
1 parent f9f15fe commit 647d38c
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 44 deletions.
Expand Up @@ -25,6 +25,5 @@ import org.neo4j.cypher.internal.util.v3_4.spi.MapToPublicExceptions
class CantCompileQueryException(message: String = "Internal error - should have used fall back to execute query, but something went horribly wrong", cause:Throwable=null)
extends CypherException(message, cause) {

if (cause != null) cause.printStackTrace()
def mapToPublic[T <: Throwable](thrower: MapToPublicExceptions[T]) = throw new CantCompileQueryException(message)
}
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.internal.kernel.api;

import org.junit.Ignore;
import org.junit.Test;

import org.neo4j.collection.primitive.Primitive;
Expand Down Expand Up @@ -104,7 +105,7 @@ public void shouldFindNodesByDisjunction() throws Exception
}
}

@Test
@Ignore
public void shouldFindNodesByConjunction() throws Exception
{
// given
Expand Down
Expand Up @@ -19,6 +19,7 @@
*/
package org.neo4j.internal.kernel.api;

import org.junit.Ignore;
import org.junit.Test;

import java.util.Arrays;
Expand Down Expand Up @@ -554,14 +555,15 @@ public void shouldNotFindNodeWithRemovedLabelInLabelScan() throws Exception
public void shouldFindUpdatedNodeInInLabelScan() throws Exception
{
// Given
Node node = createNode( "label" );
Node node = createNode( );

try ( org.neo4j.internal.kernel.api.Transaction tx = session.beginTransaction();
NodeLabelIndexCursor cursor = cursors.allocateNodeLabelIndexCursor() )
{
// when
tx.dataWrite().nodeAddLabel( node.node, node.labels[0] );
tx.dataRead().nodeLabelScan( node.labels[0], cursor );
int label = tx.tokenWrite().labelGetOrCreateForName( "label" );
tx.dataWrite().nodeAddLabel( node.node, label );
tx.dataRead().nodeLabelScan( label, cursor );

// then
assertTrue( cursor.next() );
Expand Down Expand Up @@ -609,7 +611,7 @@ public void shouldNotFindDeletedNodeInDisjunctionLabelScan() throws Exception
}

@Test
public void shouldNotFindNodeWithRemovedLabelInDisjunctionLabelScan() throws Exception
public void shouldFindNodeWithOneRemovedLabelInDisjunctionLabelScan() throws Exception
{
// Given
Node node = createNode( "label1", "label2" );
Expand All @@ -621,6 +623,26 @@ public void shouldNotFindNodeWithRemovedLabelInDisjunctionLabelScan() throws Exc
tx.dataWrite().nodeRemoveLabel( node.node, node.labels[1] );
tx.dataRead().nodeLabelUnionScan( cursor, node.labels );

// then
assertTrue( cursor.next() );
assertEquals( node.node, cursor.nodeReference() );
}
}

@Test
public void shouldNotFindNodeWithAllRemovedLabelsInDisjunctionLabelScan() throws Exception
{
// Given
Node node = createNode( "label1", "label2" );

try ( org.neo4j.internal.kernel.api.Transaction tx = session.beginTransaction();
NodeLabelIndexCursor cursor = cursors.allocateNodeLabelIndexCursor() )
{
// when
tx.dataWrite().nodeRemoveLabel( node.node, node.labels[0] );
tx.dataWrite().nodeRemoveLabel( node.node, node.labels[1] );
tx.dataRead().nodeLabelUnionScan( cursor, node.labels );

// then
assertFalse( cursor.next() );
}
Expand All @@ -646,7 +668,7 @@ public void shouldFindUpdatedNodeInInDisjunctionLabelScan() throws Exception
}
}

@Test
@Ignore
public void shouldNotFindDeletedNodeInConjunctionLabelScan() throws Exception
{
// Given
Expand Down Expand Up @@ -682,26 +704,46 @@ public void shouldNotFindNodeWithRemovedLabelInConjunctionLabelScan() throws Exc
}
}

@Test
@Ignore
public void shouldFindUpdatedNodeInInConjunctionLabelScan() throws Exception
{
// Given
Node node = createNode();
Node node = createNode("label1");

try ( org.neo4j.internal.kernel.api.Transaction tx = session.beginTransaction();
NodeLabelIndexCursor cursor = cursors.allocateNodeLabelIndexCursor() )
{
// when
int label1 = tx.tokenWrite().labelGetOrCreateForName( "label1" );
tx.dataWrite().nodeAddLabel( node.node, label1 );
tx.dataRead().nodeLabelIntersectionScan( cursor, label1 );
int label2 = tx.tokenWrite().labelGetOrCreateForName( "label2" );
tx.dataWrite().nodeAddLabel( node.node, label2 );
tx.dataRead().nodeLabelIntersectionScan( cursor, node.labels[0], label2 );

// then
assertTrue( cursor.next() );
assertEquals( node.node, cursor.nodeReference() );
}
}

@Ignore
public void shouldNotFindNodeWithJustOneUpdatedLabelInInConjunctionLabelScan() throws Exception
{
// Given
Node node = createNode();

try ( org.neo4j.internal.kernel.api.Transaction tx = session.beginTransaction();
NodeLabelIndexCursor cursor = cursors.allocateNodeLabelIndexCursor() )
{
// when
int label1 = tx.tokenWrite().labelGetOrCreateForName( "labe1" );
int label2 = tx.tokenWrite().labelGetOrCreateForName( "label2" );
tx.dataWrite().nodeAddLabel( node.node, label2 );
tx.dataRead().nodeLabelIntersectionScan( cursor, label1, label2 );

// then
assertFalse( cursor.next() );
}
}

private void assertLabels( LabelSet labels, int... expected )
{
assertEquals( expected.length, labels.numberOfLabels() );
Expand Down
Expand Up @@ -21,6 +21,7 @@

import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -720,25 +721,46 @@ public Cursor<RelationshipItem> augmentRelationshipsGetAllCursor( Cursor<Relatio
}

@Override
public ReadableDiffSets<Long> nodesWithLabelChanged( int... labels )
public ReadableDiffSets<Long> nodesWithLabelChanged( int label )
{
assert labels.length > 0;
return LABEL_STATE.get( this, label ).nodeDiffSets();
}

if ( labels.length == 1 )
{
return LABEL_STATE.get( this, labels[0] ).nodeDiffSets();
}
else
@Override
public ReadableDiffSets<Long> nodesWithAnyOfLabelsChanged( int... labels )
{
//It is enough that one of the labels is added
//It is necessary for all the labels are removed
Set<Long> added = new HashSet<>();
Set<Long> removed = new HashSet<>();
for ( int i = 0; i < labels.length; i++ )
{
DiffSets<Long> changes = new DiffSets<>();
for ( int label : labels )
ReadableDiffSets<Long> nodeDiffSets = LABEL_STATE.get( this, labels[i] ).nodeDiffSets();
if ( i == 0 )
{
removed.addAll( nodeDiffSets.getRemoved() );
}
else
{
LabelState labelState = LABEL_STATE.get( this, label );
changes.addAll( labelState.nodeDiffSets().getAdded().iterator() );
changes.removeAll( labelState.nodeDiffSets().getRemoved().iterator() );
removed.retainAll( nodeDiffSets.getRemoved() );
}
return changes;
added.addAll( nodeDiffSets.getAdded() );
}

return new DiffSets<>( added, removed );
}

@Override
public ReadableDiffSets<Long> nodesWithAllLabelsChanged( int... labels )
{
DiffSets<Long> changes = new DiffSets<>();
for ( int label : labels )
{
LabelState labelState = LABEL_STATE.get( this, label );
changes.addAll( labelState.nodeDiffSets().getAdded().iterator() );
changes.removeAll( labelState.nodeDiffSets().getRemoved().iterator() );
}
return changes;
}

@Override
Expand Down
Expand Up @@ -123,7 +123,7 @@ public void nodesWithLabel( IndexProgressor.NodeLabelClient client, int labelId
throw new UncheckedIOException( e );
}

client.initialize( new LabelScanValueIndexProgressor( cursor, openCursors, client ), false, labelId );
client.scan( new LabelScanValueIndexProgressor( cursor, openCursors, client ), false, labelId );
}

private List<PrimitiveLongResourceIterator> iteratorsForLabels( int[] labelIds )
Expand Down
Expand Up @@ -44,16 +44,38 @@ class NodeLabelIndexCursor extends IndexCursor
}

@Override
public void initialize( IndexProgressor progressor, boolean providesLabels, int... labels )
public void scan( IndexProgressor progressor, boolean providesLabels, int label )
{
super.initialize( progressor );
if ( read.hasTxStateWithChanges() )
{
changes = read.txState().nodesWithLabelChanged( labels );
changes = read.txState().nodesWithLabelChanged( label );
added = changes.augment( PrimitiveLongCollections.emptyIterator() );
}
}

@Override
public void unionScan( IndexProgressor progressor, boolean providesLabels, int... labels )
{
super.initialize( progressor );
if ( read.hasTxStateWithChanges() )
{
changes = read.txState().nodesWithAnyOfLabelsChanged( labels );
added = changes.augment( PrimitiveLongCollections.emptyIterator() );
}
}

@Override
public void intersectionScan( IndexProgressor progressor, boolean providesLabels, int... labels )
{
super.initialize( progressor );
//TODO: Currently we don't have a good way of handling this in the tx state
//The problem is for the nodes where some - but not all of the labels - are
//added in the transaction. For these we need to go to disk and check if they
//have the missing labels and hence return them or if not discard them.
throw new UnsupportedOperationException( );
}

@Override
public boolean acceptNode( long reference, LabelSet labels )
{
Expand Down
Expand Up @@ -21,7 +21,6 @@

import java.util.Arrays;

import org.neo4j.collection.primitive.PrimitiveLongResourceIterator;
import org.neo4j.internal.kernel.api.CapableIndexReference;
import org.neo4j.internal.kernel.api.IndexOrder;
import org.neo4j.internal.kernel.api.IndexQuery;
Expand Down Expand Up @@ -141,30 +140,30 @@ public final void nodeLabelScan( int label, org.neo4j.internal.kernel.api.NodeLa

NodeLabelIndexCursor indexCursor = (NodeLabelIndexCursor) cursor;
indexCursor.setRead( this );
labelScanReader().nodesWithLabel( indexCursor, label );
labelScanReader().nodesWithLabel( indexCursor, label);
}

@Override
public void nodeLabelUnionScan( org.neo4j.internal.kernel.api.NodeLabelIndexCursor cursor, int... labels )
{
ktx.assertOpen();

((NodeLabelIndexCursor) cursor).setRead( this );
labelScan( (NodeLabelIndexCursor) cursor, labelScanReader().nodesWithAnyOfLabels( labels ), labels );
NodeLabelIndexCursor client = (NodeLabelIndexCursor) cursor;
client.setRead( this );
client.unionScan( new NodeLabelIndexProgressor( labelScanReader().nodesWithAnyOfLabels( labels ), client ),
false, labels );
}

@Override
public void nodeLabelIntersectionScan( org.neo4j.internal.kernel.api.NodeLabelIndexCursor cursor, int... labels )
{
ktx.assertOpen();

((NodeLabelIndexCursor) cursor).setRead( this );
labelScan( (NodeLabelIndexCursor) cursor, labelScanReader().nodesWithAllLabels( labels ), labels );
}

private void labelScan( IndexProgressor.NodeLabelClient client, PrimitiveLongResourceIterator iterator, int... labels )
{
client.initialize( new NodeLabelIndexProgressor( iterator, client ), false, labels );
NodeLabelIndexCursor client = (NodeLabelIndexCursor) cursor;
client.setRead( this );
client.intersectionScan(
new NodeLabelIndexProgressor( labelScanReader().nodesWithAllLabels( labels ), client ),
false, labels );
}

@Override
Expand Down
Expand Up @@ -98,9 +98,13 @@ interface NodeLabelClient
* Setup the client for progressing using the supplied progressor. Called by index implementation.
* @param progressor the progressor
* @param providesLabels true if the progression can provide label information
* @param labels
* @param label the label to scan for
*/
void initialize( IndexProgressor progressor, boolean providesLabels, int... labels );
void scan( IndexProgressor progressor, boolean providesLabels, int label );

void unionScan( IndexProgressor progressor, boolean providesLabels, int... labels );

void intersectionScan( IndexProgressor progressor, boolean providesLabels, int... labels );

/**
* Accept the node id and (some) labels of a candidate index entry. Return true if the entry
Expand Down
Expand Up @@ -54,7 +54,17 @@ public interface ReadableTransactionState
/**
* Returns all nodes that, in this tx, have had the labels changed.
*/
ReadableDiffSets<Long> nodesWithLabelChanged( int... labels );
ReadableDiffSets<Long> nodesWithLabelChanged( int label );

/**
* Returns all nodes that, in this tx, have had any of the labels changed.
*/
ReadableDiffSets<Long> nodesWithAnyOfLabelsChanged( int... labels );

/**
* Returns all nodes that, in this tx, have had all the labels changed.
*/
ReadableDiffSets<Long> nodesWithAllLabelsChanged( int... labels );

/**
* Returns nodes that have been added and removed in this tx.
Expand Down
Expand Up @@ -157,8 +157,8 @@ public void shouldHandleMultipleLabels() throws Exception
state.nodeDoAddLabel( 3, 5 );

// WHEN
Set<Long> removed = state.nodesWithLabelChanged( 1, 2, 3 ).getRemoved();
Set<Long> added = state.nodesWithLabelChanged( 1, 2, 3 ).getAdded();
Set<Long> removed = state.nodesWithAllLabelsChanged( 1, 2, 3 ).getRemoved();
Set<Long> added = state.nodesWithAllLabelsChanged( 1, 2, 3 ).getAdded();

// THEN
assertEquals( asSet( 0L, 1L, 2L ), Iterables.asSet( removed ) );
Expand Down

0 comments on commit 647d38c

Please sign in to comment.