Allow dropping broken constraint indexes in more cases

Constraint indexes can normally not be dropped directly. Instead the
owning constraint should be dropped instead.

However, when an index-backed constraint is created, the index and the
constraint are created separately, and we have historically had bugs
that allowed a constraint _index_ to be created, such that it was not
linked to its owning constraint record, or such that its owning
constraint record was never created.

These orhpaned indexes are still accepted as functioning indexes by our
IndexingService, but they are not really usable without their
constraint.

Previously, these indexes would get stuck in this state. Without their
owning constraint, they cannot be dropped. One case was fixed a while
back, which allowed constraint indexes to be dropped in one particular
case. However, a number of other cases were missed.

This PR fixes the remaining cases, such that constraint indexes that
are not linked to any owning constraint can now be dropped in all cases.

We still maintian the behaviour that a constraint index cannot be
directly dropped, but this is now only checked if the index is actually
linked to a valid constraint.

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

@@ -34,7 +34,6 @@
 import org.neo4j.kernel.api.exceptions.schema.NoSuchIndexException;
 import org.neo4j.kernel.api.exceptions.schema.RepeatedPropertyInCompositeSchemaException;
 import org.neo4j.kernel.api.exceptions.schema.SchemaKernelException.OperationContext;
-import org.neo4j.kernel.api.exceptions.schema.SchemaRuleNotFoundException;
 import org.neo4j.kernel.api.exceptions.schema.TooManyLabelsException;
 import org.neo4j.kernel.api.schema.LabelSchemaDescriptor;
 import org.neo4j.kernel.api.schema.RelationTypeSchemaDescriptor;

community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/SchemaCache.java

@@ -90,6 +90,11 @@ public Iterable<ConstraintRule> constraintRules()
         return constraintRuleById.values();
     }
 
+    public boolean hasConstraintRule( Long constraintRuleId )
+    {
+        return constraintRuleId != null && constraintRuleById.containsKey( constraintRuleId );
+    }
+
     public boolean hasConstraintRule( ConstraintDescriptor descriptor )
     {
         for ( ConstraintRule rule : constraintRuleById.values() )

community/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorageLayer.java

@@ -211,7 +211,9 @@ public Long indexGetOwningUniquenessConstraintId( IndexDescriptor index )
         IndexRule rule = indexRule( index );
         if ( rule != null )
         {
-            return rule.getOwningConstraint();
+            // Think of the index as being orphaned if the owning constraint is missing or broken.
+            Long owningConstraint = rule.getOwningConstraint();
+            return schemaCache.hasConstraintRule( owningConstraint ) ? owningConstraint : null;
         }
         return null;
     }

community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/schema/IndexDefinitionImpl.java

@@ -22,6 +22,7 @@
 import java.util.Arrays;
 import java.util.stream.Collectors;
 
+import org.neo4j.graphdb.ConstraintViolationException;
 import org.neo4j.graphdb.Label;
 import org.neo4j.graphdb.schema.IndexDefinition;
 
@@ -63,14 +64,19 @@ public Iterable<String> getPropertyKeys()
     @Override
     public void drop()
     {
-        // expected to call assertInUnterminatedTransaction()
-        if ( this.isConstraintIndex() )
+        try
         {
-            throw new IllegalStateException( "Constraint indexes cannot be dropped directly, " +
-                                             "instead drop the owning uniqueness constraint." );
+            actions.dropIndexDefinitions( this );
+        }
+        catch ( ConstraintViolationException e )
+        {
+            if ( this.isConstraintIndex() )
+            {
+                throw new IllegalStateException( "Constraint indexes cannot be dropped directly, " +
+                                                 "instead drop the owning uniqueness constraint.", e );
+            }
+            throw e;
         }
-
-        actions.dropIndexDefinitions( this );
     }
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/coreapi/schema/SchemaImpl.java

@@ -468,7 +468,7 @@ public void dropIndexDefinitions( IndexDefinition indexDefinition )
                 catch ( SchemaRuleNotFoundException | DropIndexFailureException e )
                 {
                     throw new ConstraintViolationException( e.getUserMessage(
-                            new StatementTokenNameLookup( statement.readOperations() ) ) );
+                            new StatementTokenNameLookup( statement.readOperations() ) ), e );
                 }
                 catch ( InvalidTransactionTypeKernelException e )
                 {

community/kernel/src/test/java/org/neo4j/graphdb/schema/DropBrokenUniquenessConstraintIT.java

@@ -22,20 +22,23 @@
 import org.junit.Rule;
 import org.junit.Test;
 
+import java.util.List;
+
 import org.neo4j.graphdb.Label;
 import org.neo4j.graphdb.Transaction;
 import org.neo4j.kernel.impl.storageengine.impl.recordstorage.RecordStorageEngine;
 import org.neo4j.kernel.impl.store.SchemaStore;
+import org.neo4j.kernel.impl.store.record.ConstraintRule;
 import org.neo4j.kernel.impl.store.record.DynamicRecord;
 import org.neo4j.kernel.impl.store.record.IndexRule;
 import org.neo4j.storageengine.api.schema.SchemaRule;
 import org.neo4j.test.rule.DatabaseRule;
 import org.neo4j.test.rule.EmbeddedDatabaseRule;
 
 import static org.junit.Assert.assertFalse;
-
 import static org.neo4j.helpers.collection.Iterators.filter;
 import static org.neo4j.helpers.collection.Iterators.single;
+import static org.neo4j.kernel.impl.store.record.IndexRule.constraintIndexRule;
 
 public class DropBrokenUniquenessConstraintIT
 {
@@ -46,7 +49,7 @@ public class DropBrokenUniquenessConstraintIT
     public final DatabaseRule db = new EmbeddedDatabaseRule();
 
     @Test
-    public void shouldDropUniquenessConstraintWithBrokenBackingIndex() throws Exception
+    public void shouldDropUniquenessConstraintWithBackingIndexNotInUse() throws Exception
     {
         // given
         try ( Transaction tx = db.beginTx() )
@@ -69,9 +72,119 @@ public void shouldDropUniquenessConstraintWithBrokenBackingIndex() throws Except
         }
 
         // then
+        try ( Transaction ignore = db.beginTx() )
+        {
+            assertFalse( db.schema().getConstraints().iterator().hasNext() );
+            assertFalse( db.schema().getIndexes().iterator().hasNext() );
+        }
+    }
+
+    @Test
+    public void shouldDropUniquenessConstraintWithBackingIndexHavingNoOwner() throws Exception
+    {
+        // given
+        try ( Transaction tx = db.beginTx() )
+        {
+            db.schema().constraintFor( label ).assertPropertyIsUnique( key ).create();
+            tx.success();
+        }
+
+        // when intentionally breaking the schema by setting the backing index rule to unused
+        RecordStorageEngine storageEngine = db.getDependencyResolver().resolveDependency( RecordStorageEngine.class );
+        SchemaStore schemaStore = storageEngine.testAccessNeoStores().getSchemaStore();
+        SchemaRule indexRule = single( filter( rule -> rule instanceof IndexRule, schemaStore.loadAllSchemaRules() ) );
+        setOwnerNull( schemaStore, (IndexRule) indexRule );
+        // At this point the SchemaCache doesn't know about this change so we have to reload it
+        storageEngine.loadSchemaCache();
         try ( Transaction tx = db.beginTx() )
+        {
+            single( db.schema().getConstraints( label ).iterator() ).drop();
+            tx.success();
+        }
+
+        // then
+        try ( Transaction ignore = db.beginTx() )
         {
             assertFalse( db.schema().getConstraints().iterator().hasNext() );
+            assertFalse( db.schema().getIndexes().iterator().hasNext() );
+        }
+    }
+
+    @Test
+    public void shouldDropUniquenessConstraintWhereConstraintRecordIsMissing() throws Exception
+    {
+        // given
+        try ( Transaction tx = db.beginTx() )
+        {
+            db.schema().constraintFor( label ).assertPropertyIsUnique( key ).create();
+            tx.success();
+        }
+
+        // when intentionally breaking the schema by setting the backing index rule to unused
+        RecordStorageEngine storageEngine = db.getDependencyResolver().resolveDependency( RecordStorageEngine.class );
+        SchemaStore schemaStore = storageEngine.testAccessNeoStores().getSchemaStore();
+        SchemaRule indexRule = single( filter( rule -> rule instanceof ConstraintRule, schemaStore.loadAllSchemaRules() ) );
+        setSchemaRecordNotInUse( schemaStore, indexRule.getId() );
+        // At this point the SchemaCache doesn't know about this change so we have to reload it
+        storageEngine.loadSchemaCache();
+        try ( Transaction tx = db.beginTx() )
+        {
+            // We don't use single() here, because it is okay for the schema cache reload to clean up after us.
+            db.schema().getConstraints( label ).forEach( ConstraintDefinition::drop );
+            db.schema().getIndexes( label ).forEach( IndexDefinition::drop );
+            tx.success();
+        }
+
+        // then
+        try ( Transaction ignore = db.beginTx() )
+        {
+            assertFalse( db.schema().getConstraints().iterator().hasNext() );
+            assertFalse( db.schema().getIndexes().iterator().hasNext() );
+        }
+    }
+
+    @Test
+    public void shouldDropUniquenessConstraintWhereConstraintRecordIsMissingAndIndexHasNoOwner() throws Exception
+    {
+        // given
+        try ( Transaction tx = db.beginTx() )
+        {
+            db.schema().constraintFor( label ).assertPropertyIsUnique( key ).create();
+            tx.success();
+        }
+
+        // when intentionally breaking the schema by setting the backing index rule to unused
+        RecordStorageEngine storageEngine = db.getDependencyResolver().resolveDependency( RecordStorageEngine.class );
+        SchemaStore schemaStore = storageEngine.testAccessNeoStores().getSchemaStore();
+        SchemaRule constraintRule = single( filter( rule -> rule instanceof ConstraintRule, schemaStore.loadAllSchemaRules() ) );
+        setSchemaRecordNotInUse( schemaStore, constraintRule.getId() );
+        SchemaRule indexRule = single( filter( rule -> rule instanceof IndexRule, schemaStore.loadAllSchemaRules() ) );
+        setOwnerNull( schemaStore, (IndexRule) indexRule );
+        // At this point the SchemaCache doesn't know about this change so we have to reload it
+        storageEngine.loadSchemaCache();
+        try ( Transaction tx = db.beginTx() )
+        {
+            // We don't use single() here, because it is okay for the schema cache reload to clean up after us.
+            db.schema().getConstraints( label ).forEach( ConstraintDefinition::drop );
+            db.schema().getIndexes( label ).forEach( IndexDefinition::drop );
+            tx.success();
+        }
+
+        // then
+        try ( Transaction ignore = db.beginTx() )
+        {
+            assertFalse( db.schema().getConstraints().iterator().hasNext() );
+            assertFalse( db.schema().getIndexes().iterator().hasNext() );
+        }
+    }
+
+    private void setOwnerNull( SchemaStore schemaStore, IndexRule rule )
+    {
+        rule = constraintIndexRule( rule.getId(), rule.getIndexDescriptor(), rule.getProviderDescriptor(), null );
+        List<DynamicRecord> dynamicRecords = schemaStore.allocateFrom( rule );
+        for ( DynamicRecord record : dynamicRecords )
+        {
+            schemaStore.updateRecord( record );
         }
     }