This is an automated email from the ASF dual-hosted git repository. gengliang pushed a commit to branch branch-3.2 in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-3.2 by this push: new 3d69d0d [SPARK-36428][SQL][FOLLOWUP] Simplify the implementation of make_timestamp 3d69d0d is described below commit 3d69d0d0038a1065bb5d24430bf30da9a3463184 Author: gengjiaan <gengji...@360.cn> AuthorDate: Wed Aug 18 22:57:06 2021 +0800 [SPARK-36428][SQL][FOLLOWUP] Simplify the implementation of make_timestamp ### What changes were proposed in this pull request? The implement of https://github.com/apache/spark/pull/33665 make `make_timestamp` could accepts integer type as the seconds parameter. This PR let `make_timestamp` accepts `decimal(16,6)` type as the seconds parameter and cast integer to `decimal(16,6)` is safe, so we can simplify the code. ### Why are the changes needed? Simplify `make_timestamp`. ### Does this PR introduce _any_ user-facing change? 'No'. ### How was this patch tested? New tests. Closes #33775 from beliefer/SPARK-36428-followup. Lead-authored-by: gengjiaan <gengji...@360.cn> Co-authored-by: Jiaan Geng <belie...@163.com> Signed-off-by: Gengliang Wang <gengli...@apache.org> (cherry picked from commit 707eefa3c706561f904dad65f3e347028dafb6ea) Signed-off-by: Gengliang Wang <gengli...@apache.org> --- .../catalyst/expressions/datetimeExpressions.scala | 33 ++++------------- .../expressions/DateExpressionsSuite.scala | 43 +++++++++------------- .../test/resources/sql-tests/inputs/timestamp.sql | 3 ++ .../sql-tests/results/ansi/timestamp.sql.out | 28 +++++++++++++- .../sql-tests/results/datetime-legacy.sql.out | 26 ++++++++++++- .../resources/sql-tests/results/timestamp.sql.out | 26 ++++++++++++- .../results/timestampNTZ/timestamp-ansi.sql.out | 28 +++++++++++++- .../results/timestampNTZ/timestamp.sql.out | 26 ++++++++++++- 8 files changed, 157 insertions(+), 56 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala index 84dfb41..0e74eff 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala @@ -2557,22 +2557,16 @@ case class MakeTimestamp( override def children: Seq[Expression] = Seq(year, month, day, hour, min, sec) ++ timezone // Accept `sec` as DecimalType to avoid loosing precision of microseconds while converting - // them to the fractional part of `sec`. + // them to the fractional part of `sec`. For accepts IntegerType as `sec` and integer can be + // casted into decimal safely, we use DecimalType(16, 6) which is wider than DecimalType(10, 0). override def inputTypes: Seq[AbstractDataType] = - Seq(IntegerType, IntegerType, IntegerType, IntegerType, IntegerType, - TypeCollection(DecimalType(8, 6), IntegerType, NullType)) ++ timezone.map(_ => StringType) + Seq(IntegerType, IntegerType, IntegerType, IntegerType, IntegerType, DecimalType(16, 6)) ++ + timezone.map(_ => StringType) override def nullable: Boolean = if (failOnError) children.exists(_.nullable) else true override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression = copy(timeZoneId = Option(timeZoneId)) - private lazy val toDecimal = sec.dataType match { - case DecimalType() => - (secEval: Any) => secEval.asInstanceOf[Decimal] - case IntegerType => - (secEval: Any) => Decimal(BigDecimal(secEval.asInstanceOf[Int]), 8, 6) - } - private def toMicros( year: Int, month: Int, @@ -2585,8 +2579,6 @@ case class MakeTimestamp( assert(secAndMicros.scale == 6, s"Seconds fraction must have 6 digits for microseconds but got ${secAndMicros.scale}") val unscaledSecFrac = secAndMicros.toUnscaledLong - assert(secAndMicros.precision <= 8, - s"Seconds and fraction cannot have more than 8 digits but got ${secAndMicros.precision}") val totalMicros = unscaledSecFrac.toInt // 8 digits cannot overflow Int val seconds = Math.floorDiv(totalMicros, MICROS_PER_SECOND.toInt) val nanos = Math.floorMod(totalMicros, MICROS_PER_SECOND.toInt) * NANOS_PER_MICROS.toInt @@ -2627,7 +2619,7 @@ case class MakeTimestamp( day.asInstanceOf[Int], hour.asInstanceOf[Int], min.asInstanceOf[Int], - toDecimal(sec), + sec.asInstanceOf[Decimal], zid) } @@ -2635,7 +2627,6 @@ case class MakeTimestamp( val dtu = DateTimeUtils.getClass.getName.stripSuffix("$") val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName) val d = Decimal.getClass.getName.stripSuffix("$") - val decimalValue = ctx.freshName("decimalValue") val failOnErrorBranch = if (failOnError) "throw e;" else s"${ev.isNull} = true;" nullSafeCodeGen(ctx, ev, (year, month, day, hour, min, secAndNanos, timezone) => { val zoneId = timezone.map(tz => s"$dtu.getZoneId(${tz}.toString())").getOrElse(zid) @@ -2647,21 +2638,11 @@ case class MakeTimestamp( } else { s"${ev.value} = $dtu.localDateTimeToMicros(ldt);" } - val toDecimalCode = sec.dataType match { - case DecimalType() => - s"org.apache.spark.sql.types.Decimal $decimalValue = $secAndNanos;" - case IntegerType => - s""" - |org.apache.spark.sql.types.Decimal $decimalValue = - |$d$$.MODULE$$.apply(new java.math.BigDecimal($secAndNanos), 8, 6); - """.stripMargin - } s""" try { - $toDecimalCode - org.apache.spark.sql.types.Decimal secFloor = $decimalValue.floor(); + org.apache.spark.sql.types.Decimal secFloor = $secAndNanos.floor(); org.apache.spark.sql.types.Decimal nanosPerSec = $d$$.MODULE$$.apply(1000000000L, 10, 0); - int nanos = (($decimalValue.$$minus(secFloor)).$$times(nanosPerSec)).toInt(); + int nanos = (($secAndNanos.$$minus(secFloor)).$$times(nanosPerSec)).toInt(); int seconds = secFloor.toInt(); java.time.LocalDateTime ldt; if (seconds == 60) { diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala index 0026f9a..d22a95f 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/DateExpressionsSuite.scala @@ -1161,18 +1161,19 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { withSQLConf(SQLConf.TIMESTAMP_TYPE.key -> tsType.toString) { val expected = expectedAnswer("2013-07-15 08:15:23.5") - Seq(true, false).foreach { ansi => + Seq(true).foreach { ansi => withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi.toString) { var makeTimestampExpr = MakeTimestamp( Literal(2013), Literal(7), Literal(15), Literal(8), Literal(15), - Literal(Decimal(BigDecimal(23.5), 8, 6)), Some(Literal(ZoneId.systemDefault().getId))) + Literal(Decimal(BigDecimal(23.5), 16, 6)), + Some(Literal(ZoneId.systemDefault().getId))) checkEvaluation(makeTimestampExpr, expected) checkEvaluation(makeTimestampExpr.copy(year = Literal.create(null, IntegerType)), null) checkEvaluation(makeTimestampExpr.copy(month = Literal.create(null, IntegerType)), null) checkEvaluation(makeTimestampExpr.copy(day = Literal.create(null, IntegerType)), null) checkEvaluation(makeTimestampExpr.copy(hour = Literal.create(null, IntegerType)), null) checkEvaluation(makeTimestampExpr.copy(min = Literal.create(null, IntegerType)), null) - checkEvaluation(makeTimestampExpr.copy(sec = Literal.create(null, DecimalType(8, 6))), + checkEvaluation(makeTimestampExpr.copy(sec = Literal.create(null, DecimalType(16, 6))), null) checkEvaluation(makeTimestampExpr.copy(timezone = None), expected) @@ -1183,7 +1184,7 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { (makeTimestampExpr.copy(hour = Literal(25)), "Invalid value for Hour"), (makeTimestampExpr.copy(min = Literal(65)), "Invalid value for Min"), (makeTimestampExpr.copy(sec = Literal(Decimal( - BigDecimal(70.0), 8, 6))), "Invalid value for Second") + BigDecimal(70.0), 16, 6))), "Invalid value for Second") ).foreach { entry => if (ansi) { checkExceptionInExpression[DateTimeException](entry._1, EmptyRow, entry._2) @@ -1193,16 +1194,16 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { } makeTimestampExpr = MakeTimestamp(Literal(2019), Literal(6), Literal(30), - Literal(23), Literal(59), Literal(Decimal(BigDecimal(60.0), 8, 6))) + Literal(23), Literal(59), Literal(Decimal(BigDecimal(60.0), 16, 6))) if (ansi) { checkExceptionInExpression[DateTimeException](makeTimestampExpr.copy(sec = Literal( - Decimal(BigDecimal(60.5), 8, 6))), EmptyRow, "The fraction of sec must be zero") + Decimal(BigDecimal(60.5), 16, 6))), EmptyRow, "The fraction of sec must be zero") } else { checkEvaluation(makeTimestampExpr, expectedAnswer("2019-07-01 00:00:00")) } makeTimestampExpr = MakeTimestamp(Literal(2019), Literal(8), Literal(12), Literal(0), - Literal(0), Literal(Decimal(BigDecimal(58.000001), 8, 6))) + Literal(0), Literal(Decimal(BigDecimal(58.000001), 16, 6))) checkEvaluation(makeTimestampExpr, expectedAnswer("2019-08-12 00:00:58.000001")) } } @@ -1210,26 +1211,18 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { // non-ansi test withSQLConf(SQLConf.ANSI_ENABLED.key -> "false") { val makeTimestampExpr = MakeTimestamp(Literal(2019), Literal(6), Literal(30), - Literal(23), Literal(59), Literal(Decimal(BigDecimal(60.0), 8, 6))) - checkEvaluation(makeTimestampExpr.copy(sec = Literal(Decimal(BigDecimal(60.5), 8, 6))), + Literal(23), Literal(59), Literal(Decimal(BigDecimal(60.0), 16, 6))) + checkEvaluation(makeTimestampExpr.copy(sec = Literal(Decimal(BigDecimal(60.5), 16, 6))), null) } Seq(true, false).foreach { ansi => withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi.toString) { val makeTimestampExpr = MakeTimestamp(Literal(2019), Literal(8), Literal(12), - Literal(0), Literal(0), Literal(Decimal(BigDecimal(58.000001), 8, 6))) + Literal(0), Literal(0), Literal(Decimal(BigDecimal(58.000001), 16, 6))) checkEvaluation(makeTimestampExpr, expectedAnswer("2019-08-12 00:00:58.000001")) } } - - Seq(true, false).foreach { ansi => - withSQLConf(SQLConf.ANSI_ENABLED.key -> ansi.toString) { - val makeTimestampExpr = MakeTimestamp(Literal(2019), Literal(8), Literal(12), - Literal(0), Literal(0), Literal(1)) - checkEvaluation(makeTimestampExpr, expectedAnswer("2019-08-12 00:00:01")) - } - } } } } @@ -1242,20 +1235,20 @@ class DateExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper { test("extract the seconds part with fraction from timestamps") { outstandingTimezonesIds.foreach { timezone => val timestamp = MakeTimestamp(Literal(2019), Literal(8), Literal(10), - Literal(0), Literal(0), Literal(Decimal(10.123456, 8, 6)), + Literal(0), Literal(0), Literal(Decimal(10.123456, 16, 6)), Some(Literal(timezone)), Some(timezone)) def secFrac(ts: MakeTimestamp): SecondWithFraction = SecondWithFraction(ts, Some(timezone)) - checkEvaluation(secFrac(timestamp), Decimal(10.123456, 8, 6)) + checkEvaluation(secFrac(timestamp), Decimal(10.123456, 16, 6)) checkEvaluation( - secFrac(timestamp.copy(sec = Literal(Decimal(59000001, 8, 6)))), - Decimal(59000001, 8, 6)) + secFrac(timestamp.copy(sec = Literal(Decimal(59000001, 16, 6)))), + Decimal(59000001, 16, 6)) checkEvaluation( - secFrac(timestamp.copy(sec = Literal(Decimal(1, 8, 6)))), - Decimal(0.000001, 8, 6)) + secFrac(timestamp.copy(sec = Literal(Decimal(1, 16, 6)))), + Decimal(0.000001, 16, 6)) checkEvaluation( secFrac(timestamp.copy(year = Literal(10))), - Decimal(10.123456, 8, 6)) + Decimal(10.123456, 16, 6)) } } diff --git a/sql/core/src/test/resources/sql-tests/inputs/timestamp.sql b/sql/core/src/test/resources/sql-tests/inputs/timestamp.sql index a55adb0..0bc77a8 100644 --- a/sql/core/src/test/resources/sql-tests/inputs/timestamp.sql +++ b/sql/core/src/test/resources/sql-tests/inputs/timestamp.sql @@ -22,6 +22,9 @@ SELECT make_timestamp(1, 1, 1, 1, 1, 1); SELECT make_timestamp(1, 1, 1, 1, 1, 60); SELECT make_timestamp(1, 1, 1, 1, 1, 61); SELECT make_timestamp(1, 1, 1, 1, 1, null); +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999); +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999); +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999); -- [SPARK-31710] TIMESTAMP_SECONDS, TIMESTAMP_MILLISECONDS and TIMESTAMP_MICROSECONDS that always create timestamp_ltz select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null); diff --git a/sql/core/src/test/resources/sql-tests/results/ansi/timestamp.sql.out b/sql/core/src/test/resources/sql-tests/results/ansi/timestamp.sql.out index 02138a6..432c597 100644 --- a/sql/core/src/test/resources/sql-tests/results/ansi/timestamp.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/ansi/timestamp.sql.out @@ -1,5 +1,5 @@ -- Automatically generated by SQLQueryTestSuite --- Number of queries: 86 +-- Number of queries: 89 -- !query @@ -141,6 +141,32 @@ NULL -- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 59.999999):timestamp> +-- !query output +0001-01-01 01:01:59.999999 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999) +-- !query schema +struct<> +-- !query output +java.time.DateTimeException +Invalid value for SecondOfMinute (valid values 0 - 59): 99 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999) +-- !query schema +struct<> +-- !query output +java.time.DateTimeException +Invalid value for SecondOfMinute (valid values 0 - 59): 999 + + +-- !query select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null) -- !query schema struct<timestamp_seconds(1230219000):timestamp,timestamp_seconds(-1230219000):timestamp,timestamp_seconds(NULL):timestamp> diff --git a/sql/core/src/test/resources/sql-tests/results/datetime-legacy.sql.out b/sql/core/src/test/resources/sql-tests/results/datetime-legacy.sql.out index a820bf6..573ce3d 100644 --- a/sql/core/src/test/resources/sql-tests/results/datetime-legacy.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/datetime-legacy.sql.out @@ -1,5 +1,5 @@ -- Automatically generated by SQLQueryTestSuite --- Number of queries: 163 +-- Number of queries: 166 -- !query @@ -791,6 +791,30 @@ NULL -- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 59.999999):timestamp> +-- !query output +0001-01-01 01:01:59.999999 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 99.999999):timestamp> +-- !query output +NULL + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 999.999999):timestamp> +-- !query output +NULL + + +-- !query select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null) -- !query schema struct<timestamp_seconds(1230219000):timestamp,timestamp_seconds(-1230219000):timestamp,timestamp_seconds(NULL):timestamp> diff --git a/sql/core/src/test/resources/sql-tests/results/timestamp.sql.out b/sql/core/src/test/resources/sql-tests/results/timestamp.sql.out index cf8fb9e..77b4f73 100644 --- a/sql/core/src/test/resources/sql-tests/results/timestamp.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/timestamp.sql.out @@ -1,5 +1,5 @@ -- Automatically generated by SQLQueryTestSuite --- Number of queries: 86 +-- Number of queries: 89 -- !query @@ -133,6 +133,30 @@ NULL -- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 59.999999):timestamp> +-- !query output +0001-01-01 01:01:59.999999 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 99.999999):timestamp> +-- !query output +NULL + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 999.999999):timestamp> +-- !query output +NULL + + +-- !query select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null) -- !query schema struct<timestamp_seconds(1230219000):timestamp,timestamp_seconds(-1230219000):timestamp,timestamp_seconds(NULL):timestamp> diff --git a/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp-ansi.sql.out b/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp-ansi.sql.out index 5d0f9aa..d35005a 100644 --- a/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp-ansi.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp-ansi.sql.out @@ -1,5 +1,5 @@ -- Automatically generated by SQLQueryTestSuite --- Number of queries: 86 +-- Number of queries: 89 -- !query @@ -141,6 +141,32 @@ NULL -- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 59.999999):timestamp_ntz> +-- !query output +0001-01-01 01:01:59.999999 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999) +-- !query schema +struct<> +-- !query output +java.time.DateTimeException +Invalid value for SecondOfMinute (valid values 0 - 59): 99 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999) +-- !query schema +struct<> +-- !query output +java.time.DateTimeException +Invalid value for SecondOfMinute (valid values 0 - 59): 999 + + +-- !query select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null) -- !query schema struct<timestamp_seconds(1230219000):timestamp,timestamp_seconds(-1230219000):timestamp,timestamp_seconds(NULL):timestamp> diff --git a/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp.sql.out b/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp.sql.out index 0b24c1e..c1d5bdb 100644 --- a/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp.sql.out +++ b/sql/core/src/test/resources/sql-tests/results/timestampNTZ/timestamp.sql.out @@ -1,5 +1,5 @@ -- Automatically generated by SQLQueryTestSuite --- Number of queries: 86 +-- Number of queries: 89 -- !query @@ -133,6 +133,30 @@ NULL -- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 59.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 59.999999):timestamp_ntz> +-- !query output +0001-01-01 01:01:59.999999 + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 99.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 99.999999):timestamp_ntz> +-- !query output +NULL + + +-- !query +SELECT make_timestamp(1, 1, 1, 1, 1, 999.999999) +-- !query schema +struct<make_timestamp(1, 1, 1, 1, 1, 999.999999):timestamp_ntz> +-- !query output +NULL + + +-- !query select TIMESTAMP_SECONDS(1230219000),TIMESTAMP_SECONDS(-1230219000),TIMESTAMP_SECONDS(null) -- !query schema struct<timestamp_seconds(1230219000):timestamp,timestamp_seconds(-1230219000):timestamp,timestamp_seconds(NULL):timestamp> --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@spark.apache.org For additional commands, e-mail: commits-h...@spark.apache.org