diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/DateLayout.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/DateLayout.java index 6e7b30a441571..78e4377850236 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/DateLayout.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/DateLayout.java @@ -58,7 +58,15 @@ public int minorVersion() public int compare( DateSchemaKey o1, DateSchemaKey o2 ) { int comparison = o1.compareValueTo( o2 ); - return comparison != 0 ? comparison : Long.compare( o1.getEntityId(), o2.getEntityId() ); + if ( comparison == 0 ) + { + // This is a special case where we need also compare entityId to support inclusive/exclusive + if ( o1.getCompareId() || o2.getCompareId() ) + { + return Long.compare( o1.getEntityId(), o2.getEntityId() ); + } + } + return comparison; } }; @@ -87,15 +95,7 @@ public int minorVersion() public int compare( DateSchemaKey o1, DateSchemaKey o2 ) { int comparison = o1.compareValueTo( o2 ); - if ( comparison == 0 ) - { - // This is a special case where we need also compare entityId to support inclusive/exclusive - if ( o1.getCompareId() || o2.getCompareId() ) - { - return Long.compare( o1.getEntityId(), o2.getEntityId() ); - } - } - return comparison; + return comparison != 0 ? comparison : Long.compare( o1.getEntityId(), o2.getEntityId() ); } }; diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java b/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java index 1003fee823adf..46e2f7aa24f44 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/api/index/IndexProviderCompatibilityTestSuite.java @@ -57,13 +57,9 @@ public abstract class IndexProviderCompatibilityTestSuite { protected abstract SchemaIndexProvider createIndexProvider( PageCache pageCache, FileSystemAbstraction fs, File graphDbDir ); - public abstract Iterable getSupportedValues(); + public abstract boolean supportsSpatial(); - protected List commonValues = Arrays.asList( Values.of( "string1" ), Values.of( 42 ) ); - protected List temporalValues = Arrays.asList( DateValue.epochDate( 2 ), DateValue.epochDate( 5 ) ); - protected List spatialValues = Arrays.asList( - Values.pointValue( CoordinateReferenceSystem.Cartesian, 0, 0 ), - Values.pointValue( CoordinateReferenceSystem.WGS84, 12.78, 56.7 ) ); + public abstract boolean supportsTemporal(); public abstract static class Compatibility { @@ -75,7 +71,8 @@ public abstract static class Compatibility protected SchemaIndexProvider indexProvider; protected IndexDescriptor descriptor; final IndexProviderCompatibilityTestSuite testSuite; - final List allValues; + final List valueSet1; + final List valueSet2; @Before public void setup() @@ -90,7 +87,22 @@ public Compatibility( IndexProviderCompatibilityTestSuite testSuite, IndexDescri { this.testSuite = testSuite; this.descriptor = descriptor; - this.allValues = allValues( testSuite.getSupportedValues() ); + this.valueSet1 = allValues( + testSuite.supportsSpatial(), + testSuite.supportsTemporal(), + Arrays.asList( Values.of( "string1" ), Values.of( 42 ) ), + Arrays.asList( DateValue.epochDate( 2 ), DateValue.epochDate( 5 ) ), + Arrays.asList( Values.pointValue( CoordinateReferenceSystem.Cartesian, 0, 0 ), + Values.pointValue( CoordinateReferenceSystem.WGS84, 12.78, 56.7 ) ) ); + + this.valueSet2 = allValues( + testSuite.supportsSpatial(), + testSuite.supportsTemporal(), + Arrays.asList( Values.of( "string2" ), Values.of( 1337 ) ), + Arrays.asList( DateValue.epochDate( 42 ), DateValue.epochDate( 1337 ) ), + Arrays.asList( Values.pointValue( CoordinateReferenceSystem.Cartesian, 10, 10 ), + Values.pointValue( CoordinateReferenceSystem.WGS84, 87.21, 7.65 ) ) ); + pageCacheAndDependenciesRule = new PageCacheAndDependenciesRule( DefaultFileSystemRule::new, testSuite.getClass() ); } @@ -119,14 +131,44 @@ protected void withPopulator( IndexPopulator populator, ThrowingConsumer allValues( Iterable supportedValues ) + List> updates( List values ) + { + return updates( values, 0 ); + } + + List> updates( List values, long nodeIdOffset ) + { + List> updates = new ArrayList<>(); + values.forEach( entry -> updates.add( IndexEntryUpdate.add( nodeIdOffset + entry.nodeId, descriptor.schema(), entry.value ) ) ); + return updates; + } + + private static List allValues( boolean supportsSpatial, + boolean supportsTemporal, + List common, + List temporal, + List spatial ) { long nodeIds = 0; List result = new ArrayList<>(); - for ( Value value : supportedValues ) + for ( Value value : common ) { result.add( new NodeAndValue( nodeIds++, value ) ); } + if ( supportsSpatial ) + { + for ( Value value : spatial ) + { + result.add( new NodeAndValue( nodeIds++, value ) ); + } + } + if ( supportsTemporal ) + { + for ( Value value : temporal ) + { + result.add( new NodeAndValue( nodeIds++, value ) ); + } + } return result; } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java b/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java index 6bc74a4a01cbf..2a0c47ebfb03a 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexAccessorCompatibility.java @@ -22,7 +22,6 @@ import org.junit.Ignore; import org.junit.Test; -import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -127,13 +126,12 @@ public void testIndexSeekByPrefixOnNonStrings() throws Exception public void shouldUpdateWithAllValues() throws Exception { // GIVEN - List> updates = new ArrayList<>(); - allValues.forEach( entry -> updates.add( IndexEntryUpdate.add( entry.nodeId, descriptor.schema(), entry.value ) ) ); + List> updates = updates( valueSet1 ); updateAndCommit( updates ); // then int propertyKeyId = descriptor.schema().getPropertyId(); - for ( NodeAndValue entry : allValues ) + for ( NodeAndValue entry : valueSet1 ) { List result = query( IndexQuery.exact( propertyKeyId, entry.value ) ); assertThat( result, equalTo( Collections.singletonList( entry.nodeId ) ) ); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexPopulatorCompatibility.java b/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexPopulatorCompatibility.java index 5b453600f9d02..7c61b8a8a1423 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexPopulatorCompatibility.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/api/index/SimpleIndexPopulatorCompatibility.java @@ -23,12 +23,12 @@ import org.junit.Test; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongIterator; +import org.neo4j.helpers.collection.Iterables; import org.neo4j.internal.kernel.api.IndexOrder; import org.neo4j.internal.kernel.api.IndexQuery; import org.neo4j.internal.kernel.api.schema.LabelSchemaDescriptor; @@ -37,6 +37,7 @@ import org.neo4j.kernel.api.schema.index.IndexDescriptor; import org.neo4j.kernel.api.schema.index.IndexDescriptorFactory; import org.neo4j.kernel.configuration.Config; +import org.neo4j.kernel.impl.api.index.IndexUpdateMode; import org.neo4j.kernel.impl.api.index.sampling.IndexSamplingConfig; import org.neo4j.kernel.impl.index.schema.NodeValueIterator; import org.neo4j.storageengine.api.schema.IndexReader; @@ -68,7 +69,7 @@ public SimpleIndexPopulatorCompatibility( super( testSuite, descriptor ); } - private final IndexSamplingConfig indexSamplingConfig = new IndexSamplingConfig( Config.defaults() ); + final IndexSamplingConfig indexSamplingConfig = new IndexSamplingConfig( Config.defaults() ); @Test public void shouldStorePopulationFailedForRetrievalFromProviderLater() throws Exception @@ -146,7 +147,7 @@ public void shouldApplyUpdatesIdempotently() throws Exception p.close( true ); } ); - // then + // THEN try ( IndexAccessor accessor = indexProvider.getOnlineAccessor( 17, descriptor, indexSamplingConfig ) ) { try ( IndexReader reader = accessor.newReader() ) @@ -166,17 +167,12 @@ public void shouldPopulateWithAllValues() throws Exception withPopulator( indexProvider.getPopulator( 17, descriptor, indexSamplingConfig ), p -> { p.create(); - - List> updates = new ArrayList<>(); - allValues.forEach( entry -> updates.add( IndexEntryUpdate.add( entry.nodeId, descriptor.schema(), entry.value ) ) ); - - p.add( updates ); - + p.add( updates( valueSet1 ) ); p.close( true ); } ); - // then - assertHasAllValues(); + // THEN + assertHasAllValues( valueSet1 ); } @Test @@ -187,9 +183,9 @@ public void shouldUpdateWithAllValuesDuringPopulation() throws Exception { p.create(); - try ( IndexUpdater updater = p.newPopulatingUpdater( this::allValueLookup ) ) + try ( IndexUpdater updater = p.newPopulatingUpdater( this::valueSet1Lookup ) ) { - for ( NodeAndValue entry : allValues ) + for ( NodeAndValue entry : valueSet1 ) { updater.process( IndexEntryUpdate.add( entry.nodeId, descriptor.schema(), entry.value ) ); } @@ -198,13 +194,51 @@ public void shouldUpdateWithAllValuesDuringPopulation() throws Exception p.close( true ); } ); - // then - assertHasAllValues(); + // THEN + assertHasAllValues( valueSet1 ); + } + + @Test + public void shouldPopulateAndUpdate() throws Exception + { + // GIVEN + withPopulator( indexProvider.getPopulator( 17, descriptor, indexSamplingConfig ), p -> + { + p.create(); + p.add( updates( valueSet1 ) ); + p.close( true ); + } ); + + try ( IndexAccessor accessor = indexProvider.getOnlineAccessor( 17, descriptor, indexSamplingConfig ) ) + { + // WHEN + try ( IndexUpdater updater = accessor.newUpdater( IndexUpdateMode.ONLINE ) ) + { + List> updates = updates( valueSet2 ); + for ( IndexEntryUpdate update : updates ) + { + updater.process( update ); + } + } + + // THEN + try ( IndexReader reader = accessor.newReader() ) + { + int propertyKeyId = descriptor.schema().getPropertyId(); + for ( NodeAndValue entry : Iterables.concat( valueSet1, valueSet2 ) ) + { + NodeValueIterator nodes = new NodeValueIterator(); + reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) ); + assertEquals( entry.nodeId, single( nodes, NO_SUCH_NODE ) ); + } + } + accessor.close(); + } } - private Value allValueLookup( long nodeId, int propertyId ) + private Value valueSet1Lookup( long nodeId, int propertyId ) { - for ( NodeAndValue x : allValues ) + for ( NodeAndValue x : valueSet1 ) { if ( x.nodeId == nodeId ) { @@ -214,14 +248,14 @@ private Value allValueLookup( long nodeId, int propertyId ) return Values.NO_VALUE; } - private void assertHasAllValues() throws IOException, IndexNotApplicableKernelException + private void assertHasAllValues( List values ) throws IOException, IndexNotApplicableKernelException { try ( IndexAccessor accessor = indexProvider.getOnlineAccessor( 17, descriptor, indexSamplingConfig ) ) { try ( IndexReader reader = accessor.newReader() ) { int propertyKeyId = descriptor.schema().getPropertyId(); - for ( NodeAndValue entry : allValues ) + for ( NodeAndValue entry : values ) { NodeValueIterator nodes = new NodeValueIterator(); reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) ); @@ -244,14 +278,12 @@ public General( IndexProviderCompatibilityTestSuite testSuite ) public void shouldProvidePopulatorThatAcceptsDuplicateEntries() throws Exception { // when - IndexSamplingConfig indexSamplingConfig = new IndexSamplingConfig( Config.defaults() ); - Value value = Values.of( "value1" ); + long offset = valueSet1.size(); withPopulator( indexProvider.getPopulator( 17, descriptor, indexSamplingConfig ), p -> { p.create(); - p.add( Arrays.asList( - IndexEntryUpdate.add( 1, descriptor.schema(), value ), - IndexEntryUpdate.add( 2, descriptor.schema(), value ) ) ); + p.add( updates( valueSet1, 0 ) ); + p.add( updates( valueSet1, offset ) ); p.close( true ); } ); @@ -260,8 +292,13 @@ public void shouldProvidePopulatorThatAcceptsDuplicateEntries() throws Exception { try ( IndexReader reader = accessor.newReader() ) { - PrimitiveLongIterator nodes = reader.query( IndexQuery.exact( 1, value ) ); - assertEquals( asSet( 1L, 2L ), PrimitiveLongCollections.toSet( nodes ) ); + int propertyKeyId = descriptor.schema().getPropertyId(); + for ( NodeAndValue entry : valueSet1 ) + { + NodeValueIterator nodes = new NodeValueIterator(); + reader.query( nodes, IndexOrder.NONE, IndexQuery.exact( propertyKeyId, entry.value ) ); + assertEquals( asSet( entry.nodeId, entry.nodeId + offset ), PrimitiveLongCollections.toSet( nodes ) ); + } } accessor.close(); } @@ -287,7 +324,6 @@ public void shouldProvidePopulatorThatEnforcesUniqueConstraints() throws Excepti int nodeId1 = 1; int nodeId2 = 2; - IndexSamplingConfig indexSamplingConfig = new IndexSamplingConfig( Config.defaults() ); withPopulator( indexProvider.getPopulator( 17, descriptor, indexSamplingConfig ), p -> { p.create(); diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexProviderTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexProviderTest.java index a117b8c5c7e50..7ef782f96f168 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexProviderTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/api/index/inmemory/InMemoryIndexProviderTest.java @@ -21,12 +21,10 @@ import java.io.File; -import org.neo4j.helpers.collection.Iterables; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; import org.neo4j.kernel.api.index.IndexProviderCompatibilityTestSuite; import org.neo4j.kernel.api.index.SchemaIndexProvider; -import org.neo4j.values.storable.Value; public class InMemoryIndexProviderTest extends IndexProviderCompatibilityTestSuite { @@ -37,8 +35,14 @@ protected SchemaIndexProvider createIndexProvider( PageCache pageCache, FileSyst } @Override - public Iterable getSupportedValues() + public boolean supportsSpatial() { - return Iterables.concat( commonValues, spatialValues, temporalValues ); + return true; + } + + @Override + public boolean supportsTemporal() + { + return true; } } diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/FusionSchemaIndexProviderCompatibilitySuiteTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/FusionSchemaIndexProviderCompatibilitySuiteTest.java index a826a1682a765..5e633f6967033 100644 --- a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/FusionSchemaIndexProviderCompatibilitySuiteTest.java +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/FusionSchemaIndexProviderCompatibilitySuiteTest.java @@ -22,7 +22,6 @@ import java.io.File; import org.neo4j.graphdb.factory.GraphDatabaseSettings; -import org.neo4j.helpers.collection.Iterables; import org.neo4j.index.internal.gbptree.RecoveryCleanupWorkCollector; import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.pagecache.PageCache; @@ -31,7 +30,6 @@ import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Settings; import org.neo4j.kernel.impl.factory.OperationalMode; -import org.neo4j.values.storable.Value; import static org.neo4j.helpers.collection.MapUtil.stringMap; @@ -48,8 +46,14 @@ protected SchemaIndexProvider createIndexProvider( PageCache pageCache, FileSyst } @Override - public Iterable getSupportedValues() + public boolean supportsSpatial() { - return Iterables.concat( commonValues, spatialValues, temporalValues ); + return true; + } + + @Override + public boolean supportsTemporal() + { + return true; } } diff --git a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/LuceneSchemaIndexProviderCompatibilitySuiteTest.java b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/LuceneSchemaIndexProviderCompatibilitySuiteTest.java index 71b597c892617..f8b3fc296d95e 100644 --- a/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/LuceneSchemaIndexProviderCompatibilitySuiteTest.java +++ b/community/lucene-index/src/test/java/org/neo4j/kernel/api/impl/schema/LuceneSchemaIndexProviderCompatibilitySuiteTest.java @@ -30,7 +30,6 @@ import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Settings; import org.neo4j.kernel.impl.factory.OperationalMode; -import org.neo4j.values.storable.Value; import static org.neo4j.helpers.collection.MapUtil.stringMap; @@ -47,8 +46,14 @@ protected LuceneSchemaIndexProvider createIndexProvider( PageCache pageCache, Fi } @Override - public Iterable getSupportedValues() + public boolean supportsSpatial() { - return commonValues; + return false; + } + + @Override + public boolean supportsTemporal() + { + return false; } }