diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionState.java b/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionState.java index 0cf9c3476e91c..98cbccb3b5cfc 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionState.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/txstate/TransactionState.java @@ -57,11 +57,11 @@ public interface TransactionState extends ReadableTransactionState void graphDoReplaceProperty( int propertyKeyId, Value replacedValue, Value newValue ); - void nodeDoRemoveProperty( long nodeId, int propertyKeyId ); + void nodeDoRemoveProperty( long nodeId, int propertyKeyId, Value removedValue ); - void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId ); + void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId, Value removedValue ); - void graphDoRemoveProperty( int propertyKeyId ); + void graphDoRemoveProperty( int propertyKeyId, Value removedValue ); void nodeDoAddLabel( int labelId, long nodeId ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java index 80aecb6002003..06030ace91c7e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java @@ -1096,7 +1096,7 @@ public Value nodeRemoveProperty( KernelStatement state, long nodeId, int propert existingValue = properties.get().value(); autoIndexing.nodes().propertyRemoved( ops, nodeId, propertyKeyId ); - state.txState().nodeDoRemoveProperty( node.id(), propertyKeyId ); + state.txState().nodeDoRemoveProperty( node.id(), propertyKeyId, existingValue ); indexTxStateUpdater.onPropertyRemove( state, node, propertyKeyId, existingValue ); } @@ -1122,7 +1122,8 @@ public Value relationshipRemoveProperty( KernelStatement state, long relationshi existingValue = properties.get().value(); autoIndexing.relationships().propertyRemoved( ops, relationshipId, propertyKeyId ); - state.txState().relationshipDoRemoveProperty( relationship.id(), propertyKeyId ); + state.txState() + .relationshipDoRemoveProperty( relationship.id(), propertyKeyId, existingValue ); } } return existingValue; @@ -1135,7 +1136,7 @@ public Value graphRemoveProperty( KernelStatement state, int propertyKeyId ) Value existingValue = graphGetProperty( state, propertyKeyId ); if ( existingValue != Values.NO_VALUE ) { - state.txState().graphDoRemoveProperty( propertyKeyId ); + state.txState().graphDoRemoveProperty( propertyKeyId, existingValue ); } return existingValue; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java index 85e2411e267f5..c72d09bbeb371 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java @@ -19,6 +19,7 @@ */ package org.neo4j.kernel.impl.api.state; +import java.util.Collections; import java.util.Iterator; import java.util.function.Predicate; @@ -32,13 +33,13 @@ import static java.util.Collections.emptyIterator; -class PropertyContainerStateImpl implements PropertyContainerState +public class PropertyContainerStateImpl implements PropertyContainerState { private final long id; private VersionedHashMap addedProperties; private VersionedHashMap changedProperties; - private VersionedHashMap removedProperties; + private VersionedHashMap removedProperties; private final Predicate excludePropertiesWeKnowAbout = new Predicate() { @@ -61,7 +62,7 @@ public long getId() return id; } - void clear() + public void clear() { if ( changedProperties != null ) { @@ -79,10 +80,13 @@ void clear() void changeProperty( int propertyKeyId, Value value ) { - if ( addedProperties != null && addedProperties.containsKey( propertyKeyId ) ) + if ( addedProperties != null ) { - addedProperties.put( propertyKeyId, value ); - return; + if ( addedProperties.containsKey( propertyKeyId ) ) + { + addedProperties.put( propertyKeyId, value ); + return; + } } if ( changedProperties == null ) @@ -99,12 +103,16 @@ void changeProperty( int propertyKeyId, Value value ) void addProperty( int propertyKeyId, Value value ) { - if ( removedProperties != null && removedProperties.remove( propertyKeyId ) != null ) + if ( removedProperties != null ) { - // This indicates the user did remove+add as two discrete steps, which should be translated to - // a single change operation. - changeProperty( propertyKeyId, value ); - return; + Value removed = removedProperties.remove( propertyKeyId ); + if ( removed != null ) + { + // This indicates the user did remove+add as two discrete steps, which should be translated to + // a single change operation. + changeProperty( propertyKeyId, value ); + return; + } } if ( addedProperties == null ) { @@ -113,17 +121,20 @@ void addProperty( int propertyKeyId, Value value ) addedProperties.put( propertyKeyId, value ); } - void removeProperty( int propertyKeyId ) + public void removeProperty( int propertyKeyId, Value value ) { - if ( addedProperties != null && addedProperties.remove( propertyKeyId ) != null ) + if ( addedProperties != null ) { - return; + if ( addedProperties.remove( propertyKeyId ) != null ) + { + return; + } } if ( removedProperties == null ) { removedProperties = new VersionedHashMap<>(); } - removedProperties.put( propertyKeyId, Boolean.TRUE ); + removedProperties.put( propertyKeyId, value ); if ( changedProperties != null ) { changedProperties.remove( propertyKeyId ); @@ -224,8 +235,8 @@ public boolean isPropertyRemoved( int propertyKeyId ) private Iterator toPropertyIterator( VersionedHashMap propertyMap ) { - return propertyMap == null ? emptyIterator() : - Iterators.map( + return propertyMap == null ? Collections.emptyIterator() : + Iterators.map( entry -> new PropertyKeyValue( entry.getKey(), entry.getValue() ), propertyMap.entrySet().iterator() ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java index be29823bee3ad..72ce08627f1d8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/TxState.java @@ -569,23 +569,23 @@ public void graphDoReplaceProperty( int propertyKeyId, Value replacedValue, Valu } @Override - public void nodeDoRemoveProperty( long nodeId, int propertyKeyId ) + public void nodeDoRemoveProperty( long nodeId, int propertyKeyId, Value removedValue ) { - getOrCreateNodeState( nodeId ).removeProperty( propertyKeyId ); + getOrCreateNodeState( nodeId ).removeProperty( propertyKeyId, removedValue ); dataChanged(); } @Override - public void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId ) + public void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId, Value removedValue ) { - getOrCreateRelationshipState( relationshipId ).removeProperty( propertyKeyId ); + getOrCreateRelationshipState( relationshipId ).removeProperty( propertyKeyId, removedValue ); dataChanged(); } @Override - public void graphDoRemoveProperty( int propertyKeyId ) + public void graphDoRemoveProperty( int propertyKeyId, Value removedValue ) { - getOrCreateGraphState().removeProperty( propertyKeyId ); + getOrCreateGraphState().removeProperty( propertyKeyId, removedValue ); dataChanged(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java index 4500be5ff941b..99b8bfc83df37 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/Operations.java @@ -483,7 +483,7 @@ public Value nodeRemoveProperty( long node, int propertyKey ) if ( existingValue != NO_VALUE ) { autoIndexing.nodes().propertyRemoved( this, node, propertyKey ); - ktx.txState().nodeDoRemoveProperty( node, propertyKey ); + ktx.txState().nodeDoRemoveProperty( node, propertyKey, existingValue ); updater.onPropertyRemove( nodeCursor, propertyCursor, propertyKey, existingValue ); } @@ -529,7 +529,7 @@ public Value relationshipRemoveProperty( long relationship, int propertyKey ) if ( existingValue != NO_VALUE ) { autoIndexing.relationships().propertyRemoved( this, relationship, propertyKey ); - ktx.txState().relationshipDoRemoveProperty( relationship, propertyKey ); + ktx.txState().relationshipDoRemoveProperty( relationship, propertyKey, existingValue ); } return existingValue; @@ -559,7 +559,7 @@ public Value graphRemoveProperty( int propertyKey ) Value existingValue = readGraphProperty( propertyKey ); if ( existingValue != Values.NO_VALUE ) { - ktx.txState().graphDoRemoveProperty( propertyKey ); + ktx.txState().graphDoRemoveProperty( propertyKey, existingValue ); } return existingValue; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImplTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImplTest.java index 0f1c607c54f5e..acf7982cb8795 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImplTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImplTest.java @@ -42,7 +42,7 @@ public void shouldListAddedProperties() PropertyContainerStateImpl state = new PropertyContainerStateImpl( 1 ); state.addProperty( 1, Values.of( "Hello" ) ); state.addProperty( 2, Values.of( "Hello" ) ); - state.removeProperty( 1 ); + state.removeProperty( 1, Values.of( "Hello" ) ); // When Iterator added = state.addedProperties(); @@ -79,7 +79,7 @@ public void shouldConvertAddRemoveToChange() PropertyContainerStateImpl state = new PropertyContainerStateImpl( 1 ); // When - state.removeProperty( 4 ); + state.removeProperty( 4, Values.of( "a value" ) ); state.addProperty( 4, Values.of( "another value" ) ); // Then diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java index 8a3e80e6eb166..8d135b82cd900 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/coreapi/TxStateTransactionDataViewTest.java @@ -225,7 +225,7 @@ public void shouldListRemovedNodeProperties() throws Exception // Given int propertyKeyId = 1; Value prevValue = Values.of( "prevValue" ); - state.nodeDoRemoveProperty( 1L, propertyKeyId ); + state.nodeDoRemoveProperty( 1L, propertyKeyId, prevValue ); when( ops.propertyKeyGetName( propertyKeyId ) ).thenReturn( "theKey" ); long propertyId = 20L; when( storeStatement.acquireSingleNodeCursor( 1L ) ).thenReturn( @@ -250,7 +250,7 @@ public void shouldListRemovedRelationshipProperties() throws Exception // Given int propertyKeyId = 1; Value prevValue = Values.of( "prevValue" ); - state.relationshipDoRemoveProperty( 1L, propertyKeyId ); + state.relationshipDoRemoveProperty( 1L, propertyKeyId, prevValue ); when( ops.propertyKeyGetName( propertyKeyId ) ).thenReturn( "theKey" ); long propertyId = 40L; when( storeStatement.acquireSingleRelationshipCursor( 1 ) ) diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java index 83ad65ebe6a58..3f23ec3d13dc0 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/store/NeoStoresTest.java @@ -503,7 +503,7 @@ public void testProps1() throws Exception initializeStores( storeDir, stringMap() ); startTx(); StorageProperty prop2 = nodeAddProperty( nodeId, prop.propertyKeyId(), 5 ); - transaction.nodeDoRemoveProperty( nodeId, prop2.propertyKeyId() ); + transaction.nodeDoRemoveProperty( nodeId, prop2.propertyKeyId(), prop2.value() ); transaction.nodeDoDelete( nodeId ); commitTx(); ds.stop(); @@ -1328,7 +1328,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) assertEquals( "prop3", MyPropertyKeyToken.getIndexFor( keyId ).name() ); assertEquals( false, data.value().asObject() ); - transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId() ); + transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId(), prop3.value() ); } else { @@ -1389,7 +1389,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) assertEquals( "prop3", MyPropertyKeyToken.getIndexFor( keyId ).name() ); assertEquals( true, data.value().asObject() ); - transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId() ); + transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId(), prop3.value() ); } else { @@ -1455,7 +1455,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) assertEquals( "prop3", MyPropertyKeyToken.getIndexFor( keyId ).name() ); assertEquals( false, data.value().asObject() ); - transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId() ); + transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId(), prop3.value() ); } else { @@ -1501,7 +1501,7 @@ else if ( data.propertyKeyId() == prop3.propertyKeyId() ) assertEquals( "prop3", MyPropertyKeyToken.getIndexFor( keyId ).name() ); assertEquals( true, data.value().asObject() ); - transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId() ); + transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId(), prop3.value() ); } else {