From ba41165532cb3c94048c04b60814e906a39a9ee4 Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 9 Mar 2017 10:58:36 +0100 Subject: [PATCH] Refactored IndexTxStateUpdater to use NodeSchemaMatcher This simplifies the property change methods --- .../api/StateHandlingStatementOperations.java | 7 +- .../impl/api/state/IndexTxStateUpdater.java | 162 ++++++------------ .../api/state/IndexTxStateUpdaterTest.java | 16 +- 3 files changed, 59 insertions(+), 126 deletions(-) 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 4830c61e224e9..05e0ca15f5bb0 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 @@ -1045,14 +1045,13 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp if ( existingProperty == NO_SUCH_PROPERTY ) { state.txState().nodeDoAddProperty( node.id(), property ); - indexTxStateUpdater.onPropertyChange( state, node, indexTxStateUpdater.add( property ) ); + indexTxStateUpdater.onPropertyAdd( state, node, property ); return Property.noProperty( property.propertyKeyId(), EntityType.NODE, node.id() ); } else { state.txState().nodeDoChangeProperty( node.id(), existingProperty, property ); - indexTxStateUpdater - .onPropertyChange( state, node, indexTxStateUpdater.change( existingProperty, property ) ); + indexTxStateUpdater.onPropertyChange( state, node, existingProperty, property ); return existingProperty; } } @@ -1119,7 +1118,7 @@ public Property nodeRemoveProperty( KernelStatement state, long nodeId, int prop autoIndexing.nodes().propertyRemoved( ops, nodeId, propertyKeyId ); state.txState().nodeDoRemoveProperty( node.id(), existingProperty ); - indexTxStateUpdater.onPropertyChange( state, node, indexTxStateUpdater.remove( existingProperty ) ); + indexTxStateUpdater.onPropertyRemove( state, node, existingProperty ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdater.java index e2f71bf86ead6..08e72039f9b6f 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdater.java @@ -33,9 +33,9 @@ import org.neo4j.kernel.impl.api.KernelStatement; import org.neo4j.kernel.impl.api.operations.EntityReadOperations; import org.neo4j.kernel.impl.api.operations.SchemaReadOperations; +import org.neo4j.kernel.impl.api.schema.NodeSchemaMatcher; import org.neo4j.storageengine.api.NodeItem; -import static org.neo4j.kernel.api.StatementConstants.NO_SUCH_PROPERTY_KEY; import static org.neo4j.kernel.api.properties.DefinedProperty.NO_SUCH_PROPERTY; import static org.neo4j.kernel.api.schema_new.SchemaDescriptorPredicates.hasProperty; @@ -43,11 +43,13 @@ public class IndexTxStateUpdater { private final SchemaReadOperations schemaReadOps; private final EntityReadOperations readOps; + private final NodeSchemaMatcher nodeIndexMatcher; public IndexTxStateUpdater( SchemaReadOperations schemaReadOps, EntityReadOperations readOps ) { this.schemaReadOps = schemaReadOps; this.readOps = readOps; + this.nodeIndexMatcher = new NodeSchemaMatcher<>( readOps ); } // LABEL CHANGES @@ -57,8 +59,8 @@ public enum LabelChangeType { ADDED_LABEL, REMOVED_LABEL }; public void onLabelChange( KernelStatement state, int labelId, NodeItem node, LabelChangeType changeType ) throws EntityNotFoundException { - PrimitiveIntSet propertyIds = Primitive.intSet(); - propertyIds.addAll( readOps.nodeGetPropertyKeys( state, node ).iterator() ); + PrimitiveIntSet nodePropertyIds = Primitive.intSet(); + nodePropertyIds.addAll( readOps.nodeGetPropertyKeys( state, node ).iterator() ); Iterator indexes = Iterators.concat( @@ -68,9 +70,10 @@ public void onLabelChange( KernelStatement state, int labelId, NodeItem node, La while ( indexes.hasNext() ) { NewIndexDescriptor index = indexes.next(); - OrderedPropertyValues values = valuesIfPropertiesMatch( state, propertyIds, index, node ); - if ( values != null ) + int[] indexPropertyIds = index.schema().getPropertyIds(); + if ( nodeHasIndexProperties( nodePropertyIds, indexPropertyIds ) ) { + OrderedPropertyValues values = getOrderedPropertyValues( state, node, indexPropertyIds ); if ( changeType == LabelChangeType.ADDED_LABEL ) { state.txState().indexDoUpdateEntry( index.schema(), node.id(), null, values ); @@ -85,75 +88,44 @@ public void onLabelChange( KernelStatement state, int labelId, NodeItem node, La // PROPERTY CHANGES - interface PropertyUpdate - { - void updateIndexIfApplicable( KernelStatement state, NodeItem node, PrimitiveIntSet nodePropertyIds, - NewIndexDescriptor index ) throws EntityNotFoundException; - - int propertyId(); - } - - public PropertyUpdate add( DefinedProperty after ) + public void onPropertyAdd( KernelStatement state, NodeItem node, DefinedProperty after ) + throws EntityNotFoundException { - return new PropertyUpdate() - { - @Override - public void updateIndexIfApplicable( KernelStatement state, NodeItem node, - PrimitiveIntSet nodePropertyIds, NewIndexDescriptor index ) throws EntityNotFoundException - { - OrderedPropertyValues values = valuesIfPropertiesMatch( - state, nodePropertyIds, index, node, after ); - if ( values != null ) - { - values.validate(); - state.txState().indexDoUpdateEntry( index.schema(), node.id(), null, values ); - } - } - - @Override - public int propertyId() - { - return after.propertyKeyId(); - } - }; + Iterator indexes = getIndexesForProperty( state, after.propertyKeyId() ); + nodeIndexMatcher.onMatchingSchema( state, indexes, node, after.propertyKeyId(), + index -> { + OrderedPropertyValues values = getOrderedPropertyValues( state, node, after, index.schema().getPropertyIds() ); + if ( values != null ) + { + values.validate(); + state.txState().indexDoUpdateEntry( index.schema(), node.id(), null, values ); + } + }); } - public PropertyUpdate remove( DefinedProperty before ) + public void onPropertyRemove( KernelStatement state, NodeItem node, DefinedProperty before ) + throws EntityNotFoundException { - return new PropertyUpdate() - { - @Override - public void updateIndexIfApplicable( KernelStatement state, NodeItem node, - PrimitiveIntSet nodePropertyIds, NewIndexDescriptor index ) throws EntityNotFoundException - { - OrderedPropertyValues values = valuesIfPropertiesMatch( state, nodePropertyIds, index, node, - before ); - if ( values != null ) - { - state.txState().indexDoUpdateEntry( index.schema(), node.id(), values, null ); - } - } - - @Override - public int propertyId() - { - return before.propertyKeyId(); - } - }; + Iterator indexes = getIndexesForProperty( state, before.propertyKeyId() ); + nodeIndexMatcher.onMatchingSchema( state, indexes, node, before.propertyKeyId(), + index -> { + OrderedPropertyValues values = getOrderedPropertyValues( state, node, before, index.schema().getPropertyIds() ); + if ( values != null ) + { + state.txState().indexDoUpdateEntry( index.schema(), node.id(), values, null ); + } + }); } - public PropertyUpdate change( DefinedProperty before, DefinedProperty after ) + public void onPropertyChange( KernelStatement state, NodeItem node, DefinedProperty before, DefinedProperty after ) + throws EntityNotFoundException { assert before.propertyKeyId() == after.propertyKeyId(); - return new PropertyUpdate() - { - @Override - public void updateIndexIfApplicable( KernelStatement state, NodeItem node, - PrimitiveIntSet nodePropertyIds, NewIndexDescriptor index ) throws EntityNotFoundException - { - int[] indexPropertyIds = index.schema().getPropertyIds(); - if ( nodeHasIndexProperties( nodePropertyIds, indexPropertyIds ) ) - { + Iterator indexes = getIndexesForProperty( state, before.propertyKeyId() ); + nodeIndexMatcher.onMatchingSchema( state, indexes, node, before.propertyKeyId(), + index -> { + int[] indexPropertyIds = index.schema().getPropertyIds(); + Object[] valuesBefore = new Object[indexPropertyIds.length]; Object[] valuesAfter = new Object[indexPropertyIds.length]; for ( int i = 0; i < indexPropertyIds.length; i++ ) @@ -173,54 +145,20 @@ public void updateIndexIfApplicable( KernelStatement state, NodeItem node, } state.txState().indexDoUpdateEntry( index.schema(), node.id(), OrderedPropertyValues.ofUndefined( valuesBefore ), OrderedPropertyValues.ofUndefined( valuesAfter ) ); - } - } - - @Override - public int propertyId() - { - return before.propertyKeyId(); - } - }; + }); } - public void onPropertyChange( KernelStatement state, NodeItem node, PropertyUpdate update ) - throws EntityNotFoundException - { - PrimitiveIntSet nodePropertyIds = null; - Iterator indexes = getIndexesForProperty( state, update.propertyId() ); - while ( indexes.hasNext() ) - { - NewIndexDescriptor index = indexes.next(); - LabelSchemaDescriptor schema = index.schema(); - if ( node.labels().contains( schema.getLabelId() ) ) - { - if ( nodePropertyIds == null ) - { - nodePropertyIds = Primitive.intSet(); - nodePropertyIds.addAll( readOps.nodeGetPropertyKeys( state, node ).iterator() ); - } - - update.updateIndexIfApplicable( state, node, nodePropertyIds, index ); - } - } - } + // HELPERS - private OrderedPropertyValues valuesIfPropertiesMatch( KernelStatement state, PrimitiveIntSet nodeProperties, - NewIndexDescriptor index, NodeItem node ) throws EntityNotFoundException + private OrderedPropertyValues getOrderedPropertyValues( KernelStatement state, NodeItem node, + int[] indexPropertyIds ) { - return valuesIfPropertiesMatch( state, nodeProperties, index, node, NO_SUCH_PROPERTY ); + return getOrderedPropertyValues( state, node, NO_SUCH_PROPERTY, indexPropertyIds ); } - private OrderedPropertyValues valuesIfPropertiesMatch( KernelStatement state, PrimitiveIntSet nodeProperties, - NewIndexDescriptor index, NodeItem node, DefinedProperty changedProperty ) throws EntityNotFoundException + private OrderedPropertyValues getOrderedPropertyValues( KernelStatement state, NodeItem node, + DefinedProperty changedProperty, int[] indexPropertyIds ) { - int[] indexPropertyIds = index.schema().getPropertyIds(); - if ( !nodeHasIndexProperties( nodeProperties, indexPropertyIds, changedProperty.propertyKeyId() ) ) - { - return null; - } - DefinedProperty[] values = new DefinedProperty[indexPropertyIds.length]; for ( int i = 0; i < values.length; i++ ) { @@ -234,16 +172,10 @@ private OrderedPropertyValues valuesIfPropertiesMatch( KernelStatement state, Pr } private static boolean nodeHasIndexProperties( PrimitiveIntSet nodeProperties, int[] indexPropertyIds ) - { - return nodeHasIndexProperties( nodeProperties, indexPropertyIds, NO_SUCH_PROPERTY_KEY ); - } - - private static boolean nodeHasIndexProperties( - PrimitiveIntSet nodeProperties, int[] indexPropertyIds, int changedPropertyId ) { for ( int indexPropertyId : indexPropertyIds ) { - if ( indexPropertyId != changedPropertyId && !nodeProperties.contains( indexPropertyId ) ) + if ( !nodeProperties.contains( indexPropertyId ) ) { return false; } @@ -251,6 +183,8 @@ private static boolean nodeHasIndexProperties( return true; } + // Lifting this method to the schemaReadOps layer could allow more efficient finding of indexes, by introducing + // suitable maps in the SchemaCache. This can be done when we have a benchmarking reason. private Iterator getIndexesForProperty( KernelStatement state, int propertyId ) { Iterator allIndexes = diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdaterTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdaterTest.java index d2b37b4626431..252de3b7b6385 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdaterTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/state/IndexTxStateUpdaterTest.java @@ -161,10 +161,10 @@ public void shouldUpdateIndexesOnRemovedLabel() throws EntityNotFoundException public void shouldNotUpdateIndexesOnChangedIrrelevantProperty() throws EntityNotFoundException { // WHEN - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.add( property( unIndexedPropId, "whAt" ) )); - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.remove( property( unIndexedPropId, "whAt" ) )); - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.change( - property( unIndexedPropId, "whAt" ), property( unIndexedPropId, "whAt2" ) )); + indexTxUpdater.onPropertyAdd( state, node, property( unIndexedPropId, "whAt" ) ); + indexTxUpdater.onPropertyRemove( state, node, property( unIndexedPropId, "whAt" ) ); + indexTxUpdater.onPropertyChange( state, node, + property( unIndexedPropId, "whAt" ), property( unIndexedPropId, "whAt2" ) ); // THEN verify( txState, times( 0 ) ).indexDoUpdateEntry( any(), anyInt(), any(), any() ); @@ -174,7 +174,7 @@ public void shouldNotUpdateIndexesOnChangedIrrelevantProperty() throws EntityNot public void shouldUpdateIndexesOnAddedProperty() throws EntityNotFoundException { // WHEN - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.add( property( newPropId, "newHi" ) )); + indexTxUpdater.onPropertyAdd( state, node, property( newPropId, "newHi" ) ); // THEN verifyIndexUpdate( indexOn2_new.schema(), node.id(), null, values( "newHi" ) ); @@ -186,7 +186,7 @@ public void shouldUpdateIndexesOnAddedProperty() throws EntityNotFoundException public void shouldUpdateIndexesOnRemovedProperty() throws EntityNotFoundException { // WHEN - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.remove( property( propId2, "hi2" ) )); + indexTxUpdater.onPropertyRemove( state, node, property( propId2, "hi2" ) ); // THEN verifyIndexUpdate( uniqueOn1_2.schema(), node.id(), values( "hi2" ), null ); @@ -198,8 +198,8 @@ public void shouldUpdateIndexesOnRemovedProperty() throws EntityNotFoundExceptio public void shouldUpdateIndexesOnChangesProperty() throws EntityNotFoundException { // WHEN - indexTxUpdater.onPropertyChange( state, node, indexTxUpdater.change( - property( propId2, "hi2" ), property( propId2, "new2" ) )); + indexTxUpdater.onPropertyChange( state, node, + property( propId2, "hi2" ), property( propId2, "new2" ) ); // THEN verifyIndexUpdate( uniqueOn1_2.schema(), node.id(), values( "hi2" ), values( "new2" ) );