Skip to content

Commit

Permalink
Fixes legacy index tx state issue where deleted entities might be vis…
Browse files Browse the repository at this point in the history 
…ible

when querying. Problem was when querying w/ start/end node. As it was the
tx state didn't have the start/end node... something that a refactoring
between 2.1 and 2.2 introduced.
  • Loading branch information
@tinwelint
tinwelint committed May 27, 2015
1 parent 0dc0bf9 commit 9a868b8
Show file tree
Hide file tree
Showing 14 changed files with 306 additions and 43 deletions.
4 changes: 0 additions & 4 deletions community/graph-algo/src/test/java/org/neo4j/graphalgo/path/TestExactDepthPathFinder.java
View file
Expand Up @@ -19,8 +19,6 @@
*/
package org.neo4j.graphalgo.path;

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

import org.neo4j.graphalgo.GraphAlgoFactory;
Expand All @@ -33,9 +31,7 @@
import org.neo4j.graphdb.RelationshipExpander;
import org.neo4j.kernel.Traversal;

import static org.junit.Assert.fail;
import common.Neo4jAlgoTestCase;
import common.Neo4jAlgoTestCase.MyRelTypes;
import static org.junit.Assert.assertNotNull;

public class TestExactDepthPathFinder extends Neo4jAlgoTestCase
Expand Down
6 changes: 6 additions & 0 deletions community/kernel/src/main/java/org/neo4j/kernel/api/LegacyIndex.java
View file
Expand Up @@ -49,4 +49,10 @@ public interface LegacyIndex
LegacyIndexHits query( Object queryOrQueryObject, long startNode, long endNode );

void addRelationship( long entity, String key, Object value, long startNode, long endNode );

void removeRelationship( long entity, String key, Object value, long startNode, long endNode );

void removeRelationship( long entity, String key, long startNode, long endNode );

void removeRelationship( long entity, long startNode, long endNode );
}
7 changes: 4 additions & 3 deletions community/kernel/src/main/java/org/neo4j/kernel/api/LegacyIndexWrite.java
View file
Expand Up @@ -66,12 +66,13 @@ void relationshipAddToLegacyIndex( String indexName, long relationship, String k
throws EntityNotFoundException, LegacyIndexNotFoundKernelException;

void relationshipRemoveFromLegacyIndex( String indexName, long relationship, String key, Object value )
throws LegacyIndexNotFoundKernelException;
throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void relationshipRemoveFromLegacyIndex( String indexName, long relationship, String key )
throws LegacyIndexNotFoundKernelException;
throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void relationshipRemoveFromLegacyIndex( String indexName, long relationship ) throws LegacyIndexNotFoundKernelException;
void relationshipRemoveFromLegacyIndex( String indexName, long relationship )
throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void nodeLegacyIndexDrop( String indexName ) throws LegacyIndexNotFoundKernelException;

Expand Down
6 changes: 3 additions & 3 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/api/OperationsFacade.java
View file
Expand Up @@ -883,23 +883,23 @@ public void relationshipAddToLegacyIndex( String indexName, long relationship, S

@Override
public void relationshipRemoveFromLegacyIndex( String indexName, long relationship, String key, Object value )
throws LegacyIndexNotFoundKernelException
throws EntityNotFoundException, LegacyIndexNotFoundKernelException
{
statement.assertOpen();
legacyIndexWrite().relationshipRemoveFromLegacyIndex( statement, indexName, relationship, key, value );
}

@Override
public void relationshipRemoveFromLegacyIndex( String indexName, long relationship, String key )
throws LegacyIndexNotFoundKernelException
throws LegacyIndexNotFoundKernelException, EntityNotFoundException
{
statement.assertOpen();
legacyIndexWrite().relationshipRemoveFromLegacyIndex( statement, indexName, relationship, key );
}

@Override
public void relationshipRemoveFromLegacyIndex( String indexName, long relationship )
throws LegacyIndexNotFoundKernelException
throws LegacyIndexNotFoundKernelException, EntityNotFoundException
{
statement.assertOpen();
legacyIndexWrite().relationshipRemoveFromLegacyIndex( statement, indexName, relationship );
Expand Down
58 changes: 49 additions & 9 deletions ...nity/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java
View file
Expand Up @@ -1383,24 +1383,64 @@ public void visit( long relId, int type, long startNode, long endNode )
}

@Override
public void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship,
String key, Object value ) throws LegacyIndexNotFoundKernelException
public void relationshipRemoveFromLegacyIndex( final KernelStatement statement, final String indexName, long relationship,
final String key, final Object value ) throws LegacyIndexNotFoundKernelException, EntityNotFoundException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).remove( relationship, key, value );
relationshipVisit( statement, relationship, new RelationshipVisitor<LegacyIndexNotFoundKernelException>()
{
@Override
public void visit( long relId, int type, long startNode, long endNode )
throws LegacyIndexNotFoundKernelException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).removeRelationship(
relId, key, value, startNode, endNode );
}
} );
}

@Override
public void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship,
String key ) throws LegacyIndexNotFoundKernelException
public void relationshipRemoveFromLegacyIndex( final KernelStatement statement, final String indexName, long relationship,
final String key ) throws EntityNotFoundException, LegacyIndexNotFoundKernelException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).remove( relationship, key );
relationshipVisit( statement, relationship, new RelationshipVisitor<LegacyIndexNotFoundKernelException>()
{
@Override
public void visit( long relId, int type, long startNode, long endNode )
throws LegacyIndexNotFoundKernelException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).removeRelationship(
relId, key, startNode, endNode );
}
} );
}

@Override
public void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship )
throws LegacyIndexNotFoundKernelException
public void relationshipRemoveFromLegacyIndex( final KernelStatement statement, final String indexName, long relationship )
throws LegacyIndexNotFoundKernelException, EntityNotFoundException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).remove( relationship );
try
{
relationshipVisit( statement, relationship, new RelationshipVisitor<LegacyIndexNotFoundKernelException>()
{
@Override
public void visit( long relId, int type, long startNode, long endNode )
throws LegacyIndexNotFoundKernelException
{
statement.legacyIndexTxState().relationshipChanges( indexName ).removeRelationship(
relId, startNode, endNode );
}
} );
}
catch ( EntityNotFoundException e )
{
// This is a special case which is still OK. This method is called lazily where deleted relationships
// that still are referenced by a legacy index will be added for removal in this transaction.
// Ideally we'd want to include start/end node too, but we can't since the relationship doesn't exist.
// So we do the "normal" remove call on the legacy index transaction changes. The downside is that
// Some queries on this transaction state that include start/end nodes might produce invalid results.
statement.legacyIndexTxState().relationshipChanges( indexName ).remove( relationship );
throw e;
}
}

@Override
Expand Down
6 changes: 3 additions & 3 deletions ...kernel/src/main/java/org/neo4j/kernel/impl/api/operations/LegacyIndexWriteOperations.java
View file
Expand Up @@ -65,13 +65,13 @@ void relationshipAddToLegacyIndex( KernelStatement statement, String indexName,
Object value ) throws EntityNotFoundException, LegacyIndexNotFoundKernelException;

void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship, String key,
Object value ) throws LegacyIndexNotFoundKernelException;
Object value ) throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship, String key )
throws LegacyIndexNotFoundKernelException;
throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void relationshipRemoveFromLegacyIndex( KernelStatement statement, String indexName, long relationship )
throws LegacyIndexNotFoundKernelException;
throws LegacyIndexNotFoundKernelException, EntityNotFoundException;

void nodeLegacyIndexDrop( KernelStatement statement, String indexName ) throws LegacyIndexNotFoundKernelException;

Expand Down
35 changes: 27 additions & 8 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/LegacyIndexProxy.java
View file
Expand Up @@ -171,21 +171,24 @@ void add( DataWriteOperations operations, String name, long id, String key, Obje

@Override
void remove( DataWriteOperations operations, String name, long id, String key, Object value )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException
{
operations.relationshipRemoveFromLegacyIndex( name, id, key, value );
}

@Override
void remove( DataWriteOperations operations, String name, long id, String key )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException
{
operations.relationshipRemoveFromLegacyIndex( name, id, key );
}

@Override
void remove( DataWriteOperations operations, String name, long id )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException
{
operations.relationshipRemoveFromLegacyIndex( name, id );
}
Expand Down Expand Up @@ -223,13 +226,16 @@ abstract void add( DataWriteOperations operations, String name, long id, String
LegacyIndexNotFoundKernelException;

abstract void remove( DataWriteOperations operations, String name, long id, String key, Object value )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException;
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException;

abstract void remove( DataWriteOperations operations, String name, long id, String key )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException;
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException;

abstract void remove( DataWriteOperations operations, String name, long id )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException;
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException,
EntityNotFoundException;

abstract void drop( DataWriteOperations operations, String name )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException;
Expand Down Expand Up @@ -321,7 +327,8 @@ protected T fetchNextOrNull()
{
internalRemove( statement, id );
}
catch ( LegacyIndexNotFoundKernelException | InvalidTransactionTypeKernelException ignore )
catch ( LegacyIndexNotFoundKernelException |
InvalidTransactionTypeKernelException | EntityNotFoundException ignore )
{
// Ignore these failures because we are going to skip the entity anyway
}
Expand Down Expand Up @@ -411,6 +418,10 @@ public void remove( T entity, String key, Object value )
{
throw new NotFoundException( type + " index '" + name + "' doesn't exist" );
}
catch ( EntityNotFoundException e )
{
throw new NotFoundException( entity + " doesn't exist" );
}
}

@Override
Expand All @@ -428,6 +439,10 @@ public void remove( T entity, String key )
{
throw new NotFoundException( type + " index '" + name + "' doesn't exist" );
}
catch ( EntityNotFoundException e )
{
throw new NotFoundException( entity + " doesn't exist" );
}
}

@Override
Expand All @@ -445,10 +460,14 @@ public void remove( T entity )
{
throw new NotFoundException( type + " index '" + name + "' doesn't exist" );
}
catch ( EntityNotFoundException e )
{
throw new NotFoundException( entity + " doesn't exist" );
}
}

private void internalRemove( Statement statement, long id )
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException
throws InvalidTransactionTypeKernelException, LegacyIndexNotFoundKernelException, EntityNotFoundException
{
type.remove( statement.dataWriteOperations(), name, id );
}
Expand Down
19 changes: 18 additions & 1 deletion community/kernel/src/test/java/org/neo4j/kernel/impl/index/DummyIndexImplementation.java
View file
Expand Up @@ -81,7 +81,6 @@ protected boolean fetchNext()

private static class EmptyLegacyIndex implements LegacyIndex
{

private final boolean failing;

private EmptyLegacyIndex( boolean failing )
Expand Down Expand Up @@ -161,6 +160,24 @@ public void addNode( long entity, String key, Object value )
mutate();
}

@Override
public void removeRelationship( long entity, String key, Object value, long startNode, long endNode )
{
mutate();
}

@Override
public void removeRelationship( long entity, String key, long startNode, long endNode )
{
mutate();
}

@Override
public void removeRelationship( long entity, long startNode, long endNode )
{
mutate();
}

private void mutate()
{
if ( failing )
Expand Down
11 changes: 11 additions & 0 deletions community/kernel/src/test/java/org/neo4j/test/DatabaseRule.java
View file
Expand Up @@ -33,6 +33,7 @@
import org.neo4j.graphdb.factory.GraphDatabaseBuilder;
import org.neo4j.graphdb.factory.GraphDatabaseBuilderTestTools;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.graphdb.index.IndexManager;
import org.neo4j.graphdb.schema.Schema;
import org.neo4j.helpers.Function;
import org.neo4j.helpers.Provider;
Expand Down Expand Up @@ -119,6 +120,16 @@ public Node createNode( Label... labels )
return getGraphDatabaseAPI().createNode( labels );
}

public Node getNodeById( long id )
{
return getGraphDatabaseService().getNodeById( id );
}

public IndexManager index()
{
return getGraphDatabaseService().index();
}

public Schema schema()
{
return getGraphDatabaseAPI().schema();
Expand Down
13 changes: 9 additions & 4 deletions community/lucene-index/src/main/java/org/neo4j/index/impl/lucene/ExactTxData.java
View file
Expand Up @@ -37,6 +37,7 @@ public class ExactTxData extends TxData
{
private Map<String, Map<Object, Set<Object>>> data;
private boolean hasOrphans;
private boolean hasRelationshipIds;

ExactTxData( LuceneIndex index )
{
Expand All @@ -47,6 +48,10 @@ public class ExactTxData extends TxData
void add( TxDataHolder holder, Object entityId, String key, Object value )
{
idCollection( key, value, true ).add( entityId );
if ( !hasRelationshipIds && entityId instanceof RelationshipId )
{
hasRelationshipIds = true;
}
}

private Set<Object> idCollection( String key, Object value, boolean create )
Expand Down Expand Up @@ -143,7 +148,7 @@ void remove( TxDataHolder holder, Object entityId, String key, Object value )
{
return;
}

if ( key == null || value == null )
{
TxData fullData = toFullTxData();
Expand Down Expand Up @@ -171,7 +176,7 @@ Collection<Long> get( TxDataHolder holder, String key, Object value )
}
return toLongs( ids );
}

@Override
Collection<Long> getOrphans( String key )
{
Expand All @@ -192,7 +197,7 @@ private Collection<Long> toLongs( Set<Object> ids )
{
if (ids.isEmpty()) return Collections.emptySet();

if ( ids.iterator().next() instanceof Long )
if ( !hasRelationshipIds )
{
return (Collection) ids;
}
Expand All @@ -206,7 +211,7 @@ private Collection<Long> toLongs( Set<Object> ids )
return longs;
}
}

@Override
IndexSearcher asSearcher( TxDataHolder holder, QueryContext context )
{
Expand Down

0 comments on commit 9a868b8

Please sign in to comment.