From 0fbbe195bb3b63665f3ecbc93ba2f51c2b771296 Mon Sep 17 00:00:00 2001
From: Wenchen Fan <cloud0fan@outlook.com>
Date: Sat, 11 Jul 2015 16:53:44 +0800
Subject: [PATCH] address comments

---
 .../spark/sql/catalyst/expressions/Cast.scala |  4 ++
 .../apache/spark/unsafe/types/Interval.java   | 10 +++++
 .../spark/unsafe/types/IntervalSuite.java     | 37 ++++++++++++++-----
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
index 88254e24cb693..fe2818d8802b3 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala
@@ -453,6 +453,10 @@ case class Cast(child: Expression, dataType: DataType) extends UnaryExpression w
       case (_, StringType) =>
         defineCodeGen(ctx, ev, c => s"${ctx.stringType}.fromString(String.valueOf($c))")
 
+      case (StringType, IntervalType) =>
+        defineCodeGen(ctx, ev, c =>
+          s"org.apache.spark.unsafe.types.Interval.fromString($c.toString)")
+
       // fallback for DecimalType, this must be before other numeric types
       case (_, dt: DecimalType) =>
         super.genCode(ctx, ev)
diff --git a/unsafe/src/main/java/org/apache/spark/unsafe/types/Interval.java b/unsafe/src/main/java/org/apache/spark/unsafe/types/Interval.java
index 834d8a04810cd..eb7475e9df869 100644
--- a/unsafe/src/main/java/org/apache/spark/unsafe/types/Interval.java
+++ b/unsafe/src/main/java/org/apache/spark/unsafe/types/Interval.java
@@ -32,6 +32,16 @@ public final class Interval implements Serializable {
   public static final long MICROS_PER_DAY = MICROS_PER_HOUR * 24;
   public static final long MICROS_PER_WEEK = MICROS_PER_DAY * 7;
 
+  /**
+   * A function to generate regex which matches interval string's unit part like "3 years".
+   *
+   * First, we can leave out some units in interval string, and we only care about the value of
+   * unit, so here we use non-capturing group to wrap the actual regex.
+   * At the beginning of the actual regex, we should match spaces before the unit part.
+   * Next is the number part, starts with an optional "-" to represent negative value. We use
+   * capturing group to wrap this part as we need the value later.
+   * Finally is the unit name, ends with an optional "s".
+   */
   private static String unitRegex(String unit) {
     return "(?:\\s+(-?\\d+)\\s+" + unit + "s?)?";
   }
diff --git a/unsafe/src/test/java/org/apache/spark/unsafe/types/IntervalSuite.java b/unsafe/src/test/java/org/apache/spark/unsafe/types/IntervalSuite.java
index de24a94e21f59..25e3651dda332 100644
--- a/unsafe/src/test/java/org/apache/spark/unsafe/types/IntervalSuite.java
+++ b/unsafe/src/test/java/org/apache/spark/unsafe/types/IntervalSuite.java
@@ -59,31 +59,50 @@ public void toStringTest() {
 
   @Test
   public void fromStringTest() {
+    testSingleUnit("year", 3, 36, 0);
+    testSingleUnit("month", 3, 3, 0);
+    testSingleUnit("week", 3, 0, 3 * MICROS_PER_WEEK);
+    testSingleUnit("day", 3, 0, 3 * MICROS_PER_DAY);
+    testSingleUnit("hour", 3, 0, 3 * MICROS_PER_HOUR);
+    testSingleUnit("minute", 3, 0, 3 * MICROS_PER_MINUTE);
+    testSingleUnit("second", 3, 0, 3 * MICROS_PER_SECOND);
+    testSingleUnit("millisecond", 3, 0, 3 * MICROS_PER_MILLI);
+    testSingleUnit("microsecond", 3, 0, 3);
+
     String s;
     Interval i;
 
-    s = "interval  2 weeks -6 minute";
-    i = new Interval(0, 2 * MICROS_PER_WEEK - 6 * MICROS_PER_MINUTE);
-    assertEquals(Interval.fromString(s), i);
-
-    s = "interval   -5 years 23 month";
+    s = "interval   -5  years  23   month";
     i = new Interval(-5 * 12 + 23, 0);
     assertEquals(Interval.fromString(s), i);
 
-    s = "interval   3month 1 hour";
+    // Error cases
     i = null;
+
+    s = "interval   3month 1 hour";
     assertEquals(Interval.fromString(s), i);
 
     s = "interval 3 moth 1 hour";
-    i = null;
     assertEquals(Interval.fromString(s), i);
 
     s = "interval";
-    i = null;
+    assertEquals(Interval.fromString(s), i);
+
+    s = "int";
+    assertEquals(Interval.fromString(s), i);
+
+    s = "";
     assertEquals(Interval.fromString(s), i);
 
     s = null;
-    i = null;
     assertEquals(Interval.fromString(s), i);
   }
+
+  private void testSingleUnit(String unit, int number, int months, long microseconds) {
+    String s1 = "interval " + number + " " + unit;
+    String s2 = "interval " + number + " " + unit + "s";
+    Interval i = new Interval(months, microseconds);
+    assertEquals(Interval.fromString(s1), i);
+    assertEquals(Interval.fromString(s2), i);
+  }
 }