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 98cbccb3b5cfc..0cf9c3476e91c 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, Value removedValue ); + void nodeDoRemoveProperty( long nodeId, int propertyKeyId ); - void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId, Value removedValue ); + void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId ); - void graphDoRemoveProperty( int propertyKeyId, Value removedValue ); + void graphDoRemoveProperty( int propertyKeyId ); 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 06030ace91c7e..80aecb6002003 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, existingValue ); + state.txState().nodeDoRemoveProperty( node.id(), propertyKeyId ); indexTxStateUpdater.onPropertyRemove( state, node, propertyKeyId, existingValue ); } @@ -1122,8 +1122,7 @@ public Value relationshipRemoveProperty( KernelStatement state, long relationshi existingValue = properties.get().value(); autoIndexing.relationships().propertyRemoved( ops, relationshipId, propertyKeyId ); - state.txState() - .relationshipDoRemoveProperty( relationship.id(), propertyKeyId, existingValue ); + state.txState().relationshipDoRemoveProperty( relationship.id(), propertyKeyId ); } } return existingValue; @@ -1136,7 +1135,7 @@ public Value graphRemoveProperty( KernelStatement state, int propertyKeyId ) Value existingValue = graphGetProperty( state, propertyKeyId ); if ( existingValue != Values.NO_VALUE ) { - state.txState().graphDoRemoveProperty( propertyKeyId, existingValue ); + state.txState().graphDoRemoveProperty( propertyKeyId ); } 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 c72d09bbeb371..85e2411e267f5 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,7 +19,6 @@ */ package org.neo4j.kernel.impl.api.state; -import java.util.Collections; import java.util.Iterator; import java.util.function.Predicate; @@ -33,13 +32,13 @@ import static java.util.Collections.emptyIterator; -public class PropertyContainerStateImpl implements PropertyContainerState +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() { @@ -62,7 +61,7 @@ public long getId() return id; } - public void clear() + void clear() { if ( changedProperties != null ) { @@ -80,13 +79,10 @@ public void clear() void changeProperty( int propertyKeyId, Value value ) { - if ( addedProperties != null ) + if ( addedProperties != null && addedProperties.containsKey( propertyKeyId ) ) { - if ( addedProperties.containsKey( propertyKeyId ) ) - { - addedProperties.put( propertyKeyId, value ); - return; - } + addedProperties.put( propertyKeyId, value ); + return; } if ( changedProperties == null ) @@ -103,16 +99,12 @@ void changeProperty( int propertyKeyId, Value value ) void addProperty( int propertyKeyId, Value value ) { - if ( removedProperties != null ) + if ( removedProperties != null && removedProperties.remove( propertyKeyId ) != null ) { - 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; - } + // 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 ) { @@ -121,20 +113,17 @@ void addProperty( int propertyKeyId, Value value ) addedProperties.put( propertyKeyId, value ); } - public void removeProperty( int propertyKeyId, Value value ) + void removeProperty( int propertyKeyId ) { - if ( addedProperties != null ) + if ( addedProperties != null && addedProperties.remove( propertyKeyId ) != null ) { - if ( addedProperties.remove( propertyKeyId ) != null ) - { - return; - } + return; } if ( removedProperties == null ) { removedProperties = new VersionedHashMap<>(); } - removedProperties.put( propertyKeyId, value ); + removedProperties.put( propertyKeyId, Boolean.TRUE ); if ( changedProperties != null ) { changedProperties.remove( propertyKeyId ); @@ -235,8 +224,8 @@ public boolean isPropertyRemoved( int propertyKeyId ) private Iterator toPropertyIterator( VersionedHashMap propertyMap ) { - return propertyMap == null ? Collections.emptyIterator() : - Iterators.map( + return propertyMap == null ? 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 72ce08627f1d8..be29823bee3ad 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, Value removedValue ) + public void nodeDoRemoveProperty( long nodeId, int propertyKeyId ) { - getOrCreateNodeState( nodeId ).removeProperty( propertyKeyId, removedValue ); + getOrCreateNodeState( nodeId ).removeProperty( propertyKeyId ); dataChanged(); } @Override - public void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId, Value removedValue ) + public void relationshipDoRemoveProperty( long relationshipId, int propertyKeyId ) { - getOrCreateRelationshipState( relationshipId ).removeProperty( propertyKeyId, removedValue ); + getOrCreateRelationshipState( relationshipId ).removeProperty( propertyKeyId ); dataChanged(); } @Override - public void graphDoRemoveProperty( int propertyKeyId, Value removedValue ) + public void graphDoRemoveProperty( int propertyKeyId ) { - getOrCreateGraphState().removeProperty( propertyKeyId, removedValue ); + getOrCreateGraphState().removeProperty( propertyKeyId ); 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 99b8bfc83df37..4500be5ff941b 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, existingValue ); + ktx.txState().nodeDoRemoveProperty( node, propertyKey ); 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, existingValue ); + ktx.txState().relationshipDoRemoveProperty( relationship, propertyKey ); } return existingValue; @@ -559,7 +559,7 @@ public Value graphRemoveProperty( int propertyKey ) Value existingValue = readGraphProperty( propertyKey ); if ( existingValue != Values.NO_VALUE ) { - ktx.txState().graphDoRemoveProperty( propertyKey, existingValue ); + ktx.txState().graphDoRemoveProperty( propertyKey ); } 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 acf7982cb8795..0f1c607c54f5e 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, Values.of( "Hello" ) ); + state.removeProperty( 1 ); // When Iterator added = state.addedProperties(); @@ -79,7 +79,7 @@ public void shouldConvertAddRemoveToChange() PropertyContainerStateImpl state = new PropertyContainerStateImpl( 1 ); // When - state.removeProperty( 4, Values.of( "a value" ) ); + state.removeProperty( 4 ); 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 8d135b82cd900..8a3e80e6eb166 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, prevValue ); + state.nodeDoRemoveProperty( 1L, propertyKeyId ); 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, prevValue ); + state.relationshipDoRemoveProperty( 1L, propertyKeyId ); 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 3f23ec3d13dc0..83ad65ebe6a58 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(), prop2.value() ); + transaction.nodeDoRemoveProperty( nodeId, prop2.propertyKeyId() ); 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(), prop3.value() ); + transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId() ); } 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(), prop3.value() ); + transaction.relationshipDoRemoveProperty( rel, prop3.propertyKeyId() ); } 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(), prop3.value() ); + transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId() ); } 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(), prop3.value() ); + transaction.nodeDoRemoveProperty( node, prop3.propertyKeyId() ); } else {