diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java index f55b2c05e86af..335d70c2baabd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/ConstraintViolationTransactionFailureException.java @@ -28,7 +28,7 @@ public class ConstraintViolationTransactionFailureException extends TransactionF { public ConstraintViolationTransactionFailureException( String msg, KernelException cause ) { - super( Status.Schema.ConstraintValidationFailed, msg, cause ); + super( Status.Schema.ConstraintValidationFailed, cause, msg ); } public ConstraintViolationTransactionFailureException( String msg ) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/schema/UniquePropertyValueValidationException.java b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/schema/UniquePropertyValueValidationException.java index 5f3bb917b3ceb..e769bbcd338cd 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/schema/UniquePropertyValueValidationException.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/exceptions/schema/UniquePropertyValueValidationException.java @@ -26,26 +26,26 @@ import org.neo4j.kernel.api.TokenNameLookup; import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException; import org.neo4j.kernel.api.schema_new.LabelSchemaDescriptor; -import org.neo4j.kernel.api.schema_new.constaints.UniquenessConstraintDescriptor; +import org.neo4j.kernel.api.schema_new.constaints.IndexBackedConstraintDescriptor; public class UniquePropertyValueValidationException extends ConstraintValidationException { private final Set conflicts; - public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint, + public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint, ConstraintValidationException.Phase phase, IndexEntryConflictException conflict ) { this( constraint, phase, Collections.singleton( conflict ) ); } - public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint, + public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint, ConstraintValidationException.Phase phase, Set conflicts ) { super( constraint, phase, phase == Phase.VERIFICATION ? "Existing data" : "New data" ); this.conflicts = conflicts; } - public UniquePropertyValueValidationException( UniquenessConstraintDescriptor constraint, + public UniquePropertyValueValidationException( IndexBackedConstraintDescriptor constraint, ConstraintValidationException.Phase phase, Throwable cause ) { super( constraint, phase, phase == Phase.VERIFICATION ? "Existing data" : "New data", cause ); diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java index cb731c349ea2e..039fa0a1dcf56 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/ConstraintEnforcingEntityOperations.java @@ -56,6 +56,7 @@ import org.neo4j.kernel.api.schema_new.RelationTypeSchemaDescriptor; import org.neo4j.kernel.api.schema_new.SchemaDescriptor; import org.neo4j.kernel.api.schema_new.constaints.ConstraintDescriptor; +import org.neo4j.kernel.api.schema_new.constaints.IndexBackedConstraintDescriptor; import org.neo4j.kernel.api.schema_new.constaints.NodeExistenceConstraintDescriptor; import org.neo4j.kernel.api.schema_new.constaints.NodeKeyConstraintDescriptor; import org.neo4j.kernel.api.schema_new.constaints.RelExistenceConstraintDescriptor; @@ -91,7 +92,7 @@ public class ConstraintEnforcingEntityOperations implements EntityOperations, Sc private final SchemaWriteOperations schemaWriteOperations; private final SchemaReadOperations schemaReadOperations; private final ConstraintSemantics constraintSemantics; - private final NodeSchemaMatcher nodeSchemaMatcher; + private final NodeSchemaMatcher nodeSchemaMatcher; public ConstraintEnforcingEntityOperations( ConstraintSemantics constraintSemantics, EntityWriteOperations entityWriteOperations, @@ -104,7 +105,7 @@ public ConstraintEnforcingEntityOperations( this.entityReadOperations = entityReadOperations; this.schemaWriteOperations = schemaWriteOperations; this.schemaReadOperations = schemaReadOperations; - nodeSchemaMatcher = new NodeSchemaMatcher<>( entityReadOperations ); + nodeSchemaMatcher = new NodeSchemaMatcher( entityReadOperations ); } @Override @@ -120,7 +121,7 @@ public boolean nodeAddLabel( KernelStatement state, long nodeId, int labelId ) ConstraintDescriptor constraint = constraints.next(); if ( constraint.type().enforcesUniqueness() ) { - UniquenessConstraintDescriptor uniqueConstraint = (UniquenessConstraintDescriptor) constraint; + IndexBackedConstraintDescriptor uniqueConstraint = (IndexBackedConstraintDescriptor) constraint; ExactPredicate[] propertyValues = getAllPropertyValues( state, uniqueConstraint.schema(), node ); if ( propertyValues != null ) { @@ -141,15 +142,13 @@ public Property nodeSetProperty( KernelStatement state, long nodeId, DefinedProp { NodeItem node = cursor.get(); Iterator constraints = getConstraintsInvolvingProperty( state, property.propertyKeyId() ); - Iterator uniquenessConstraints = - new CastingIterator<>( constraints, UniquenessConstraintDescriptor.class ); + Iterator uniquenessConstraints = + new CastingIterator<>( constraints, IndexBackedConstraintDescriptor.class ); nodeSchemaMatcher.onMatchingSchema( state, uniquenessConstraints, node, property.propertyKeyId(), - constraint -> { - validateNoExistingNodeWithExactValues( state, constraint, - getAllPropertyValues( state, constraint.schema(), node, property ), - node.id() ); - } ); + constraint -> validateNoExistingNodeWithExactValues( state, constraint, + getAllPropertyValues( state, constraint.schema(), node, property ), + node.id() ) ); } return entityWriteOperations.nodeSetProperty( state, nodeId, property ); @@ -217,7 +216,7 @@ private ExactPredicate[] getAllPropertyValues( KernelStatement state, SchemaDesc private void validateNoExistingNodeWithExactValues( KernelStatement state, - UniquenessConstraintDescriptor constraint, + IndexBackedConstraintDescriptor constraint, ExactPredicate[] propertyValues, long modifiedNode ) throws ConstraintValidationException @@ -561,6 +560,10 @@ public void uniqueIndexDrop( KernelStatement state, NewIndexDescriptor descripto public NodeKeyConstraintDescriptor nodeKeyConstraintCreate( KernelStatement state, LabelSchemaDescriptor descriptor ) throws AlreadyConstrainedException, CreateConstraintFailureException, AlreadyIndexedException { + Iterator> nodes = new NodeLoadingIterator( nodesGetForLabel( state, descriptor.getLabelId() ), + ( id ) -> nodeCursorById( state, id ) ); + constraintSemantics.validateNodeKeyConstraint( nodes, descriptor, + (node, propertyKey) -> entityReadOperations.nodeHasProperty(state, node, propertyKey) ); return schemaWriteOperations.nodeKeyConstraintCreate( state, descriptor ); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcher.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcher.java index eec904b4d854f..587d6d2b8e429 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcher.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcher.java @@ -32,9 +32,8 @@ /** * This class holds functionality to match LabelSchemaDescriptors to nodes - * @param the type to match. Must implement LabelSchemaSupplier */ -public class NodeSchemaMatcher +public class NodeSchemaMatcher { private final EntityReadOperations readOps; @@ -57,10 +56,11 @@ public NodeSchemaMatcher( EntityReadOperations readOps ) * @param specialPropertyId This property id will always count as a match for the descriptor, regardless of * whether the node has this property or not * @param action The action to take on match + * @param the type to match. Must implement LabelSchemaDescriptor.Supplier * @param The type of exception that can be thrown when taking the action * @throws EXCEPTION This exception is propagated from the action */ - public void onMatchingSchema( + public void onMatchingSchema( KernelStatement state, Iterator schemaSuppliers, NodeItem node, 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 1b2a28a24143a..ab8b948a30a73 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 @@ -47,13 +47,13 @@ public class IndexTxStateUpdater { private final SchemaReadOperations schemaReadOps; private final EntityReadOperations readOps; - private final NodeSchemaMatcher nodeIndexMatcher; + private final NodeSchemaMatcher nodeIndexMatcher; public IndexTxStateUpdater( SchemaReadOperations schemaReadOps, EntityReadOperations readOps ) { this.schemaReadOps = schemaReadOps; this.readOps = readOps; - this.nodeIndexMatcher = new NodeSchemaMatcher<>( readOps ); + this.nodeIndexMatcher = new NodeSchemaMatcher( readOps ); } // LABEL CHANGES diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintSemantics.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintSemantics.java index 78232e9362808..c6731eb9bd1b6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintSemantics.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/ConstraintSemantics.java @@ -41,6 +41,9 @@ */ public interface ConstraintSemantics { + void validateNodeKeyConstraint( Iterator> allNodes, LabelSchemaDescriptor descriptor, + BiPredicate hasProperty ) throws CreateConstraintFailureException; + void validateNodePropertyExistenceConstraint( Iterator> allNodes, LabelSchemaDescriptor descriptor, BiPredicate hasProperty ) throws CreateConstraintFailureException; diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/StandardConstraintSemantics.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/StandardConstraintSemantics.java index 32817cc17b757..286f3688b24c8 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/StandardConstraintSemantics.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/constraints/StandardConstraintSemantics.java @@ -48,7 +48,7 @@ public void validateNodePropertyExistenceConstraint( Iterator> LabelSchemaDescriptor descriptor, BiPredicate hasProperty ) throws CreateConstraintFailureException { - throw propertyExistenceConstraintsNotAllowed( descriptor, ERROR_MESSAGE_NODE_KEY ); + throw propertyExistenceConstraintsNotAllowed( descriptor ); } @Override @@ -56,7 +56,15 @@ public void validateRelationshipPropertyExistenceConstraint( Cursor hasPropertyCheck ) throws CreateConstraintFailureException { - throw propertyExistenceConstraintsNotAllowed( descriptor, ERROR_MESSAGE_EXISTS ); + throw propertyExistenceConstraintsNotAllowed( descriptor ); + } + + @Override + public void validateNodeKeyConstraint( Iterator> allNodes, + LabelSchemaDescriptor descriptor, BiPredicate hasProperty ) + throws CreateConstraintFailureException + { + throw nodeKeyConstraintsNotAllowed( descriptor ); } @Override @@ -80,12 +88,20 @@ protected ConstraintDescriptor readNonStandardConstraint( ConstraintRule rule, S throw new IllegalStateException( errorMessage ); } - private CreateConstraintFailureException propertyExistenceConstraintsNotAllowed( SchemaDescriptor descriptor, String errorMessage ) + private CreateConstraintFailureException propertyExistenceConstraintsNotAllowed( SchemaDescriptor descriptor ) { // When creating a Property Existence Constraint in Community Edition return new CreateConstraintFailureException( - ConstraintDescriptorFactory.existsForSchema( descriptor ), - new IllegalStateException( errorMessage ) ); + ConstraintDescriptorFactory.existsForSchema( descriptor ), + new IllegalStateException( ERROR_MESSAGE_EXISTS ) ); + } + + private CreateConstraintFailureException nodeKeyConstraintsNotAllowed( SchemaDescriptor descriptor ) + { + // When creating a Node Key Constraint in Community Edition + return new CreateConstraintFailureException( + ConstraintDescriptorFactory.existsForSchema( descriptor ), + new IllegalStateException( ERROR_MESSAGE_NODE_KEY ) ); } @Override @@ -99,14 +115,14 @@ public ConstraintRule createUniquenessConstraintRule( public ConstraintRule createNodeKeyConstraintRule( long ruleId, NodeKeyConstraintDescriptor descriptor, long indexId ) throws CreateConstraintFailureException { - throw propertyExistenceConstraintsNotAllowed( descriptor.schema(), ERROR_MESSAGE_NODE_KEY ); + throw nodeKeyConstraintsNotAllowed( descriptor.schema() ); } @Override public ConstraintRule createExistenceConstraint( long ruleId, ConstraintDescriptor descriptor ) throws CreateConstraintFailureException { - throw propertyExistenceConstraintsNotAllowed( descriptor.schema(), ERROR_MESSAGE_EXISTS ); + throw propertyExistenceConstraintsNotAllowed( descriptor.schema() ); } @Override diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcherTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcherTest.java index 3134376c28069..52bcfc7bf545c 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcherTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/schema/NodeSchemaMatcherTest.java @@ -69,7 +69,7 @@ public class NodeSchemaMatcherTest NewIndexDescriptor indexWithMissingLabel = forLabel( nonExistentLabelId, propId1, propId2 ); NewIndexDescriptor indexOnSpecialProperty = forLabel( labelId1, propId1, specialPropId ); - private NodeSchemaMatcher nodeSchemaMatcher; + private NodeSchemaMatcher nodeSchemaMatcher; @Before public void setup() @@ -91,7 +91,7 @@ public void setup() when( readOps.nodeGetProperty( state, node, propId2 ) ).thenReturn( "hi2" ); when( readOps.nodeGetProperty( state, node, unIndexedPropId ) ).thenReturn( "hi3" ); - nodeSchemaMatcher = new NodeSchemaMatcher<>( readOps ); + nodeSchemaMatcher = new NodeSchemaMatcher( readOps ); } @Test diff --git a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeConstraintAcceptanceTest.scala b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeConstraintAcceptanceTest.scala index 5792fb02f9bcf..2890aa5d5ebaf 100644 --- a/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeConstraintAcceptanceTest.scala +++ b/enterprise/cypher/acceptance-spec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/CompositeConstraintAcceptanceTest.scala @@ -19,12 +19,15 @@ */ package org.neo4j.internal.cypher.acceptance +import org.hamcrest.CoreMatchers.containsString +import org.junit.Assert.assertThat import org.neo4j.cypher.javacompat.internal.GraphDatabaseCypherService -import org.neo4j.cypher.{CypherExecutionException, ExecutionEngineFunSuite, NewPlannerTestSupport} -import org.neo4j.graphdb.{ConstraintViolationException, Node} +import org.neo4j.cypher._ +import org.neo4j.graphdb.ConstraintViolationException import org.neo4j.kernel.GraphDatabaseQueryService -import org.neo4j.test.{TestEnterpriseGraphDatabaseFactory, TestGraphDatabaseFactory} +import org.neo4j.test.TestEnterpriseGraphDatabaseFactory import org.scalatest.matchers.{MatchResult, Matcher} +import org.neo4j.cypher.internal.frontend.v3_2.helpers.StringHelper._ import scala.collection.JavaConverters._ @@ -124,6 +127,84 @@ class CompositeConstraintAcceptanceTest extends ExecutionEngineFunSuite with New graph should not(haveConstraints("NODE_KEY:Person(firstname,lastname)")) } + test("trying to add duplicate node when unique constraint exists") { + createLabeledNode(Map("name" -> "A"), "Person") + executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT person.name IS UNIQUE") + + expectError( + "CREATE (n:Person) SET n.name = 'A'", + String.format("Node(0) already exists with label `Person` and property `name` = 'A'") + ) + } + + test("trying to add a node key constraint when duplicates exist") { + createLabeledNode(Map("name" -> "A"), "Person") + createLabeledNode(Map("name" -> "A"), "Person") + + expectError( + "CREATE CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY", + String.format("Unable to create CONSTRAINT ON ( person:Person ) ASSERT person.name IS NODE KEY:%n" + + "Both Node(0) and Node(1) have the label `Person` and property `name` = 'A'") + ) + } + + test("trying to add duplicate node when node key constraint exists") { + createLabeledNode(Map("name" -> "A"), "Person") + executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY") + + expectError( + "CREATE (n:Person) SET n.name = 'A'", + String.format("Node(0) already exists with label `Person` and property `name` = 'A'") + ) + } + + test("drop a non existent node key constraint") { + expectError( + "DROP CONSTRAINT ON (person:Person) ASSERT (person.name) IS NODE KEY", + "No such constraint" + ) + } + + test("trying to add duplicate node when composite unique constraint exists") { + createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person") + executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS UNIQUE") + + expectError( + "CREATE (n:Person) SET n.name = 'A', n.surname = 'B'", + String.format("Node(0) already exists with label `Person` and properties `name` = 'A', `surname` = 'B'") + ) + } + + test("trying to add a composite node key constraint when duplicates exist") { + createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person") + createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person") + + expectError( + "CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS NODE KEY", + String.format("Unable to create CONSTRAINT ON ( person:Person ) ASSERT (person.name, person.surname) IS NODE KEY:%n" + + "Both Node(0) and Node(1) have the label `Person` and properties `name` = 'A', `surname` = 'B'") + ) + } + + test("trying to add duplicate node when composite node key constraint exists") { + createLabeledNode(Map("name" -> "A", "surname" -> "B"), "Person") + executeQuery("CREATE CONSTRAINT ON (person:Person) ASSERT (person.name, person.surname) IS NODE KEY") + + expectError( + "CREATE (n:Person) SET n.name = 'A', n.surname = 'B'", + String.format("Node(0) already exists with label `Person` and properties `name` = 'A', `surname` = 'B'") + ) + } + + private def expectError(query: String, expectedError: String) { + val error = intercept[CypherException](executeQuery(query)) + assertThat(error.getMessage, containsString(expectedError)) + } + + private def executeQuery(query: String) { + executeWithCostPlannerAndInterpretedRuntimeOnly(query.fixNewLines).toList + } + case class haveConstraints(expectedConstraints: String*) extends Matcher[GraphDatabaseQueryService] { def apply(graph: GraphDatabaseQueryService): MatchResult = { graph.inTx { diff --git a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/EnterpriseConstraintSemantics.java b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/EnterpriseConstraintSemantics.java index a87b9daa6b48e..0149a2d667507 100644 --- a/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/EnterpriseConstraintSemantics.java +++ b/enterprise/kernel/src/main/java/org/neo4j/kernel/impl/enterprise/EnterpriseConstraintSemantics.java @@ -87,6 +87,14 @@ public void validateNodePropertyExistenceConstraint( Iterator> } } + @Override + public void validateNodeKeyConstraint( Iterator> allNodes, + LabelSchemaDescriptor descriptor, BiPredicate hasPropertyCheck ) + throws CreateConstraintFailureException + { + validateNodePropertyExistenceConstraint( allNodes, descriptor, hasPropertyCheck ); + } + private void validateNodePropertyExistenceConstraint( NodeItem node, int propertyKey, LabelSchemaDescriptor descriptor, BiPredicate hasPropertyCheck ) throws CreateConstraintFailureException