diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java index ffcd1dee23390..da211295ac3ee 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java @@ -75,10 +75,10 @@ void nodeIndexSeek( IndexReference index, NodeValueIndexCursor cursor, IndexOrde /** * Checks if a node exists in the database - * @param id The id of the node to check + * @param reference The reference of the node to check * @return true if the node exists, otherwise false */ - boolean nodeExists( long id ); + boolean nodeExists( long reference ); /** * @param reference diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeTransactionStateTestBase.java similarity index 99% rename from community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java rename to community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeTransactionStateTestBase.java index d94462005aa50..df6c26fc8fed1 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/NodeTransactionStateTestBase.java @@ -37,7 +37,7 @@ import static org.neo4j.values.storable.Values.stringValue; @SuppressWarnings( "Duplicates" ) -public abstract class TransactionStateTestBase extends KernelAPIWriteTestBase +public abstract class NodeTransactionStateTestBase extends KernelAPIWriteTestBase { @Test public void shouldSeeNodeInTransaction() throws Exception diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java new file mode 100644 index 0000000000000..fbcf24a792fa1 --- /dev/null +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/RelationshipTransactionStateTestBase.java @@ -0,0 +1,65 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.internal.kernel.api; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings( "Duplicates" ) +public abstract class RelationshipTransactionStateTestBase extends KernelAPIWriteTestBase +{ + @Test + public void shouldSeeSingleRelationshipInTransaction() throws Exception + { + int label; + long n1, n2; + try ( Transaction tx = session.beginTransaction() ) + { + n1 = tx.dataWrite().nodeCreate(); + n2 = tx.dataWrite().nodeCreate(); + long decoyNode = tx.dataWrite().nodeCreate(); + label = tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ); // to have >1 relationship in the db + tx.dataWrite().relationshipCreate( n2, label, decoyNode ); + tx.success(); + } + + try ( Transaction tx = session.beginTransaction() ) + { + label = tx.tokenWrite().relationshipTypeGetOrCreateForName( "R" ); + long r = tx.dataWrite().relationshipCreate( n1, label, n2 ); + try ( RelationshipScanCursor relationship = cursors.allocateRelationshipScanCursor() ) + { + tx.dataRead().singleRelationship( r, relationship ); + assertTrue( "should find relationship", relationship.next() ); + + assertEquals( label, relationship.label() ); + assertEquals( n1, relationship.sourceNodeReference() ); + assertEquals( n2, relationship.targetNodeReference() ); + assertEquals( r, relationship.relationshipReference() ); + + assertFalse( "should only find one relationship", relationship.next() ); + } + tx.success(); + } + } +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java index fe98d1863bf89..a1022ff8071fc 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java @@ -95,23 +95,23 @@ public AllStoreHolder( StorageEngine engine, } @Override - public boolean nodeExists( long id ) + public boolean nodeExists( long reference ) { ktx.assertOpen(); if ( hasTxStateWithChanges() ) { TransactionState txState = txState(); - if ( txState.nodeIsDeletedInThisTx( id ) ) + if ( txState.nodeIsDeletedInThisTx( reference ) ) { return false; } - else if ( txState.nodeIsAddedInThisTx( id ) ) + else if ( txState.nodeIsAddedInThisTx( reference ) ) { return true; } } - return storeReadLayer.nodeExists( id ); + return storeReadLayer.nodeExists( reference ); } @Override diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/HasChanges.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/HasChanges.java new file mode 100644 index 0000000000000..690afa1bdd3d7 --- /dev/null +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/HasChanges.java @@ -0,0 +1,27 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.kernel.impl.newapi; + +enum HasChanges +{ + MAYBE, + YES, + NO +} diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeCursor.java index e2357581ca4c0..7cbe3b3a4bb8f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/NodeCursor.java @@ -48,13 +48,6 @@ class NodeCursor extends NodeRecord implements org.neo4j.internal.kernel.api.Nod private HasChanges hasChanges = HasChanges.MAYBE; private Set addedNodes; - private enum HasChanges - { - MAYBE, - YES, - NO - } - NodeCursor() { super( NO_ID ); @@ -203,6 +196,7 @@ else if ( hasChanges && txs.nodeIsDeletedInThisTx( next ) ) { read.node( this, next++, pageCursor ); } + if ( next > highMark ) { if ( isSingle() ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipCursor.java index 490df64b92510..31786538058ce 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipCursor.java @@ -19,18 +19,32 @@ */ package org.neo4j.kernel.impl.newapi; +import java.util.Set; + import org.neo4j.internal.kernel.api.RelationshipDataAccessor; +import org.neo4j.kernel.impl.api.RelationshipVisitor; import org.neo4j.kernel.impl.store.record.RelationshipRecord; -abstract class RelationshipCursor extends RelationshipRecord implements RelationshipDataAccessor +import static java.util.Collections.emptySet; + +abstract class RelationshipCursor extends RelationshipRecord implements RelationshipDataAccessor, RelationshipVisitor { Read read; + private HasChanges hasChanges = HasChanges.MAYBE; + Set addedRelationships; RelationshipCursor() { super( NO_ID ); } + protected void init( Read read ) + { + this.read = read; + this.hasChanges = HasChanges.MAYBE; + this.addedRelationships = emptySet(); + } + @Override public long relationshipReference() { @@ -84,4 +98,52 @@ public long propertiesReference() { return getNextProp(); } + + protected abstract boolean shouldGetAddedTxStateSnapshot(); + + /** + * RelationshipCursor should only see changes that are there from the beginning + * otherwise it will not be stable. + */ + protected boolean hasChanges() + { + switch ( hasChanges ) + { + case MAYBE: + boolean changes = read.hasTxStateWithChanges(); + if ( changes ) + { + if ( shouldGetAddedTxStateSnapshot() ) + { + addedRelationships = read.txState().addedAndRemovedRelationships().getAddedSnapshot(); + } + hasChanges = HasChanges.YES; + } + else + { + hasChanges = HasChanges.NO; + } + return changes; + case YES: + return true; + case NO: + return false; + default: + throw new IllegalStateException( "Style guide, why are you making me do this" ); + } + } + + protected void loadFromTxState( long reference ) + { + read.txState().relationshipVisit( reference, this ); + } + + @Override + public void visit( long relationshipId, int typeId, long startNodeId, long endNodeId ) throws RuntimeException + { + setId( relationshipId ); + setType( typeId ); + setFirstNode( startNodeId ); + setSecondNode( endNodeId ); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipScanCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipScanCursor.java index ee2f4a2b0f2dd..8d89d61c893ea 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipScanCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipScanCursor.java @@ -20,6 +20,7 @@ package org.neo4j.kernel.impl.newapi; import org.neo4j.io.pagecache.PageCursor; +import org.neo4j.kernel.api.txstate.TransactionState; class RelationshipScanCursor extends RelationshipCursor implements org.neo4j.internal.kernel.api.RelationshipScanCursor { @@ -41,7 +42,7 @@ void scan( int label, Read read ) next = 0; this.label = label; highMark = read.relationshipHighMark(); - this.read = read; + init( read ); } void single( long reference, Read read ) @@ -57,46 +58,72 @@ void single( long reference, Read read ) next = reference; label = -1; highMark = NO_ID; - this.read = read; + init( read ); } @Override public boolean next() { + if ( next == NO_ID ) + { + reset(); + return false; + } + + // Check tx state + boolean hasChanges = hasChanges(); + TransactionState txs = hasChanges ? read.txState() : null; + do { - if ( next == NO_ID ) + if ( hasChanges && containsRelationship( txs ) ) + { + loadFromTxState( next++ ); + setInUse( true ); + } + else if ( hasChanges && txs.relationshipIsDeletedInThisTx( next ) ) { - reset(); - return false; + next++; + setInUse( false ); } - do + else { read.relationship( this, next++, pageCursor ); - if ( next > highMark ) + } + + if ( next > highMark ) + { + if ( isSingle() ) { - if ( highMark == NO_ID ) + next = NO_ID; + return isWantedLabelAndInUse(); + } + else + { + highMark = read.relationshipHighMark(); + if ( next > highMark ) { next = NO_ID; - return (label == -1 || label() == label) && inUse(); - } - else - { - highMark = read.relationshipHighMark(); - if ( next > highMark ) - { - next = NO_ID; - return (label == -1 || label() == label) && inUse(); - } + return isWantedLabelAndInUse(); } } } - while ( !inUse() ); } - while ( label != -1 && label() != label ); + while ( !isWantedLabelAndInUse() ); + return true; } + private boolean isWantedLabelAndInUse() + { + return (label == -1 || label() == label) && inUse(); + } + + private boolean containsRelationship( TransactionState txs ) + { + return isSingle() ? txs.relationshipIsAddedInThisTx( next ) : addedRelationships.contains( next ); + } + @Override public boolean shouldRetry() { @@ -139,4 +166,14 @@ public String toString() ", underlying record=" + super.toString() + " ]"; } } + + private boolean isSingle() + { + return highMark == NO_ID; + } + + protected boolean shouldGetAddedTxStateSnapshot() + { + return !isSingle(); + } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipTraversalCursor.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipTraversalCursor.java index ed111e40e912f..157617c78bb48 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipTraversalCursor.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/RelationshipTraversalCursor.java @@ -438,6 +438,12 @@ private void reset() buffer = null; } + @Override + protected boolean shouldGetAddedTxStateSnapshot() + { + return true; + } + @Override public boolean isClosed() { diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/TransactionStateTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/NodeTransactionStateTest.java similarity index 85% rename from community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/TransactionStateTest.java rename to community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/NodeTransactionStateTest.java index 7b971578e9bbb..26320310e2bbf 100644 --- a/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/TransactionStateTest.java +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/NodeTransactionStateTest.java @@ -19,9 +19,9 @@ */ package org.neo4j.kernel.impl.newapi; -import org.neo4j.internal.kernel.api.TransactionStateTestBase; +import org.neo4j.internal.kernel.api.NodeTransactionStateTestBase; -public class TransactionStateTest extends TransactionStateTestBase +public class NodeTransactionStateTest extends NodeTransactionStateTestBase { @Override public WriteTestSupport newTestSupport() diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/RelationshipTransactionStateTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/RelationshipTransactionStateTest.java new file mode 100644 index 0000000000000..4796d1c3944f7 --- /dev/null +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/impl/newapi/RelationshipTransactionStateTest.java @@ -0,0 +1,31 @@ +/* + * Copyright (c) 2002-2018 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.kernel.impl.newapi; + +import org.neo4j.internal.kernel.api.RelationshipTransactionStateTestBase; + +public class RelationshipTransactionStateTest extends RelationshipTransactionStateTestBase +{ + @Override + public WriteTestSupport newTestSupport() + { + return new WriteTestSupport(); + } +}