Overwriting property with same value no longer generates write command

It seems obvious, but setting a node/relationship property to the same value
as it already is doesn't require any change to that node/relationship.
Even so this was the case previously. A command would be generated, property
record would be updates on apply, indexing changes would be applied...
all for nothing.

community/kernel/src/main/java/org/neo4j/kernel/impl/api/StateHandlingStatementOperations.java

@@ -60,6 +60,7 @@
 import org.neo4j.kernel.api.exceptions.schema.TooManyLabelsException;
 import org.neo4j.kernel.api.index.IndexDescriptor;
 import org.neo4j.kernel.api.index.InternalIndexState;
+import org.neo4j.kernel.api.legacyindex.AutoIndexOperations;
 import org.neo4j.kernel.api.legacyindex.AutoIndexing;
 import org.neo4j.kernel.api.properties.DefinedProperty;
 import org.neo4j.kernel.api.properties.Property;
@@ -80,6 +81,7 @@
 import org.neo4j.kernel.impl.index.LegacyIndexStore;
 import org.neo4j.kernel.impl.util.Cursors;
 import org.neo4j.register.Register.DoubleLongRegister;
+import org.neo4j.storageengine.api.EntityItem;
 import org.neo4j.storageengine.api.EntityType;
 import org.neo4j.storageengine.api.LabelItem;
 import org.neo4j.storageengine.api.NodeItem;
@@ -960,30 +962,45 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp
         try ( Cursor<NodeItem> cursor = nodeCursorById( state, nodeId ) )
         {
             NodeItem node = cursor.get();
-            Property existingProperty;
-            try ( Cursor<PropertyItem> properties = node.property( property.propertyKeyId() ) )
+            Property existingProperty = readProperty( property.propertyKeyId(), node, EntityType.NODE );
+
+            if ( !property.equals( existingProperty ) )
             {
-                if ( !properties.next() )
-                {
-                    autoIndexing.nodes().propertyAdded( ops, nodeId, property );
-                    existingProperty = Property.noProperty( property.propertyKeyId(), EntityType.NODE, node.id() );
-                }
-                else
-                {
-                    existingProperty = Property.property( properties.get().propertyKeyId(), properties.get().value() );
-                    autoIndexing.nodes().propertyChanged( ops, nodeId, existingProperty, property );
-                }
-            }
+                autoIndexProperty( nodeId, property, ops, existingProperty, autoIndexing.nodes() );
 
-            state.txState().nodeDoReplaceProperty( node.id(), existingProperty, property );
+                state.txState().nodeDoReplaceProperty( node.id(), existingProperty, property );
 
-            DefinedProperty before = definedPropertyOrNull( existingProperty );
-            indexesUpdateProperty( state, node, property.propertyKeyId(), before, property );
+                DefinedProperty before = definedPropertyOrNull( existingProperty );
+                indexesUpdateProperty( state, node, property.propertyKeyId(), before, property );
+            }
 
             return existingProperty;
         }
     }
 
+    private void autoIndexProperty( long entityId, DefinedProperty property, DataWriteOperations ops,
+            Property existingProperty, AutoIndexOperations indexing ) throws AutoIndexingKernelException
+    {
+        if ( existingProperty.isDefined() )
+        {
+            indexing.propertyChanged( ops, entityId, existingProperty, property );
+        }
+        else
+        {
+            indexing.propertyAdded( ops, entityId, property );
+        }
+    }
+
+    private Property readProperty( int propertyKeyId, EntityItem entity, EntityType entityType )
+    {
+        try ( Cursor<PropertyItem> properties = entity.property( propertyKeyId ) )
+        {
+            return properties.next()
+                    ? Property.property( properties.get().propertyKeyId(), properties.get().value() )
+                    : Property.noProperty( propertyKeyId, entityType, entity.id() );
+        }
+    }
+
     @Override
     public Property relationshipSetProperty( KernelStatement state,
             long relationshipId,
@@ -994,23 +1011,13 @@ public Property relationshipSetProperty( KernelStatement state,
         try ( Cursor<RelationshipItem> cursor = relationshipCursorById( state, relationshipId ) )
         {
             RelationshipItem relationship = cursor.get();
-            Property existingProperty;
-            try ( Cursor<PropertyItem> properties = relationship.property( property.propertyKeyId() ) )
+            Property existingProperty = readProperty( property.propertyKeyId(), relationship, EntityType.RELATIONSHIP );
+
+            if ( !property.equals( existingProperty ) )
             {
-                if ( !properties.next() )
-                {
-                    autoIndexing.relationships().propertyAdded( ops, relationshipId, property );
-                    existingProperty = Property.noProperty( property.propertyKeyId(), EntityType.RELATIONSHIP,
-                            relationship.id() );
-                }
-                else
-                {
-                    existingProperty = Property.property( properties.get().propertyKeyId(), properties.get().value() );
-                    autoIndexing.relationships().propertyChanged( ops, relationshipId, existingProperty, property );
-                }
+                autoIndexProperty( relationshipId, property, ops, existingProperty, autoIndexing.relationships() );
+                state.txState().relationshipDoReplaceProperty( relationship.id(), existingProperty, property );
             }
-
-            state.txState().relationshipDoReplaceProperty( relationship.id(), existingProperty, property );
             return existingProperty;
         }
     }
@@ -1034,22 +1041,13 @@ public Property nodeRemoveProperty( KernelStatement state, long nodeId, int prop
         try ( Cursor<NodeItem> cursor = nodeCursorById( state, nodeId ) )
         {
             NodeItem node = cursor.get();
-            Property existingProperty;
-            try ( Cursor<PropertyItem> properties = node.property( propertyKeyId ) )
+            Property existingProperty = readProperty( propertyKeyId, node, EntityType.NODE );
+            if ( existingProperty.isDefined() )
             {
-                if ( !properties.next() )
-                {
-                    existingProperty = Property.noProperty( propertyKeyId, EntityType.NODE, node.id() );
-                }
-                else
-                {
-                    existingProperty = Property.property( properties.get().propertyKeyId(), properties.get().value() );
-
-                    autoIndexing.nodes().propertyRemoved( ops, nodeId, propertyKeyId );
-                    state.txState().nodeDoRemoveProperty( node.id(), (DefinedProperty) existingProperty );
+                autoIndexing.nodes().propertyRemoved( ops, nodeId, propertyKeyId );
+                state.txState().nodeDoRemoveProperty( node.id(), (DefinedProperty) existingProperty );
 
-                    indexesUpdateProperty( state, node, propertyKeyId, (DefinedProperty) existingProperty, null );
-                }
+                indexesUpdateProperty( state, node, propertyKeyId, (DefinedProperty) existingProperty, null );
             }
             return existingProperty;
         }
@@ -1065,21 +1063,12 @@ public Property relationshipRemoveProperty( KernelStatement state,
         try ( Cursor<RelationshipItem> cursor = relationshipCursorById( state, relationshipId ) )
         {
             RelationshipItem relationship = cursor.get();
-            Property existingProperty;
-            try ( Cursor<PropertyItem> properties = relationship.property( propertyKeyId ) )
+            Property existingProperty = readProperty( propertyKeyId, relationship, EntityType.RELATIONSHIP );
+            if ( existingProperty.isDefined() )
             {
-                if ( !properties.next() )
-                {
-                    existingProperty = Property.noProperty( propertyKeyId, EntityType.RELATIONSHIP, relationship.id() );
-                }
-                else
-                {
-                    existingProperty = Property.property( properties.get().propertyKeyId(), properties.get().value() );
-
-                    autoIndexing.relationships().propertyRemoved( ops, relationshipId, propertyKeyId );
-                    state.txState().relationshipDoRemoveProperty( relationship.id(),
-                            (DefinedProperty) existingProperty );
-                }
+                autoIndexing.relationships().propertyRemoved( ops, relationshipId, propertyKeyId );
+                state.txState().relationshipDoRemoveProperty( relationship.id(),
+                        (DefinedProperty) existingProperty );
             }
             return existingProperty;
         }

community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/StateHandlingStatementOperationsTest.java

@@ -36,6 +36,7 @@
 import org.neo4j.kernel.api.constraints.UniquenessConstraint;
 import org.neo4j.kernel.api.exceptions.index.IndexNotFoundKernelException;
 import org.neo4j.kernel.api.index.IndexDescriptor;
+import org.neo4j.kernel.api.properties.Property;
 import org.neo4j.kernel.api.txstate.TransactionState;
 import org.neo4j.kernel.impl.api.IndexReaderFactory;
 import org.neo4j.kernel.impl.api.KernelStatement;
@@ -48,12 +49,15 @@
 import org.neo4j.kernel.impl.util.diffsets.DiffSets;
 import org.neo4j.storageengine.api.LabelItem;
 import org.neo4j.storageengine.api.NodeItem;
+import org.neo4j.storageengine.api.PropertyItem;
+import org.neo4j.storageengine.api.RelationshipItem;
 import org.neo4j.storageengine.api.StorageStatement;
 import org.neo4j.storageengine.api.StoreReadLayer;
 import org.neo4j.storageengine.api.schema.IndexReader;
 import org.neo4j.storageengine.api.schema.LabelScanReader;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
@@ -422,6 +426,74 @@ public void nodeCursorGetFromUniqueIndexSeekClosesIndexReader() throws Exception
         verify( indexReader ).close();
     }
 
+    @Test
+    public void shouldNotRecordNodeSetPropertyOnSameValue() throws Exception
+    {
+        // GIVEN
+        int propertyKeyId = 5;
+        long nodeId = 0;
+        String value = "The value";
+        KernelStatement kernelStatement = mock( KernelStatement.class );
+        StoreStatement storeStatement = mock( StoreStatement.class );
+        Cursor<NodeItem> ourNode = nodeCursorWithProperty( propertyKeyId, value );
+        when( storeStatement.acquireSingleNodeCursor( nodeId ) ).thenReturn( ourNode );
+        when( kernelStatement.getStoreStatement() ).thenReturn( storeStatement );
+        StateHandlingStatementOperations operations = newTxStateOps( mock( StoreReadLayer.class ) );
+
+        // WHEN
+        operations.nodeSetProperty( kernelStatement, nodeId, Property.stringProperty( propertyKeyId, value ) );
+
+        // THEN
+        assertFalse( kernelStatement.hasTxStateWithChanges() );
+    }
+
+    @Test
+    public void shouldNotRecordRelationshipSetPropertyOnSameValue() throws Exception
+    {
+        // GIVEN
+        int propertyKeyId = 5;
+        long relationshipId = 0;
+        String value = "The value";
+        KernelStatement kernelStatement = mock( KernelStatement.class );
+        StoreStatement storeStatement = mock( StoreStatement.class );
+        Cursor<RelationshipItem> ourRelationship = relationshipCursorWithProperty( propertyKeyId, value );
+        when( storeStatement.acquireSingleRelationshipCursor( relationshipId ) ).thenReturn( ourRelationship );
+        when( kernelStatement.getStoreStatement() ).thenReturn( storeStatement );
+        StateHandlingStatementOperations operations = newTxStateOps( mock( StoreReadLayer.class ) );
+
+        // WHEN
+        operations.relationshipSetProperty( kernelStatement, relationshipId,
+                Property.stringProperty( propertyKeyId, value ) );
+
+        // THEN
+        assertFalse( kernelStatement.hasTxStateWithChanges() );
+    }
+
+    private Cursor<NodeItem> nodeCursorWithProperty( int propertyKeyId, String value )
+    {
+        Cursor<PropertyItem> propertyCursor = propertyCursor( propertyKeyId, value );
+        NodeItem item = mock( NodeItem.class );
+        when( item.property( propertyKeyId ) ).thenReturn( propertyCursor );
+        return Cursors.cursor( item );
+    }
+
+    private Cursor<RelationshipItem> relationshipCursorWithProperty( int propertyKeyId, String value )
+    {
+        Cursor<PropertyItem> propertyCursor = propertyCursor( propertyKeyId, value );
+        RelationshipItem item = mock( RelationshipItem.class );
+        when( item.property( propertyKeyId ) ).thenReturn( propertyCursor );
+        return Cursors.cursor( item );
+    }
+
+    private Cursor<PropertyItem> propertyCursor( int propertyKeyId, String value )
+    {
+        PropertyItem propertyItem = mock( PropertyItem.class );
+        when( propertyItem.propertyKeyId() ).thenReturn( propertyKeyId );
+        when( propertyItem.value() ).thenReturn( value );
+        Cursor<PropertyItem> propertyCursor = Cursors.cursor( propertyItem );
+        return propertyCursor;
+    }
+
     private StateHandlingStatementOperations newTxStateOps( StoreReadLayer delegate )
     {
         return new StateHandlingStatementOperations( delegate,

community/kernel/src/test/java/org/neo4j/kernel/impl/event/ExpectedTransactionData.java

@@ -126,17 +126,23 @@ void deletedRelationship( Relationship relationship )
     void assignedProperty( Node node, String key, Object value, Object valueBeforeTx )
     {
         valueBeforeTx = removeProperty( expectedRemovedNodeProperties, node, key, valueBeforeTx );
-        Map<String,PropertyEntryImpl<Node>> map = expectedAssignedNodeProperties.get( node );
-        PropertyEntryImpl<Node> prev = map.get( key );
-        map.put( key, property( node, key, value, prev != null ? prev.previouslyCommitedValue() : valueBeforeTx ) );
+        if ( !value.equals( valueBeforeTx ) )
+        {
+            Map<String,PropertyEntryImpl<Node>> map = expectedAssignedNodeProperties.get( node );
+            PropertyEntryImpl<Node> prev = map.get( key );
+            map.put( key, property( node, key, value, prev != null ? prev.previouslyCommitedValue() : valueBeforeTx ) );
+        }
     }
 
     void assignedProperty( Relationship rel, String key, Object value, Object valueBeforeTx )
     {
         valueBeforeTx = removeProperty( expectedRemovedRelationshipProperties, rel, key, valueBeforeTx );
-        Map<String,PropertyEntryImpl<Relationship>> map = expectedAssignedRelationshipProperties.get( rel );
-        PropertyEntryImpl<Relationship> prev = map.get( key );
-        map.put( key, property( rel, key, value, prev != null ? prev.previouslyCommitedValue() : valueBeforeTx ) );
+        if ( !value.equals( valueBeforeTx ) )
+        {
+            Map<String,PropertyEntryImpl<Relationship>> map = expectedAssignedRelationshipProperties.get( rel );
+            PropertyEntryImpl<Relationship> prev = map.get( key );
+            map.put( key, property( rel, key, value, prev != null ? prev.previouslyCommitedValue() : valueBeforeTx ) );
+        }
     }
 
     void assignedLabel( Node node, Label label )