Do not throw when reading SchemaKey time zones

community/bolt/src/test/java/org/neo4j/bolt/v2/messaging/Neo4jPackV2Test.java

@@ -35,7 +35,7 @@
 import org.neo4j.bolt.v1.messaging.Neo4jPack;
 import org.neo4j.bolt.v1.packstream.PackedInputArray;
 import org.neo4j.bolt.v1.packstream.PackedOutputArray;
-import org.neo4j.kernel.impl.store.TimeZoneMapping;
+import org.neo4j.kernel.impl.store.TimeZones;
 import org.neo4j.values.AnyValue;
 import org.neo4j.values.storable.CoordinateReferenceSystem;
 import org.neo4j.values.storable.DateTimeValue;
@@ -74,7 +74,9 @@
 public class Neo4jPackV2Test
 {
     private static final String[] TIME_ZONE_NAMES =
-            TimeZoneMapping.supportedTimeZones().stream().filter( s -> ZoneId.getAvailableZoneIds().contains( s ) ).toArray( length -> new String[length] );
+            TimeZones.supportedTimeZones().stream()
+                    .filter( s -> ZoneId.getAvailableZoneIds().contains( s ) )
+                    .toArray( length -> new String[length] );
 
     private static final int RANDOM_VALUES_TO_TEST = 1_000;
     private static final int RANDOM_LISTS_TO_TEST = 1_000;

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

@@ -22,7 +22,7 @@
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 
-import org.neo4j.kernel.impl.store.TimeZoneMapping;
+import org.neo4j.kernel.impl.store.TimeZones;
 import org.neo4j.values.storable.DateTimeValue;
 import org.neo4j.values.storable.Value;
 import org.neo4j.values.storable.Values;
@@ -53,7 +53,7 @@ class ZonedDateTimeSchemaKey extends ComparableNativeSchemaKey<ZonedDateTimeSche
     public Value asValue()
     {
         return zoneId >= 0 ?
-            DateTimeValue.datetime( epochSecondUTC, nanoOfSecond, ZoneId.of( TimeZoneMapping.map( zoneId ) ) ) :
+            DateTimeValue.datetime( epochSecondUTC, nanoOfSecond, ZoneId.of( TimeZones.map( zoneId ) ) ) :
             DateTimeValue.datetime( epochSecondUTC, nanoOfSecond, ZoneOffset.ofTotalSeconds( zoneOffsetSeconds ) );
     }
 
@@ -82,7 +82,7 @@ public int compareValueTo( ZonedDateTimeSchemaKey other )
         if ( compare == 0 )
         {
             compare = Integer.compare( nanoOfSecond, other.nanoOfSecond );
-            if ( compare == 0 && ( zoneId != other.zoneId || zoneOffsetSeconds != other.zoneOffsetSeconds ) )
+            if ( compare == 0 && ( differentValidZoneId( other ) || differentValidZoneOffset( other ) ) )
             {
                 // In the rare case of comparing the same instant in different time zones, we settle for
                 // mapping to values and comparing using the general values comparator.
@@ -112,7 +112,7 @@ public void writeDateTime( long epochSecondUTC, int nano, String zoneId )
     {
         this.epochSecondUTC = epochSecondUTC;
         this.nanoOfSecond = nano;
-        this.zoneId = TimeZoneMapping.map( zoneId );
+        this.zoneId = TimeZones.map( zoneId );
         this.zoneOffsetSeconds = 0;
     }
 
@@ -126,4 +126,17 @@ protected Value assertCorrectType( Value value )
         }
         return value;
     }
+
+    // We need to check validity upfront without throwing exceptions, because the PageCursor might give garbage bytes
+    private boolean differentValidZoneOffset( ZonedDateTimeSchemaKey other )
+    {
+        return zoneOffsetSeconds != other.zoneOffsetSeconds &&
+                TimeZones.validZoneOffset( zoneOffsetSeconds ) && TimeZones.validZoneOffset( other.zoneOffsetSeconds );
+    }
+
+    // We need to check validity upfront without throwing exceptions, because the PageCursor might give garbage bytes
+    private boolean differentValidZoneId( ZonedDateTimeSchemaKey other )
+    {
+        return zoneId != other.zoneId && TimeZones.validZoneId( zoneId ) && TimeZones.validZoneId( other.zoneId );
+    }
 }

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

@@ -21,6 +21,7 @@
 
 import java.time.ZoneOffset;
 
+import org.neo4j.kernel.impl.store.TimeZones;
 import org.neo4j.values.storable.TimeValue;
 import org.neo4j.values.storable.Value;
 import org.neo4j.values.storable.Values;
@@ -66,7 +67,7 @@ public void initValueAsHighest()
     public int compareValueTo( ZonedTimeSchemaKey other )
     {
         int compare = Long.compare( nanosOfDayUTC, other.nanosOfDayUTC );
-        if ( compare == 0 && zoneOffsetSeconds != other.zoneOffsetSeconds )
+        if ( compare == 0 && differentValidZoneOffset( other ) )
         {
             compare = Values.COMPARATOR.compare( asValue(), other.asValue() );
         }
@@ -97,4 +98,11 @@ protected Value assertCorrectType( Value value )
         }
         return value;
     }
+
+    // We need to check validity upfront without throwing exceptions, because the PageCursor might give garbage bytes
+    private boolean differentValidZoneOffset( ZonedTimeSchemaKey other )
+    {
+        return zoneOffsetSeconds != other.zoneOffsetSeconds &&
+                TimeZones.validZoneOffset( zoneOffsetSeconds ) && TimeZones.validZoneOffset( other.zoneOffsetSeconds );
+    }
 }

community/kernel/src/main/java/org/neo4j/kernel/impl/store/TemporalType.java

@@ -251,7 +251,7 @@ public Value decodeForTemporal( long[] valueBlocks, int offset )
                         long epochSecond = valueBlocks[1 + offset];
                         short zoneNumber = (short) valueBlocks[2 + offset];
                         return DateTimeValue.datetime( epochSecond, nanoOfSecond,
-                                ZoneId.of( TimeZoneMapping.map( zoneNumber ) ) );
+                                ZoneId.of( TimeZones.map( zoneNumber ) ) );
                     }
                 }
 
@@ -283,7 +283,7 @@ public ArrayValue decodeArray( Value dataValue )
                             {
                                 short zoneNumber = (short) (zoneValue >>> 1);
                                 dateTimes[i] = ZonedDateTime.ofInstant( Instant.ofEpochSecond( epochSecond, nanos ),
-                                        ZoneId.of( TimeZoneMapping.map( zoneNumber ) ) );
+                                        ZoneId.of( TimeZones.map( zoneNumber ) ) );
                             }
                         }
                         return Values.dateTimeArray( dateTimes );
@@ -478,7 +478,7 @@ public static long[] encodeLocalDateTime( int keyId, long epochSecond, long nano
     public static long[] encodeDateTime( int keyId, long epochSecond, long nanoOfSecond, String zoneId )
     {
         int idBits = StandardFormatSettings.PROPERTY_TOKEN_MAXIMUM_ID_BITS;
-        short zoneNumber = TimeZoneMapping.map( zoneId );
+        short zoneNumber = TimeZones.map( zoneId );
 
         long keyAndType = keyId | (((long) (PropertyType.TEMPORAL.intValue()) << idBits));
         long temporalTypeBits = TemporalType.TEMPORAL_DATE_TIME.temporalType << (idBits + 4);
@@ -629,7 +629,7 @@ public static byte[] encodeDateTimeArray( ZonedDateTime[] dateTimes )
             else
             {
                 String timeZoneId = dateTimes[i].getZone().getId();
-                short zoneNumber = TimeZoneMapping.map( timeZoneId );
+                short zoneNumber = TimeZones.map( timeZoneId );
                 // Set lowest bit to 0 means zone id
                 data[i * BLOCKS_DATETIME + 2] = zoneNumber << 1;
             }

community/kernel/src/main/java/org/neo4j/kernel/impl/store/TimeZoneMapping.java → community/kernel/src/main/java/org/neo4j/kernel/impl/store/TimeZones.java

@@ -32,18 +32,31 @@
 
 import static java.util.Collections.unmodifiableSet;
 
-public class TimeZoneMapping
+public class TimeZones
 {
     /**
      * Prevent instance creation.
      */
-    private TimeZoneMapping()
+    private TimeZones()
     {
     }
 
     private static final List<String> TIME_ZONE_SHORT_TO_STRING = new ArrayList<>( 1024 );
     private static final Map<String,Short> TIME_ZONE_STRING_TO_SHORT = new HashMap<>( 1024 );
 
+    private static final long MIN_ZONE_OFFSET_SECONDS = -18 * 3600;
+    private static final long MAX_ZONE_OFFSET_SECONDS = 18 * 3600;
+
+    public static boolean validZoneOffset( int zoneOffsetSeconds )
+    {
+        return zoneOffsetSeconds >= MIN_ZONE_OFFSET_SECONDS && zoneOffsetSeconds <= MAX_ZONE_OFFSET_SECONDS;
+    }
+
+    public static boolean validZoneId( short zoneId )
+    {
+        return zoneId >= 0 && zoneId < TIME_ZONE_SHORT_TO_STRING.size();
+    }
+
     static final String LATEST_SUPPORTED_IANA_VERSION;
 
     /**
@@ -72,7 +85,7 @@ public static Set<String> supportedTimeZones()
     {
         String latestVersion = "";
         Pattern version = Pattern.compile( "# tzdata([0-9]{4}[a-z])" );
-        try ( BufferedReader reader = new BufferedReader( new InputStreamReader( TimeZoneMapping.class.getResourceAsStream( "/TZIDS" ) ) ) )
+        try ( BufferedReader reader = new BufferedReader( new InputStreamReader( TimeZones.class.getResourceAsStream( "/TZIDS" ) ) ) )
         {
             for ( String line; (line = reader.readLine()) != null; )
             {

community/kernel/src/test/java/org/neo4j/kernel/impl/store/TimeZoneMappingTest.java → community/kernel/src/test/java/org/neo4j/kernel/impl/store/TimeZonesTest.java

@@ -47,23 +47,23 @@
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.fail;
 
-public class TimeZoneMappingTest
+public class TimeZonesTest
 {
     @Test
     public void weSupportAllJavaZoneIds()
     {
         ZoneId.getAvailableZoneIds().forEach( s ->
         {
-            short num = TimeZoneMapping.map( s );
+            short num = TimeZones.map( s );
             assertThat( "Our time zone table does not have a mapping for " + s, num, greaterThanOrEqualTo( (short) 0 ) );
 
-            String nameFromTable = TimeZoneMapping.map( num );
+            String nameFromTable = TimeZones.map( num );
             if ( !s.equals( nameFromTable ) )
             {
                 // The test is running on an older Java version and `s` has been removed since, thus it points to a different zone now.
                 // That zone should point to itself, however.
-                assertThat( "Our time zone table has inconsistent mapping for " + nameFromTable, TimeZoneMapping.map( TimeZoneMapping.map( nameFromTable ) ),
-                        equalTo( nameFromTable ) );
+                assertThat( "Our time zone table has inconsistent mapping for " + nameFromTable,
+                        TimeZones.map( TimeZones.map( nameFromTable ) ), equalTo( nameFromTable ) );
             }
         } );
     }
@@ -73,9 +73,9 @@ public void weSupportDeletedZoneIdEastSaskatchewan()
     {
         try
         {
-            short eastSaskatchewan = TimeZoneMapping.map( "Canada/East-Saskatchewan" );
-            assertThat( "Our time zone table does not remap Canada/East-Saskatchewan to Canada/Saskatchewan", TimeZoneMapping.map( eastSaskatchewan ),
-                    equalTo( "Canada/Saskatchewan" ) );
+            short eastSaskatchewan = TimeZones.map( "Canada/East-Saskatchewan" );
+            assertThat( "Our time zone table does not remap Canada/East-Saskatchewan to Canada/Saskatchewan",
+                    TimeZones.map( eastSaskatchewan ), equalTo( "Canada/Saskatchewan" ) );
         }
         catch ( IllegalArgumentException e )
         {
@@ -94,7 +94,7 @@ public void updateTimeZoneMappings() throws IOException
             {
                 String substring = line.substring( line.indexOf( ' ' ) + 1 );
                 String release = substring.substring( 0, substring.indexOf( ' ' ) );
-                if ( TimeZoneMapping.LATEST_SUPPORTED_IANA_VERSION.equals( release ) )
+                if ( TimeZones.LATEST_SUPPORTED_IANA_VERSION.equals( release ) )
                 {
                     return false; // stop reading
                 }
@@ -122,7 +122,7 @@ public void updateTimeZoneMappings() throws IOException
     @Test
     public void tzidsOrderMustNotChange() throws IOException
     {
-        try ( BufferedReader reader = new BufferedReader( new InputStreamReader( TimeZoneMapping.class.getResourceAsStream( "/TZIDS" ) ) ) )
+        try ( BufferedReader reader = new BufferedReader( new InputStreamReader( TimeZones.class.getResourceAsStream( "/TZIDS" ) ) ) )
         {
             String text = reader.lines().collect( Collectors.joining( "\n" ) );
             MessageDigest digest = MessageDigest.getInstance( "SHA-256" );
@@ -155,7 +155,7 @@ private void upgradeIANATo( String release, Set<String> upgradedAlready, StringB
         } );
 
         // TODO assertion for removals
-        Set<String> neo4jSupportedTzs = TimeZoneMapping.supportedTimeZones();
+        Set<String> neo4jSupportedTzs = TimeZones.supportedTimeZones();
         Set<String> removedTzs = new HashSet<>( neo4jSupportedTzs );
         removedTzs.removeAll( ianaSupportedTzs );
         //assertThat( "There were removals from the IANA database. Please upgrade manually.", removedTzs, empty() );

community/values/src/main/java/org/neo4j/values/storable/TemporalValue.java

@@ -69,8 +69,6 @@ public abstract class TemporalValue<T extends Temporal, V extends TemporalValue<
         // (therefore the type itself is public)
     }
 
-    private static final String DEFAULT_WHEN = "statement";
-
     public abstract TemporalValue add( DurationValue duration );
 
     public abstract TemporalValue sub( DurationValue duration );