do not allow leading 'interval' in the interval literal string

Author: cloud-fan

common/unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java

@@ -81,7 +81,7 @@ public int hashCode() {
 
   @Override
   public String toString() {
-    StringBuilder sb = new StringBuilder("interval");
+    StringBuilder sb = new StringBuilder();
 
     if (months != 0) {
       appendUnit(sb, months / 12, "year");
@@ -98,18 +98,19 @@ public String toString() {
       rest %= MICROS_PER_MINUTE;
       if (rest != 0) {
         String s = BigDecimal.valueOf(rest, 6).stripTrailingZeros().toPlainString();
-        sb.append(' ').append(s).append(" seconds");
+        sb.append(s).append(" seconds ");
       }
     } else if (months == 0 && days == 0) {
-      sb.append(" 0 microseconds");
+      sb.append("0 seconds ");
     }
 
+    sb.setLength(sb.length() - 1);
     return sb.toString();
   }
 
   private void appendUnit(StringBuilder sb, long value, String unit) {
     if (value != 0) {
-      sb.append(' ').append(value).append(' ').append(unit).append('s');
+      sb.append(value).append(' ').append(unit).append('s').append(' ');
     }
   }
 }

docs/sql-migration-guide.md

@@ -220,6 +220,8 @@ license: |
 
   - Since Spark 3.0, the interval literal syntax does not allow multiple from-to units anymore. For example, `SELECT INTERVAL '1-1' YEAR TO MONTH '2-2' YEAR TO MONTH'` throws parser exception.
 
+  - Since Spark 3.0, when casting string to interval type, strings with leading "interval" such as "interval 1 day" will be treated as invalid and the cast returns null. In Spark version 2.4 and earlier, the leading "interval" is allowed and required. To allow the leading "interval", you can set `spark.sql.legacy.allowIntervalPrefixInCast` to true.
+
 ## Upgrading from Spark SQL 2.4 to 2.4.1
 
   - The value of `spark.executor.heartbeatInterval`, when specified without units like "30" rather than "30s", was

sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4

@@ -80,7 +80,7 @@ singleTableSchema
     ;
 
 singleInterval
-    : INTERVAL? multiUnitsInterval EOF
+    : multiUnitsInterval EOF
     ;
 
 statement

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala

@@ -465,6 +465,9 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
 
   // IntervalConverter
   private[this] def castToInterval(from: DataType): Any => Any = from match {
+    case StringType if SQLConf.get.getConf(SQLConf.LEGACY_ALLOW_INTERVAL_PREFIX_IN_CAST) =>
+      buildCast[UTF8String](_, s => IntervalUtils.legacyCastStringToInterval(s.toString))
+
     case StringType =>
       buildCast[UTF8String](_, s => IntervalUtils.safeFromString(s.toString))
   }
@@ -1214,10 +1217,15 @@ abstract class CastBase extends UnaryExpression with TimeZoneAwareExpression wit
   private[this] def castToIntervalCode(from: DataType): CastFunction = from match {
     case StringType =>
       val util = IntervalUtils.getClass.getCanonicalName.stripSuffix("$")
+      val func = if (SQLConf.get.getConf(SQLConf.LEGACY_ALLOW_INTERVAL_PREFIX_IN_CAST)) {
+        "legacyCastStringToInterval"
+      } else {
+        "safeFromString"
+      }
       (c, evPrim, evNull) =>
-        code"""$evPrim = $util.safeFromString($c.toString());
-           if(${evPrim} == null) {
-             ${evNull} = true;
+        code"""$evPrim = $util.$func($c.toString());
+           if($evPrim == null) {
+             $evNull = true;
            }
          """.stripMargin
 

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala

@@ -365,4 +365,13 @@ object IntervalUtils {
   def isNegative(interval: CalendarInterval, daysPerMonth: Int = 31): Boolean = {
     getDuration(interval, TimeUnit.MICROSECONDS, daysPerMonth) < 0
   }
+
+  def legacyCastStringToInterval(str: String): CalendarInterval = {
+    val trimmed = str.trim
+    if (trimmed.regionMatches(true, 0, "interval", 0, 8)) {
+      safeFromString(trimmed.drop(8))
+    } else {
+      safeFromString(trimmed)
+    }
+  }
 }

sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala

@@ -2042,6 +2042,14 @@ object SQLConf {
       .booleanConf
       .createWithDefault(true)
 
+  val LEGACY_ALLOW_INTERVAL_PREFIX_IN_CAST =
+    buildConf("spark.sql.legacy.allowIntervalPrefixInCast")
+      .doc("When true, it's allowed to have the 'interval' prefix in the string that is being " +
+        "casted to interval type. For example, `CAST('interval 1 day' AS INTERVAL)` returns " +
+        "null if this config is set to false.")
+      .booleanConf
+      .createWithDefault(false)
+
   val ADDITIONAL_REMOTE_REPOSITORIES =
     buildConf("spark.sql.additionalRemoteRepositories")
       .doc("A comma-delimited string config of the optional additional remote Maven mirror " +

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala

@@ -664,14 +664,21 @@ abstract class CastSuiteBase extends SparkFunSuite with ExpressionEvalHelper {
     import org.apache.spark.unsafe.types.CalendarInterval
 
     checkEvaluation(Cast(Literal(""), CalendarIntervalType), null)
-    checkEvaluation(Cast(Literal("interval -3 month 1 day 7 hours"), CalendarIntervalType),
+    checkEvaluation(Cast(Literal("-3 month 1 day 7 hours"), CalendarIntervalType),
       new CalendarInterval(-3, 1, 7 * CalendarInterval.MICROS_PER_HOUR))
+    checkEvaluation(Cast(Literal("interval -3 month 1 day 7 hours"), CalendarIntervalType), null)
+
+    withSQLConf(SQLConf.LEGACY_ALLOW_INTERVAL_PREFIX_IN_CAST.key -> "true") {
+      checkEvaluation(Cast(Literal("interval -3 month 1 day 7 hours"), CalendarIntervalType),
+        new CalendarInterval(-3, 1, 7 * CalendarInterval.MICROS_PER_HOUR))
+      checkEvaluation(Cast(Literal("INTERVAL 1 Second 1 microsecond"), CalendarIntervalType),
+        new CalendarInterval(0, 0, 1000001))
+    }
+
     checkEvaluation(Cast(Literal.create(
       new CalendarInterval(15, 9, -3 * CalendarInterval.MICROS_PER_HOUR), CalendarIntervalType),
       StringType),
-      "interval 1 years 3 months 9 days -3 hours")
-    checkEvaluation(Cast(Literal("INTERVAL 1 Second 1 microsecond"), CalendarIntervalType),
-      new CalendarInterval(0, 0, 1000001))
+      "1 years 3 months 9 days -3 hours")
     checkEvaluation(Cast(Literal("1 MONTH 1 Microsecond"), CalendarIntervalType),
       new CalendarInterval(1, 0, 1))
   }

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala

@@ -50,7 +50,7 @@ class IntervalUtilsSuite extends SparkFunSuite {
       }
     }
 
-    for (input <- Seq("interval", "interval1 day", "foo", "foo 1 day")) {
+    for (input <- Seq("interval", "interval 1 day", "interval1 day", "foo", "foo 1 day")) {
       try {
         fromString(input)
         fail("Expected to throw an exception for the invalid input")
@@ -82,13 +82,11 @@ class IntervalUtilsSuite extends SparkFunSuite {
 
   private def testSingleUnit(
       unit: String, number: Int, months: Int, days: Int, microseconds: Long): Unit = {
-    for (prefix <- Seq("interval ", "")) {
-      val input1 = prefix + number + " " + unit
-      val input2 = prefix + number + " " + unit + "s"
-      val result = new CalendarInterval(months, days, microseconds)
-      assert(fromString(input1) == result)
-      assert(fromString(input2) == result)
-    }
+    val input1 = number + " " + unit
+    val input2 = number + " " + unit + "s"
+    val result = new CalendarInterval(months, days, microseconds)
+    assert(fromString(input1) == result)
+    assert(fromString(input2) == result)
   }
 
   test("from year-month string") {
@@ -186,4 +184,11 @@ class IntervalUtilsSuite extends SparkFunSuite {
     assert(!isNegative("1 year -360 days", 31))
     assert(!isNegative("-1 year 380 days", 31))
   }
+
+  test("legacyCastStringToInterval") {
+    for (input <- Seq("inTERval 1 year", "1 year")) {
+      val result = new CalendarInterval(12, 0, 0)
+      assert(IntervalUtils.legacyCastStringToInterval(input) == result)
+    }
+  }
 }

sql/core/src/test/resources/sql-tests/inputs/cast.sql

@@ -58,5 +58,5 @@ DESC FUNCTION EXTENDED boolean;
 -- TODO: migrate all cast tests here.
 
 -- cast string to interval and interval to string
-SELECT CAST('interval 3 month 1 hour' AS interval);
+SELECT CAST('3 month 1 hour' AS interval);
 SELECT CAST(interval 3 month 1 hour AS string);

sql/core/src/test/resources/sql-tests/inputs/literals.sql

@@ -123,7 +123,6 @@ SELECT 3.14, -3.14, 3.14e8, 3.14e-8, -3.14e8, -3.14e-8, 3.14e+8, 3.14E8, 3.14E-8
 select map(1, interval 1 day, 2, interval 3 week);
 
 -- typed interval expression
-select interval 'interval 3 year 1 hour';
 select interval '3 year 1 hour';
 
 -- typed integer expression
@@ -133,6 +132,7 @@ select integer '2147483648';
 
 -- malformed interval literal
 select interval;
+select interval 'interval 3 year 1 hour';
 select interval 1 fake_unit;
 select interval 1 year to month;
 select interval '1' year to second;