DateTime sorting of offsets and named zones

Always sort DateTimes with a offset before one with a named zone if they have the same instant and local time.
neo4j · Mar 16, 2018 · 5d20173 · 5d20173
1 parent 903afd0
commit 5d20173
Show file tree

Hide file tree

Showing 3 changed files with 8 additions and 11 deletions.
diff --git a/...unity/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java b/...unity/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/ZonedDateTimeSchemaKey.java
@@ -82,7 +82,7 @@ public int compareValueTo( ZonedDateTimeSchemaKey other )
         if ( compare == 0 )
         {
             compare = Integer.compare( nanoOfSecond, other.nanoOfSecond );
-            if ( compare == 0 && ( differentValidZoneId( other ) || differentValidZoneOffset( other ) ) )
+            if ( compare == 0 && hasValidTimeZone() && other.hasValidTimeZone() )
             {
                 // 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.
@@ -129,15 +129,8 @@ protected Value assertCorrectType( Value value )
     }
 
     // We need to check validity upfront without throwing exceptions, because the PageCursor might give garbage bytes
-    private boolean differentValidZoneOffset( ZonedDateTimeSchemaKey other )
+    private boolean hasValidTimeZone()
     {
-        return zoneOffsetMinutes != other.zoneOffsetMinutes &&
-                TimeZones.validZoneOffset( zoneOffsetMinutes * 60 ) && TimeZones.validZoneOffset( other.zoneOffsetMinutes * 60 );
-    }
-
-    // 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 );
+        return TimeZones.validZoneId( zoneId ) || TimeZones.validZoneOffset( zoneOffsetMinutes * 60 );
     }
 }
diff --git a/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java b/community/values/src/main/java/org/neo4j/values/storable/DateTimeValue.java
@@ -449,7 +449,10 @@ int unsafeCompareTo( Value other )
                 {
                     ZoneId thisZone = value.getZone();
                     ZoneId thatZone = that.getZone();
-                    if ( areDifferentZones( thisZone, thatZone ) )
+                    boolean thisIsOffset = thisZone instanceof ZoneOffset;
+                    boolean thatIsOffset = thatZone instanceof ZoneOffset;
+                    cmp = Boolean.compare( thatIsOffset, thisIsOffset ); // offsets before named zones
+                    if ( cmp == 0 && areDifferentZones( thisZone, thatZone ) )
                     {
                         cmp = thisZone.getId().compareTo( thatZone.getId() );
                     }

diff --git a/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java b/community/values/src/test/java/org/neo4j/values/storable/ValueComparisonTest.java
@@ -104,6 +104,7 @@ public class ValueComparisonTest
             // DateTime and the likes
             datetime(2018, 2, 2, 0, 0, 0, 0, "+00:00"),
             datetime(2018, 2, 2, 1, 30, 0, 0, "+01:00"),
+            datetime(2018, 2, 2, 1, 30, 0, 0, "Europe/Stockholm"), // same offset as +01:00, but name zones come after offsets
             datetime(2018, 2, 2, 1, 0, 0, 0, "+00:00"),
             datetime(2018, 3, 2, 1, 0, 0, 0, "Europe/Berlin"),
             datetime(2018, 3, 2, 1, 0, 0, 0, "Europe/Stockholm"), // same offset as Europe/Berlin, so compared by zone name