Spatial index populator and accessor now verifyDeferredConstraints

When populating uniqueness index with unique points that collided on the
space filling curve the population would wrongly throw a constraint violation
exception because it assumed that if two points collide on space filling curve
they must be the same. This is not true. Points that are not equal can map
to the same space filling curve tile.

Spatial index need three changes to fix this:
- Less strict value merger
Spatial populator use a less strict ConflictDetectingValueMerger to allow
collisions on the space filling curve.

- Updater don't check for conflicts
Spatial populating updater is not wrapped by a DeferredConflictCheckingIndexUpdater
because it would only use index itself to verify conflicts. It would thus
throw constraint violation exceptions without verifying that the points where
in fact equal.

- Spatial index population fall back to verifyDeferredConstraints
This is the same way lucene index populator verify uniqueness for numbers.
After population is complete and before constraint is created we scan index
and for all points that collide on the space filling curve, we go to property
store to check if the points are actually equal or not. If they are, we
throw an constraint violation exception on the first violation we find.
This is done both in SpatialIndexPopulator and SpatialIndexAccessor because
there are two paths where vi verify constraints like this.
1. ConstraintIndexCreator -> OnlineIndexProxy -> IndexAccessor.
2. BatchInserterImpl -> FlippableIndexProxy -> PopulatingIndexProxy -> IndexPopulator

NOTE! The other native indexes still function the same way, which is safe
because they store the actual property values and can verify uniqueness exactly.

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeIndexAccessor.java

@@ -30,6 +30,7 @@
 import org.neo4j.io.pagecache.IOLimiter;
 import org.neo4j.io.pagecache.PageCache;
 import org.neo4j.io.pagecache.PageCursor;
+import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
 import org.neo4j.kernel.api.index.IndexAccessor;
 import org.neo4j.kernel.api.index.IndexProvider;
 import org.neo4j.kernel.api.index.NodePropertyAccessor;
@@ -123,7 +124,7 @@ public ResourceIterator<File> snapshotFiles()
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {   // Not needed since uniqueness is verified automatically w/o cost for every update.
     }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/NativeIndexPopulator.java

@@ -127,12 +127,17 @@ protected synchronized void create( Consumer<PageCursor> headerWriter )
 
         // true:  tree uniqueness is (value,entityId)
         // false: tree uniqueness is (value) <-- i.e. more strict
-        mainConflictDetector = new ConflictDetectingValueMerger<>( descriptor.type() == GENERAL );
+        mainConflictDetector = getMainConflictDetector();
         // for updates we have to have uniqueness on (value,entityId) to allow for intermediary violating updates.
         // there are added conflict checks after updates have been applied.
         updatesConflictDetector = new ConflictDetectingValueMerger<>( true );
     }
 
+    ConflictDetectingValueMerger<KEY,VALUE> getMainConflictDetector()
+    {
+        return new ConflictDetectingValueMerger<>( descriptor.type() == GENERAL );
+    }
+
     @Override
     public synchronized void drop()
     {
@@ -160,7 +165,7 @@ public void add( Collection<? extends IndexEntryUpdate<?>> updates ) throws Inde
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {
         // No-op, uniqueness is checked for each update in add(IndexEntryUpdate)
     }
@@ -174,7 +179,7 @@ public IndexUpdater newPopulatingUpdater( NodePropertyAccessor accessor )
     IndexUpdater newPopulatingUpdater()
     {
         IndexUpdater updater = new CollectingIndexUpdater( updates -> processUpdates( updates, updatesConflictDetector ) );
-        if ( descriptor.type() == UNIQUE )
+        if ( descriptor.type() == UNIQUE && canCheckConflictsWithoutStoreAccess() )
         {
             // The index population detects conflicts on the fly, however for updates coming in we're in a position
             // where we cannot detect conflicts while applying, but instead afterwards.
@@ -183,6 +188,11 @@ IndexUpdater newPopulatingUpdater()
         return updater;
     }
 
+    boolean canCheckConflictsWithoutStoreAccess()
+    {
+        return true;
+    }
+
     abstract NativeIndexReader<KEY,VALUE> newReader();
 
     @Override

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ParallelNativeIndexPopulator.java

@@ -132,7 +132,7 @@ public void add( Collection<? extends IndexEntryUpdate<?>> scanBatch ) throws In
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {
         ensureMerged();
         completePopulator.verifyDeferredConstraints( nodePropertyAccessor );

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexAccessor.java

@@ -34,6 +34,7 @@
 import org.neo4j.io.fs.FileSystemAbstraction;
 import org.neo4j.io.pagecache.IOLimiter;
 import org.neo4j.io.pagecache.PageCache;
+import org.neo4j.kernel.api.exceptions.index.IndexEntryConflictException;
 import org.neo4j.kernel.api.index.IndexAccessor;
 import org.neo4j.kernel.api.index.IndexPopulator;
 import org.neo4j.kernel.api.index.IndexProvider;
@@ -164,9 +165,12 @@ public ResourceIterator<File> snapshotFiles()
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {
-        // Not needed since uniqueness is verified automatically w/o cost for every update.
+        for ( NativeIndexAccessor<?,?> part : this )
+        {
+            part.verifyDeferredConstraints( nodePropertyAccessor );
+        }
     }
 
     @Override
@@ -198,6 +202,13 @@ public SpatialIndexPartReader<NativeIndexValue> newReader()
             assertOpen();
             return new SpatialIndexPartReader<>( tree, layout, descriptor, searchConfiguration );
         }
+
+        @Override
+        public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
+        {
+            SpatialVerifyDeferredConstraint.verify( nodePropertyAccessor, layout, tree, descriptor );
+            super.verifyDeferredConstraints( nodePropertyAccessor );
+        }
     }
 
     static class PartFactory implements Factory<PartAccessor>

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/SpatialIndexPopulator.java

@@ -91,9 +91,12 @@ public void add( Collection<? extends IndexEntryUpdate<?>> updates ) throws Inde
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {
-        // No-op, uniqueness is checked for each update in add(IndexEntryUpdate)
+        for ( IndexPopulator part : this )
+        {
+            part.verifyDeferredConstraints( nodePropertyAccessor );
+        }
     }
 
     @Override
@@ -153,6 +156,28 @@ static class PartPopulator extends NativeIndexPopulator<SpatialIndexKey,NativeIn
             this.settings = fileLayout.settings;
         }
 
+        @Override
+        public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
+        {
+            SpatialVerifyDeferredConstraint.verify( nodePropertyAccessor, layout, tree, descriptor );
+            super.verifyDeferredConstraints( nodePropertyAccessor );
+        }
+
+        @Override
+        boolean canCheckConflictsWithoutStoreAccess()
+        {
+            return false;
+        }
+
+        @Override
+        ConflictDetectingValueMerger<SpatialIndexKey,NativeIndexValue> getMainConflictDetector()
+        {
+            // Because of lossy point representation in index we need to always compare on node id,
+            // even for unique indexes. If we don't we risk throwing constraint violation exception
+            // for points that are in fact unique.
+            return new ConflictDetectingValueMerger<>( true );
+        }
+
         @Override
         NativeIndexReader<SpatialIndexKey, NativeIndexValue> newReader()
         {

community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/WorkSyncedNativeIndexPopulator.java

@@ -96,7 +96,7 @@ public void add( Collection<? extends IndexEntryUpdate<?>> updates ) throws Inde
     }
 
     @Override
-    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor )
+    public void verifyDeferredConstraints( NodePropertyAccessor nodePropertyAccessor ) throws IndexEntryConflictException
     {
         actual.verifyDeferredConstraints( nodePropertyAccessor );
     }

community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/ParallelNativeIndexPopulatorTest.java

@@ -39,6 +39,7 @@
 import java.util.function.Consumer;
 import java.util.function.Supplier;
 
+import org.neo4j.function.ThrowingConsumer;
 import org.neo4j.gis.spatial.index.curves.SpaceFillingCurveConfiguration;
 import org.neo4j.graphdb.mockfs.EphemeralFileSystemAbstraction;
 import org.neo4j.internal.kernel.api.IndexOrder;
@@ -222,8 +223,8 @@ void shouldMergePartsOnAccessingSampleResultAfterPopulation() throws Exception
         shouldMergePartsOnAccessingFirstCompleteMethodAfterPopulation( ParallelNativeIndexPopulator::sampleResult );
     }
 
-    private void shouldMergePartsOnAccessingFirstCompleteMethodAfterPopulation( Consumer<ParallelNativeIndexPopulator<GenericKey,NativeIndexValue>> method )
-            throws Exception
+    private void shouldMergePartsOnAccessingFirstCompleteMethodAfterPopulation(
+            ThrowingConsumer<ParallelNativeIndexPopulator<GenericKey,NativeIndexValue>,IndexEntryConflictException> method ) throws Exception
     {
         try ( EphemeralFileSystemAbstraction fs = new EphemeralFileSystemAbstraction(); JobScheduler jobScheduler = createInitialisedScheduler() )
         {

community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/SpatialUniqueIndexPopulatorTest.java

@@ -59,4 +59,18 @@ IndexLayout<SpatialIndexKey,NativeIndexValue> createLayout()
     {
         return new SpatialLayout( crs, configuredSettings.forCRS( crs ).curve() );
     }
+
+    @Override
+    public void addShouldThrowOnDuplicateValues()
+    {   // Spatial can not throw on duplicate values during population because it
+        // might throw for points that are in fact unique. Instead, uniqueness will
+        // be verified by ConstraintIndexCreator when population is finished.
+    }
+
+    @Override
+    public void updaterShouldThrowOnDuplicateValues()
+    {   // Spatial can not throw on duplicate values during population because it
+        // might throw for points that are in fact unique. Instead, uniqueness will
+        // be verified by ConstraintIndexCreator when population is finished.
+    }
 }