Skip to content

Commit

Permalink
Check store capability before storing temporal values.
Browse files Browse the repository at this point in the history 
This creates a new Capability for the most recent store format.
In the property store, the most two recent capabilities, both
introduced in 3.4 (points and temporal) are merged into one boolean.
  • Loading branch information
@sherfert
sherfert committed Feb 2, 2018
1 parent 743cb0e commit 9e8766d
Show file tree
Hide file tree
Showing 11 changed files with 253 additions and 65 deletions.
10 changes: 10 additions & 0 deletions ...nity/kernel/src/main/java/org/neo4j/kernel/impl/api/store/StorePropertyPayloadCursor.java
View file
Expand Up @@ -27,6 +27,7 @@
import org.neo4j.kernel.impl.store.PropertyType;
import org.neo4j.kernel.impl.store.RecordCursor;
import org.neo4j.kernel.impl.store.ShortArray;
import org.neo4j.kernel.impl.store.TemporalType;
import org.neo4j.kernel.impl.store.record.DynamicRecord;
import org.neo4j.kernel.impl.store.record.PropertyBlock;
import org.neo4j.kernel.impl.store.record.Record;
Expand Down Expand Up @@ -57,6 +58,7 @@
import static org.neo4j.kernel.impl.store.PropertyType.SHORT_ARRAY;
import static org.neo4j.kernel.impl.store.PropertyType.SHORT_STRING;
import static org.neo4j.kernel.impl.store.PropertyType.STRING;
import static org.neo4j.kernel.impl.store.PropertyType.TEMPORAL;
import static org.neo4j.kernel.impl.store.record.RecordLoad.FORCE;

/**
Expand Down Expand Up @@ -234,6 +236,12 @@ Value geometryValue()
return GeometryType.decode( data, position );
}

Value temporalValue()
{
assertOfType( TEMPORAL );
return TemporalType.decode( data, position );
}

Value value()
{
switch ( type() )
Expand Down Expand Up @@ -264,6 +272,8 @@ Value value()
return arrayValue();
case GEOMETRY:
return geometryValue();
case TEMPORAL:
return temporalValue();
default:
throw new IllegalStateException( "No such type:" + type() );
}
Expand Down
2 changes: 1 addition & 1 deletion ...o4j/kernel/impl/storageengine/impl/recordstorage/RecordStorageCommandCreationContext.java
View file
Expand Up @@ -67,7 +67,7 @@ public class RecordStorageCommandCreationContext implements CommandCreationConte
new StandardDynamicRecordAllocator( idBatches.idGenerator( StoreType.PROPERTY_ARRAY ),
neoStores.getPropertyStore().getArrayStore().getRecordDataSize() ),
idBatches.idGenerator( StoreType.PROPERTY ),
propertyTraverser, neoStores.getPropertyStore().allowStorePoints() );
propertyTraverser, neoStores.getPropertyStore().allowStorePointsAndTemporal() );
}

public long nextId( StoreType storeType )
Expand Down
99 changes: 73 additions & 26 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/store/PropertyStore.java
View file
Expand Up @@ -149,7 +149,11 @@ public class PropertyStore extends CommonAbstractStore<PropertyRecord,NoStoreHea
private final DynamicStringStore stringStore;
private final PropertyKeyTokenStore propertyKeyTokenStore;
private final DynamicArrayStore arrayStore;
private final boolean allowStorePoints;

// In 3.4 we introduced capabilities to store points and temporal data types
// this variable here can be removed once the support for older store versions (that do not have these two
// capabilities) has ceased, the variable can be removed.
private final boolean allowStorePointsAndTemporal;

public PropertyStore(
File fileName,
Expand All @@ -168,7 +172,8 @@ public PropertyStore(
this.stringStore = stringPropertyStore;
this.propertyKeyTokenStore = propertyKeyTokenStore;
this.arrayStore = arrayPropertyStore;
allowStorePoints = recordFormats.hasCapability( Capability.POINT_PROPERTIES );
allowStorePointsAndTemporal = recordFormats.hasCapability( Capability.POINT_PROPERTIES )
&& recordFormats.hasCapability( Capability.TEMPORAL_PROPERTIES );
}

@Override
Expand Down Expand Up @@ -307,11 +312,11 @@ public static void allocateArrayRecords( Collection<DynamicRecord> target, Objec

public void encodeValue( PropertyBlock block, int keyId, Value value )
{
encodeValue( block, keyId, value, stringStore, arrayStore, allowStorePoints );
encodeValue( block, keyId, value, stringStore, arrayStore, allowStorePointsAndTemporal );
}

public static void encodeValue( PropertyBlock block, int keyId, Value value, DynamicRecordAllocator stringAllocator, DynamicRecordAllocator arrayAllocator,
boolean allowStorePoints )
boolean allowStorePointsAndTemporal )
{
if ( value instanceof ArrayValue )
{
Expand All @@ -325,7 +330,7 @@ public static void encodeValue( PropertyBlock block, int keyId, Value value, Dyn

// Fall back to dynamic array store
List<DynamicRecord> arrayRecords = new ArrayList<>();
allocateArrayRecords( arrayRecords, asObject, arrayAllocator, allowStorePoints );
allocateArrayRecords( arrayRecords, asObject, arrayAllocator, allowStorePointsAndTemporal );
setSingleBlockValue( block, keyId, PropertyType.ARRAY, Iterables.first( arrayRecords ).getId() );
for ( DynamicRecord valueRecord : arrayRecords )
{
Expand All @@ -335,7 +340,7 @@ public static void encodeValue( PropertyBlock block, int keyId, Value value, Dyn
}
else
{
value.writeTo( new PropertyBlockValueWriter( block, keyId, stringAllocator, allowStorePoints ) );
value.writeTo( new PropertyBlockValueWriter( block, keyId, stringAllocator, allowStorePointsAndTemporal ) );
}
}

Expand Down Expand Up @@ -409,14 +414,14 @@ private static class PropertyBlockValueWriter implements ValueWriter<IllegalArgu
private final PropertyBlock block;
private final int keyId;
private final DynamicRecordAllocator stringAllocator;
private final boolean allowStorePoints;
private final boolean allowStorePointsAndTemporal;

PropertyBlockValueWriter( PropertyBlock block, int keyId, DynamicRecordAllocator stringAllocator, boolean allowStorePoints )
PropertyBlockValueWriter( PropertyBlock block, int keyId, DynamicRecordAllocator stringAllocator, boolean allowStorePointsAndTemporal )
{
this.block = block;
this.keyId = keyId;
this.stringAllocator = stringAllocator;
this.allowStorePoints = allowStorePoints;
this.allowStorePointsAndTemporal = allowStorePointsAndTemporal;
}

@Override
Expand Down Expand Up @@ -528,7 +533,7 @@ public void writeByteArray( byte[] value ) throws IllegalArgumentException
@Override
public void writePoint( CoordinateReferenceSystem crs, double[] coordinate ) throws IllegalArgumentException
{
if ( allowStorePoints )
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( GeometryType.encodePoint( keyId, crs, coordinate ) );
}
Expand All @@ -541,50 +546,92 @@ public void writePoint( CoordinateReferenceSystem crs, double[] coordinate ) thr
@Override
public void writeDuration( long months, long days, long seconds, int nanos ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeDuration( keyId, months, days, seconds, nanos) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeDuration( keyId, months, days, seconds, nanos) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeDate( long epochDay ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeDate( keyId, epochDay ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeDate( keyId, epochDay ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeLocalTime( long nanoOfDay ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeLocalTime( keyId, nanoOfDay ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeLocalTime( keyId, nanoOfDay ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeTime( long nanosOfDayUTC, int offsetSeconds ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeTime( keyId, nanosOfDayUTC, offsetSeconds ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeTime( keyId, nanosOfDayUTC, offsetSeconds ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeLocalDateTime( long epochSecond, int nano ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeLocalDateTime( keyId, epochSecond, nano ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeLocalDateTime( keyId, epochSecond, nano ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeDateTime( long epochSecondUTC, int nano, int offsetSeconds ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeDateTime( keyId, epochSecondUTC, nano, offsetSeconds ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeDateTime( keyId, epochSecondUTC, nano, offsetSeconds ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}

@Override
public void writeDateTime( long epochSecondUTC, int nano, String zoneId ) throws IllegalArgumentException
{
// TODO check capabilities
block.setValueBlocks( TemporalType.encodeDateTime( keyId, epochSecondUTC, nano, zoneId ) );
if ( allowStorePointsAndTemporal )
{
block.setValueBlocks( TemporalType.encodeDateTime( keyId, epochSecondUTC, nano, zoneId ) );
}
else
{
throw new UnsupportedFormatCapabilityException( Capability.TEMPORAL_PROPERTIES );
}
}
}

Expand Down Expand Up @@ -677,9 +724,9 @@ public PropertyRecord newRecord()
return new PropertyRecord( -1 );
}

public boolean allowStorePoints()
public boolean allowStorePointsAndTemporal()
{
return allowStorePoints;
return allowStorePointsAndTemporal;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/Capability.java
View file
Expand Up @@ -57,6 +57,11 @@ public enum Capability
*/
POINT_PROPERTIES( CapabilityType.STORE ),

/**
* Temporal types are an addition to the format, not a change
*/
TEMPORAL_PROPERTIES( CapabilityType.STORE ),

/**
* Records can spill over into secondary units (another record with a header saying it's a secondary unit to another record).
*/
Expand Down
2 changes: 1 addition & 1 deletion community/kernel/src/main/java/org/neo4j/kernel/impl/store/format/standard/StandardV3_4.java
View file
Expand Up @@ -43,7 +43,7 @@ public class StandardV3_4 extends BaseRecordFormats
public StandardV3_4()
{
super( STORE_VERSION, StoreVersion.STANDARD_V3_4.introductionVersion(), 8, Capability.SCHEMA,
Capability.DENSE_NODES, Capability.LUCENE_5, Capability.POINT_PROPERTIES );
Capability.DENSE_NODES, Capability.LUCENE_5, Capability.POINT_PROPERTIES, Capability.TEMPORAL_PROPERTIES );
}

@Override
Expand Down
11 changes: 5 additions & 6 deletions community/kernel/src/main/java/org/neo4j/kernel/impl/transaction/state/PropertyCreator.java
View file
Expand Up @@ -40,21 +40,21 @@ public class PropertyCreator
private final DynamicRecordAllocator arrayRecordAllocator;
private final IdSequence propertyRecordIdGenerator;
private final PropertyTraverser traverser;
private final boolean allowStorePoints;
private final boolean allowStorePointsAndTemporal;

public PropertyCreator( PropertyStore propertyStore, PropertyTraverser traverser )
{
this( propertyStore.getStringStore(), propertyStore.getArrayStore(), propertyStore, traverser, propertyStore.allowStorePoints() );
this( propertyStore.getStringStore(), propertyStore.getArrayStore(), propertyStore, traverser, propertyStore.allowStorePointsAndTemporal() );
}

public PropertyCreator( DynamicRecordAllocator stringRecordAllocator, DynamicRecordAllocator arrayRecordAllocator, IdSequence propertyRecordIdGenerator,
PropertyTraverser traverser, boolean allowStorePoints )
PropertyTraverser traverser, boolean allowStorePointsAndTemporal )
{
this.stringRecordAllocator = stringRecordAllocator;
this.arrayRecordAllocator = arrayRecordAllocator;
this.propertyRecordIdGenerator = propertyRecordIdGenerator;
this.traverser = traverser;
this.allowStorePoints = allowStorePoints;
this.allowStorePointsAndTemporal = allowStorePointsAndTemporal;
}

public <P extends PrimitiveRecord> void primitiveSetProperty(
Expand Down Expand Up @@ -194,8 +194,7 @@ public PropertyBlock encodePropertyValue( int propertyKey, Value value )

public PropertyBlock encodeValue( PropertyBlock block, int propertyKey, Value value )
{
PropertyStore.encodeValue( block, propertyKey, value, stringRecordAllocator, arrayRecordAllocator,
allowStorePoints );
PropertyStore.encodeValue( block, propertyKey, value, stringRecordAllocator, arrayRecordAllocator, allowStorePointsAndTemporal );
return block;
}

Expand Down
2 changes: 1 addition & 1 deletion community/kernel/src/main/java/org/neo4j/unsafe/impl/batchimport/EntityImporter.java
View file
Expand Up @@ -120,7 +120,7 @@ private PropertyBlock nextPropertyBlock()
private void encodeProperty( PropertyBlock block, int key, Object value )
{
PropertyStore.encodeValue( block, key, Values.of( value ), dynamicStringRecordAllocator, dynamicArrayRecordAllocator,
propertyStore.allowStorePoints() );
propertyStore.allowStorePointsAndTemporal() );
}

protected long createAndWritePropertyChain()
Expand Down
21 changes: 2 additions & 19 deletions community/neo4j/src/test/java/migration/PointPropertiesRecordFormatIT.java
View file
Expand Up @@ -30,7 +30,6 @@
import org.neo4j.graphdb.ResourceIterator;
import org.neo4j.graphdb.Transaction;
import org.neo4j.graphdb.TransactionFailureException;
import org.neo4j.graphdb.factory.GraphDatabaseFactory;
import org.neo4j.helpers.Exceptions;
import org.neo4j.kernel.impl.store.format.standard.Standard;
import org.neo4j.kernel.impl.store.format.standard.StandardV3_2;
Expand All @@ -39,15 +38,13 @@
import org.neo4j.test.rule.TestDirectory;
import org.neo4j.values.storable.PointValue;

import static migration.RecordFormatMigrationIT.startDatabaseWithFormat;
import static migration.RecordFormatMigrationIT.startNonUpgradableDatabaseWithFormat;
import static org.hamcrest.Matchers.arrayWithSize;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.allow_upgrade;
import static org.neo4j.graphdb.factory.GraphDatabaseSettings.record_format;
import static org.neo4j.kernel.configuration.Settings.FALSE;
import static org.neo4j.kernel.configuration.Settings.TRUE;
import static org.neo4j.values.storable.CoordinateReferenceSystem.Cartesian;
import static org.neo4j.values.storable.Values.pointValue;

Expand Down Expand Up @@ -190,18 +187,4 @@ public void failToOpenStoreWithPointPropertyUsingOldFormat()
assertSame( StoreUpgrader.AttemptedDowngradeException.class, Exceptions.rootCause( t ).getClass() );
}
}

private GraphDatabaseService startNonUpgradableDatabaseWithFormat( File storeDir, String formatName )
{
return new GraphDatabaseFactory().newEmbeddedDatabaseBuilder( storeDir )
.setConfig( record_format, formatName )
.setConfig( allow_upgrade, FALSE ).newGraphDatabase();
}

private GraphDatabaseService startDatabaseWithFormat( File storeDir, String formatName )
{
return new GraphDatabaseFactory().newEmbeddedDatabaseBuilder( storeDir )
.setConfig( record_format, formatName )
.setConfig( allow_upgrade, TRUE ).newGraphDatabase();
}
}

0 comments on commit 9e8766d

Please sign in to comment.