From 3b2bd489612cc7da4efb2c068507cc44e4e2408d Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Fri, 1 Nov 2019 13:09:07 +0300 Subject: [PATCH 1/7] Add tests --- .../sql/catalyst/util/IntervalUtilsSuite.scala | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala index 22944035f31db..77fa31a5568e7 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala @@ -184,4 +184,20 @@ class IntervalUtilsSuite extends SparkFunSuite { assert(!isNegative("1 year -360 days", 31)) assert(!isNegative("-1 year 380 days", 31)) } + + test("from bounded day-time string") { + val input = "5 12:40:30.999999999" + + def check(from: String, to: String, expected: String): Unit = { + withClue(s"from = $from, to = $to") { + assert(fromDayTimeString(input, from, to) === fromString(expected)) + } + } + + check("hour", "minute", "12 hours 40 minutes") + check("hour", "second", "12 hours 40 minutes 30.999999 seconds") + check("minute", "second", "40 minutes 30.999999 seconds") + check("day", "hour", "5 days 12 hours") + check("day", "minute", "5 days 12 hours 40 minutes") + } } From 34ebb1b28ec2c68f145f8a66fa499b78b4b0a882 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Fri, 1 Nov 2019 13:31:39 +0300 Subject: [PATCH 2/7] Fix --- .../sql/catalyst/util/IntervalUtils.scala | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala index 90e2402a5d7da..45be193c5d5b9 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala @@ -198,7 +198,7 @@ object IntervalUtils { try { val sign = if (m.group(1) != null && m.group(1) == "-") -1 else 1 - val days = if (m.group(2) == null) { + var days = if (m.group(2) == null) { 0 } else { toLongWithRange("day", m.group(3), 0, Integer.MAX_VALUE) @@ -219,16 +219,24 @@ object IntervalUtils { } // Hive allow nanosecond precision interval var secondsFraction = parseNanos(m.group(9), seconds < 0) - to match { - case "hour" => + (from, to) match { + case ("day", "second") => // No-op + case ("day", "minute") => + seconds = 0 + secondsFraction = 0 + case ("day", "hour") => minutes = 0 seconds = 0 secondsFraction = 0 - case "minute" => + case ("hour", "second") => + days = 0 + case ("hour", "minute") => + days = 0 seconds = 0 secondsFraction = 0 - case "second" => - // No-op + case ("minute", "second") => + days = 0 + hours = 0 case _ => throw new IllegalArgumentException( s"Cannot support (interval '$input' $from to $to) expression") From 4edf3118ce55108cc6bae3949a76d9a52c3fbc9f Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Fri, 1 Nov 2019 13:45:10 +0300 Subject: [PATCH 3/7] Regenerate literals.sql.out --- .../src/test/resources/sql-tests/results/literals.sql.out | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/core/src/test/resources/sql-tests/results/literals.sql.out b/sql/core/src/test/resources/sql-tests/results/literals.sql.out index 550b9bd936a05..7bf530b18cb92 100644 --- a/sql/core/src/test/resources/sql-tests/results/literals.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/literals.sql.out @@ -419,9 +419,9 @@ interval 15 hours 40 minutes 32 seconds 998 milliseconds 999 microseconds -- !query 46 select interval '20 40:32.99899999' minute to second -- !query 46 schema -struct +struct -- !query 46 output -interval 2 weeks 6 days 40 minutes 32 seconds 998 milliseconds 999 microseconds +interval 40 minutes 32 seconds 998 milliseconds 999 microseconds -- !query 47 From d7bc01c5b632ebaf3616f8955a3492bbf4bf7464 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Fri, 1 Nov 2019 13:48:00 +0300 Subject: [PATCH 4/7] Regenerate interval.sql.out --- .../results/postgreSQL/interval.sql.out | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out b/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out index bed5d7a56c1f8..7b210e3b7c715 100644 --- a/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out @@ -149,46 +149,46 @@ interval 1 days 2 hours 3 minutes 4 seconds -- !query 18 SELECT interval '1 2:03' hour to minute -- !query 18 schema -struct +struct -- !query 18 output -interval 1 days 2 hours 3 minutes +interval 2 hours 3 minutes -- !query 19 SELECT interval '1 2:03:04' hour to minute -- !query 19 schema -struct +struct -- !query 19 output -interval 1 days 2 hours 3 minutes +interval 2 hours 3 minutes -- !query 20 SELECT interval '1 2:03' hour to second -- !query 20 schema -struct +struct -- !query 20 output -interval 1 days 2 hours 3 minutes +interval 2 hours 3 minutes -- !query 21 SELECT interval '1 2:03:04' hour to second -- !query 21 schema -struct +struct -- !query 21 output -interval 1 days 2 hours 3 minutes 4 seconds +interval 2 hours 3 minutes 4 seconds -- !query 22 SELECT interval '1 2:03' minute to second -- !query 22 schema -struct +struct -- !query 22 output -interval 1 days 2 minutes 3 seconds +interval 2 minutes 3 seconds -- !query 23 SELECT interval '1 2:03:04' minute to second -- !query 23 schema -struct +struct -- !query 23 output -interval 1 days 2 hours 3 minutes 4 seconds +interval 3 minutes 4 seconds From a5e60f69738728edb98015cdca47ed9a98eda7c7 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Sat, 2 Nov 2019 11:23:10 +0300 Subject: [PATCH 5/7] Put checking under the dialect config --- .../sql/catalyst/util/IntervalUtils.scala | 12 ++++++---- .../catalyst/util/IntervalUtilsSuite.scala | 24 ++++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala index 45be193c5d5b9..39ea88cb498f0 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala @@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit import scala.util.control.NonFatal import org.apache.spark.sql.catalyst.parser.{CatalystSqlParser, ParseException} +import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.types.Decimal import org.apache.spark.unsafe.types.CalendarInterval @@ -219,6 +220,7 @@ object IntervalUtils { } // Hive allow nanosecond precision interval var secondsFraction = parseNanos(m.group(9), seconds < 0) + val resetValuesUpToFrom = !SQLConf.get.usePostgreSQLDialect (from, to) match { case ("day", "second") => // No-op case ("day", "minute") => @@ -229,14 +231,16 @@ object IntervalUtils { seconds = 0 secondsFraction = 0 case ("hour", "second") => - days = 0 + if (resetValuesUpToFrom) days = 0 case ("hour", "minute") => - days = 0 + if (resetValuesUpToFrom) days = 0 seconds = 0 secondsFraction = 0 case ("minute", "second") => - days = 0 - hours = 0 + if (resetValuesUpToFrom) { + days = 0 + hours = 0 + } case _ => throw new IllegalArgumentException( s"Cannot support (interval '$input' $from to $to) expression") diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala index 77fa31a5568e7..50e3156ec0d98 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala @@ -20,11 +20,13 @@ package org.apache.spark.sql.catalyst.util import java.util.concurrent.TimeUnit import org.apache.spark.SparkFunSuite +import org.apache.spark.sql.catalyst.plans.SQLHelper import org.apache.spark.sql.catalyst.util.IntervalUtils.{fromDayTimeString, fromString, fromYearMonthString} +import org.apache.spark.sql.internal.SQLConf import org.apache.spark.unsafe.types.CalendarInterval import org.apache.spark.unsafe.types.CalendarInterval._ -class IntervalUtilsSuite extends SparkFunSuite { +class IntervalUtilsSuite extends SparkFunSuite with SQLHelper { test("fromString: basic") { testSingleUnit("YEAR", 3, 36, 0) @@ -194,10 +196,20 @@ class IntervalUtilsSuite extends SparkFunSuite { } } - check("hour", "minute", "12 hours 40 minutes") - check("hour", "second", "12 hours 40 minutes 30.999999 seconds") - check("minute", "second", "40 minutes 30.999999 seconds") - check("day", "hour", "5 days 12 hours") - check("day", "minute", "5 days 12 hours 40 minutes") + withSQLConf(SQLConf.DIALECT.key -> "Spark") { + check("hour", "minute", "12 hours 40 minutes") + check("hour", "second", "12 hours 40 minutes 30.999999 seconds") + check("minute", "second", "40 minutes 30.999999 seconds") + check("day", "hour", "5 days 12 hours") + check("day", "minute", "5 days 12 hours 40 minutes") + } + + withSQLConf(SQLConf.DIALECT.key -> "PostgreSQL") { + check("hour", "minute", "5 days 12 hours 40 minutes") + check("hour", "second", "5 days 12 hours 40 minutes 30.999999 seconds") + check("minute", "second", "5 days 12 hours 40 minutes 30.999999 seconds") + check("day", "hour", "5 days 12 hours") + check("day", "minute", "5 days 12 hours 40 minutes") + } } } From e8607720b64570e232aea01b7714a66e129ed208 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Sat, 2 Nov 2019 11:28:29 +0300 Subject: [PATCH 6/7] Regenerate interval.sql.out --- .../results/postgreSQL/interval.sql.out | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out b/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out index 7b210e3b7c715..bed5d7a56c1f8 100644 --- a/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/postgreSQL/interval.sql.out @@ -149,46 +149,46 @@ interval 1 days 2 hours 3 minutes 4 seconds -- !query 18 SELECT interval '1 2:03' hour to minute -- !query 18 schema -struct +struct -- !query 18 output -interval 2 hours 3 minutes +interval 1 days 2 hours 3 minutes -- !query 19 SELECT interval '1 2:03:04' hour to minute -- !query 19 schema -struct +struct -- !query 19 output -interval 2 hours 3 minutes +interval 1 days 2 hours 3 minutes -- !query 20 SELECT interval '1 2:03' hour to second -- !query 20 schema -struct +struct -- !query 20 output -interval 2 hours 3 minutes +interval 1 days 2 hours 3 minutes -- !query 21 SELECT interval '1 2:03:04' hour to second -- !query 21 schema -struct +struct -- !query 21 output -interval 2 hours 3 minutes 4 seconds +interval 1 days 2 hours 3 minutes 4 seconds -- !query 22 SELECT interval '1 2:03' minute to second -- !query 22 schema -struct +struct -- !query 22 output -interval 2 minutes 3 seconds +interval 1 days 2 minutes 3 seconds -- !query 23 SELECT interval '1 2:03:04' minute to second -- !query 23 schema -struct +struct -- !query 23 output -interval 3 minutes 4 seconds +interval 1 days 2 hours 3 minutes 4 seconds From 296de1f26d4eee4d006364584db7124d65ede753 Mon Sep 17 00:00:00 2001 From: Maxim Gekk Date: Mon, 11 Nov 2019 11:08:45 +0300 Subject: [PATCH 7/7] Merge remote-tracking branch 'remotes/origin/master' into fix-from-day-time-string # Conflicts: # sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala # sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala --- .../sql/catalyst/util/IntervalUtils.scala | 12 +++++----- .../catalyst/util/IntervalUtilsSuite.scala | 22 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala index 67d55147afb67..ae676d999f6fb 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/IntervalUtils.scala @@ -227,21 +227,21 @@ object IntervalUtils { var secondsFraction = parseNanos(m.group(9), seconds < 0) val resetValuesUpToFrom = !SQLConf.get.usePostgreSQLDialect (from, to) match { - case ("day", "second") => // No-op - case ("day", "minute") => + case (DAY, SECOND) => // No-op + case (DAY, MINUTE) => seconds = 0 secondsFraction = 0 - case ("day", "hour") => + case (DAY, HOUR) => minutes = 0 seconds = 0 secondsFraction = 0 - case ("hour", "second") => + case (HOUR, SECOND) => if (resetValuesUpToFrom) days = 0 - case ("hour", "minute") => + case (HOUR, MINUTE) => if (resetValuesUpToFrom) days = 0 seconds = 0 secondsFraction = 0 - case ("minute", "second") => + case (MINUTE, SECOND) => if (resetValuesUpToFrom) { days = 0 hours = 0 diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala index 44e84e2bededa..7ba4e3f1f8389 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/util/IntervalUtilsSuite.scala @@ -271,26 +271,26 @@ class IntervalUtilsSuite extends SparkFunSuite with SQLHelper { test("from bounded day-time string") { val input = "5 12:40:30.999999999" - def check(from: String, to: String, expected: String): Unit = { + def check(from: IntervalUnit, to: IntervalUnit, expected: String): Unit = { withClue(s"from = $from, to = $to") { assert(fromDayTimeString(input, from, to) === fromString(expected)) } } withSQLConf(SQLConf.DIALECT.key -> "Spark") { - check("hour", "minute", "12 hours 40 minutes") - check("hour", "second", "12 hours 40 minutes 30.999999 seconds") - check("minute", "second", "40 minutes 30.999999 seconds") - check("day", "hour", "5 days 12 hours") - check("day", "minute", "5 days 12 hours 40 minutes") + check(HOUR, MINUTE, "12 hours 40 minutes") + check(HOUR, SECOND, "12 hours 40 minutes 30.999999 seconds") + check(MINUTE, SECOND, "40 minutes 30.999999 seconds") + check(DAY, HOUR, "5 days 12 hours") + check(DAY, MINUTE, "5 days 12 hours 40 minutes") } withSQLConf(SQLConf.DIALECT.key -> "PostgreSQL") { - check("hour", "minute", "5 days 12 hours 40 minutes") - check("hour", "second", "5 days 12 hours 40 minutes 30.999999 seconds") - check("minute", "second", "5 days 12 hours 40 minutes 30.999999 seconds") - check("day", "hour", "5 days 12 hours") - check("day", "minute", "5 days 12 hours 40 minutes") + check(HOUR, MINUTE, "5 days 12 hours 40 minutes") + check(HOUR, SECOND, "5 days 12 hours 40 minutes 30.999999 seconds") + check(MINUTE, SECOND, "5 days 12 hours 40 minutes 30.999999 seconds") + check(DAY, HOUR, "5 days 12 hours") + check(DAY, MINUTE, "5 days 12 hours 40 minutes") } } }