Fix comments from PR review.

neo4j · Mar 3, 2018 · 49d707f · 49d707f
1 parent 1716c84
commit 49d707f
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 31 deletions.
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
@@ -218,7 +218,7 @@ else if ( selectingTime || selectingDate )
                         AnyValue timeField = fields.get( Field.time );
                         if ( !(timeField instanceof TemporalValue) )
                         {
-                            throw new IllegalArgumentException( String.format( "Cannot construct local time from: %s", timeField ) );
+                            throw new IllegalArgumentException( String.format( "Cannot construct time from: %s", timeField ) );
                         }
                         TemporalValue t = (TemporalValue) timeField;
                         time = t.getTimePart( defaultZone ).toLocalTime();

diff --git a/community/values/src/main/java/org/neo4j/values/storable/DurationValue.java b/community/values/src/main/java/org/neo4j/values/storable/DurationValue.java
@@ -53,7 +53,6 @@
 import static java.util.Collections.unmodifiableList;
 import static java.util.Objects.requireNonNull;
 import static java.util.regex.Pattern.CASE_INSENSITIVE;
-import static org.neo4j.values.storable.IntegralValue.safeCastIntegral;
 import static org.neo4j.values.storable.NumberType.NO_NUMBER;
 import static org.neo4j.values.storable.NumberValue.safeCastFloatingPoint;
 

diff --git a/community/values/src/main/java/org/neo4j/values/storable/TemporalValue.java b/community/values/src/main/java/org/neo4j/values/storable/TemporalValue.java
@@ -634,7 +634,15 @@ else if ( field == Field.time || field == Field.date )
             {
                 return new SelectDateOrTimeDTBuilder( date, time ).assign( field, value );
             }
-            else if ( field.field.isDateBased() )
+            else
+            {
+                return assignNonComposite( field, value );
+            }
+        }
+
+        DateTimeBuilder assignNonComposite( Field field, AnyValue value )
+        {
+            if ( field.field.isDateBased() )
             {
                 if ( date == null )
                 {
@@ -678,23 +686,10 @@ else if ( field == Field.datetime )
             {
                 throw new IllegalArgumentException( "cannot re-assign " + field );
             }
-            else if ( field.field.isDateBased() )
-            {
-                if ( date == null )
-                {
-                    date = new ConstructDate();
-                }
-                date = date.assign( field, value );
-            }
             else
             {
-                if ( time == null )
-                {
-                    time = new ConstructTime();
-                }
-                time.assign( field, value );
+                return assignNonComposite( field, value );
             }
-            return this;
         }
     }
 
@@ -712,23 +707,10 @@ DateTimeBuilder assign( Field field, AnyValue value )
             {
                 throw new IllegalArgumentException( field.name() + " cannot be selected together with date or time." );
             }
-            else if ( field != Field.time && (field == Field.date || field.field.isDateBased()) )
-            {
-                if ( date == null )
-                {
-                    date = new ConstructDate();
-                }
-                date = date.assign( field, value );
-            }
             else
             {
-                if ( time == null )
-                {
-                    time = new ConstructTime();
-                }
-                time.assign( field, value );
+                return assignNonComposite( field, value );
             }
-            return this;
         }
     }
 

diff --git a/...ec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/TemporalAcceptanceTest.scala b/...ec-suite/src/test/scala/org/neo4j/internal/cypher/acceptance/TemporalAcceptanceTest.scala
@@ -19,6 +19,8 @@
  */
 package org.neo4j.internal.cypher.acceptance
 
+import java.time.format.DateTimeParseException
+
 import org.neo4j.cypher._
 import org.neo4j.graphdb.QueryExecutionException
 
@@ -422,6 +424,18 @@ class TemporalAcceptanceTest extends ExecutionEngineFunSuite with QueryStatistic
     }
   }
 
+  // Parsing
+
+  test("should not allow decimals on any but the least significant given value") {
+    for (arg <- Seq("P1.5Y1M", "P1Y1.5M1D", "P1Y1M1.5DT1H", "P1Y1M1DT1.5H1M", "P1Y1M1DT1H1.5M1S")) {
+      val query = s"RETURN duration('$arg')"
+      val exception = intercept[DateTimeParseException] {
+        println(graph.execute(query).next())
+      }
+      exception.getMessage should be ("Text cannot be parsed to a Duration")
+    }
+  }
+
   private def shouldNotTruncate(receivers: Seq[String], truncationUnit: String, args: Seq[String]): Unit = {
     for (receiver <- receivers; arg <- args) {
       val query = s"RETURN $receiver.truncate('$truncationUnit', $arg)"