From 665ac467fb856e88355d9e06b532701132320106 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Tue, 30 Jun 2020 07:02:11 +0800 Subject: [PATCH 01/10] forbid time field steps for date start/end --- .../catalyst/expressions/collectionOperations.scala | 8 ++++++++ .../expressions/CollectionExpressionsSuite.scala | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index 026a2a677bae..b74ba557181a 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2612,6 +2612,9 @@ object Sequence { val stepDays = step.days val stepMicros = step.microseconds + require(scale != MICROS_PER_DAY || stepMonths != 0 || stepDays != 0, + "sequence step must be a day interval if start and end values are dates") + if (stepMonths == 0 && stepMicros == 0 && scale == MICROS_PER_DAY) { backedSequenceImpl.eval(start, stop, fromLong(stepDays)) @@ -2679,6 +2682,11 @@ object Sequence { |final int $stepDays = $step.days; |final long $stepMicros = $step.microseconds; | + |if (${scale}L == ${MICROS_PER_DAY}L && $stepMonths == 0 && $stepDays == 0) { + | throw new IllegalArgumentException( + | "sequence step must be a day interval if start and end values are dates"); + |} + | |if ($stepMonths == 0 && $stepMicros == 0 && ${scale}L == ${MICROS_PER_DAY}L) { | ${backedSequenceImpl.genCode(ctx, start, stop, stepDays, arr, elemType)}; | diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index 3a0c02b29d92..087a9216d142 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -1854,4 +1854,16 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper Literal(stringToInterval("interval 1 year"))), Seq(Date.valueOf("2018-01-01"))) } + + test("SPARK-31982: Sequence step must be a day interval " + + "if start and end values are dates") { + val e = intercept[Exception]( + checkEvaluation(Sequence( + Cast(Literal("2011-03-01"), DateType), + Cast(Literal("2011-04-01"), DateType), + Option(Literal(stringToInterval("interval 1 hour")))), + Seq(Date.valueOf("2011-03-01"), Date.valueOf("2011-04-01")))) + assert(e.getCause.getMessage.contains( + "sequence step must be a day interval if start and end values are dates")) + } } From 7733184e02c2e0b4951f4d4ec8b0542ce7e45653 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Tue, 30 Jun 2020 07:29:41 +0800 Subject: [PATCH 02/10] SPARK-32198 -> SPARK-32133 --- .../sql/catalyst/expressions/CollectionExpressionsSuite.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index 087a9216d142..5bb29bee04ba 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -1855,7 +1855,7 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper Seq(Date.valueOf("2018-01-01"))) } - test("SPARK-31982: Sequence step must be a day interval " + + test("SPARK-32133: Sequence step must be a day interval " + "if start and end values are dates") { val e = intercept[Exception]( checkEvaluation(Sequence( From 74c683f86d8a6cb72765afba9fa038fc48b3632d Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Tue, 30 Jun 2020 17:11:45 +0800 Subject: [PATCH 03/10] improve code --- .../expressions/collectionOperations.scala | 22 ++++++++++++++----- .../CollectionExpressionsSuite.scala | 18 ++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index b74ba557181a..956648821316 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2612,8 +2612,10 @@ object Sequence { val stepDays = step.days val stepMicros = step.microseconds - require(scale != MICROS_PER_DAY || stepMonths != 0 || stepDays != 0, - "sequence step must be a day interval if start and end values are dates") + if(scale == MICROS_PER_DAY && stepMonths == 0 && stepDays == 0) { + throw new IllegalArgumentException( + "sequence step must be a day interval if start and end values are dates") + } if (stepMonths == 0 && stepMicros == 0 && scale == MICROS_PER_DAY) { backedSequenceImpl.eval(start, stop, fromLong(stepDays)) @@ -2677,15 +2679,23 @@ object Sequence { |${genSequenceLengthCode(ctx, startMicros, stopMicros, intervalInMicros, arrLength)} """.stripMargin + val check = if (scale == MICROS_PER_DAY) { + s""" + if ($stepMonths == 0 && $stepDays == 0) { + throw new IllegalArgumentException( + "sequence step must be a day interval if start and end values are dates"); + } + """ + } else { + "" + } + s""" |final int $stepMonths = $step.months; |final int $stepDays = $step.days; |final long $stepMicros = $step.microseconds; | - |if (${scale}L == ${MICROS_PER_DAY}L && $stepMonths == 0 && $stepDays == 0) { - | throw new IllegalArgumentException( - | "sequence step must be a day interval if start and end values are dates"); - |} + |$check | |if ($stepMonths == 0 && $stepMicros == 0 && ${scale}L == ${MICROS_PER_DAY}L) { | ${backedSequenceImpl.genCode(ctx, start, stop, stepDays, arr, elemType)}; diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index 5bb29bee04ba..c87e8b02343d 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -1857,13 +1857,15 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper test("SPARK-32133: Sequence step must be a day interval " + "if start and end values are dates") { - val e = intercept[Exception]( - checkEvaluation(Sequence( - Cast(Literal("2011-03-01"), DateType), - Cast(Literal("2011-04-01"), DateType), - Option(Literal(stringToInterval("interval 1 hour")))), - Seq(Date.valueOf("2011-03-01"), Date.valueOf("2011-04-01")))) - assert(e.getCause.getMessage.contains( - "sequence step must be a day interval if start and end values are dates")) + checkExceptionInExpression[IllegalArgumentException](Sequence( + Cast(Literal("2011-03-01"), DateType), + Cast(Literal("2011-04-01"), DateType), + Option(Literal(stringToInterval("interval 1 hour")))), null, + "sequence step must be a day interval if start and end values are dates") + checkEvaluation(Sequence( + Cast(Literal("2011-03-01"), DateType), + Cast(Literal("2011-03-02"), DateType), + Option(Literal(stringToInterval("interval 1 day")))), + Seq(Date.valueOf("2011-03-01"), Date.valueOf("2011-03-02"))) } } From 9f910d7586b337041753a344b0163f5209cecaf3 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Wed, 1 Jul 2020 07:28:36 +0800 Subject: [PATCH 04/10] move test case to Sequence of dates --- .../CollectionExpressionsSuite.scala | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala index c87e8b02343d..856c1fad9b20 100644 --- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala +++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala @@ -933,6 +933,13 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper Literal(negateExact(stringToInterval("interval 1 month")))), EmptyRow, s"sequence boundaries: 0 to 2678400000000 by -${28 * MICROS_PER_DAY}") + + // SPARK-32133: Sequence step must be a day interval if start and end values are dates + checkExceptionInExpression[IllegalArgumentException](Sequence( + Cast(Literal("2011-03-01"), DateType), + Cast(Literal("2011-04-01"), DateType), + Option(Literal(stringToInterval("interval 1 hour")))), null, + "sequence step must be a day interval if start and end values are dates") } } @@ -1854,18 +1861,4 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper Literal(stringToInterval("interval 1 year"))), Seq(Date.valueOf("2018-01-01"))) } - - test("SPARK-32133: Sequence step must be a day interval " + - "if start and end values are dates") { - checkExceptionInExpression[IllegalArgumentException](Sequence( - Cast(Literal("2011-03-01"), DateType), - Cast(Literal("2011-04-01"), DateType), - Option(Literal(stringToInterval("interval 1 hour")))), null, - "sequence step must be a day interval if start and end values are dates") - checkEvaluation(Sequence( - Cast(Literal("2011-03-01"), DateType), - Cast(Literal("2011-03-02"), DateType), - Option(Literal(stringToInterval("interval 1 day")))), - Seq(Date.valueOf("2011-03-01"), Date.valueOf("2011-03-02"))) - } } From 9857fdc162c10c286a84353bd15bb42275abff19 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Thu, 2 Jul 2020 19:54:37 +0800 Subject: [PATCH 05/10] format improve --- .../catalyst/expressions/collectionOperations.scala | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index 956648821316..707a2b9f4e97 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2612,7 +2612,7 @@ object Sequence { val stepDays = step.days val stepMicros = step.microseconds - if(scale == MICROS_PER_DAY && stepMonths == 0 && stepDays == 0) { + if (scale == MICROS_PER_DAY && stepMonths == 0 && stepDays == 0) { throw new IllegalArgumentException( "sequence step must be a day interval if start and end values are dates") } @@ -2682,13 +2682,13 @@ object Sequence { val check = if (scale == MICROS_PER_DAY) { s""" if ($stepMonths == 0 && $stepDays == 0) { - throw new IllegalArgumentException( - "sequence step must be a day interval if start and end values are dates"); + throw new IllegalArgumentException( + "sequence step must be a day interval if start and end values are dates"); } """ - } else { - "" - } + } else { + "" + } s""" |final int $stepMonths = $step.months; From b0eb77fd0a9e93baba111ffaf5ed9f7a35731ad4 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Fri, 3 Jul 2020 06:50:10 +0800 Subject: [PATCH 06/10] code style improve --- .../catalyst/expressions/collectionOperations.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index 707a2b9f4e97..ba9e61d9811b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2681,11 +2681,11 @@ object Sequence { val check = if (scale == MICROS_PER_DAY) { s""" - if ($stepMonths == 0 && $stepDays == 0) { - throw new IllegalArgumentException( - "sequence step must be a day interval if start and end values are dates"); - } - """ + |if ($stepMonths == 0 && $stepDays == 0) { + | throw new IllegalArgumentException( + | "sequence step must be a day interval if start and end values are dates"); + |} + """.stripMargin } else { "" } From df2138c6ec5e4e0ded1fc42cffe6e1fa04473c47 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Fri, 3 Jul 2020 06:57:00 +0800 Subject: [PATCH 07/10] check position indentation --- .../spark/sql/catalyst/expressions/collectionOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index ba9e61d9811b..bddcda6acfdc 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2695,7 +2695,7 @@ object Sequence { |final int $stepDays = $step.days; |final long $stepMicros = $step.microseconds; | - |$check + | $check | |if ($stepMonths == 0 && $stepMicros == 0 && ${scale}L == ${MICROS_PER_DAY}L) { | ${backedSequenceImpl.genCode(ctx, start, stop, stepDays, arr, elemType)}; From b863eef24abd448e517e22b1a7545d6660041e03 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Fri, 3 Jul 2020 18:28:33 +0800 Subject: [PATCH 08/10] remove 2 space indentation --- .../spark/sql/catalyst/expressions/collectionOperations.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index bddcda6acfdc..ba9e61d9811b 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2695,7 +2695,7 @@ object Sequence { |final int $stepDays = $step.days; |final long $stepMicros = $step.microseconds; | - | $check + |$check | |if ($stepMonths == 0 && $stepMicros == 0 && ${scale}L == ${MICROS_PER_DAY}L) { | ${backedSequenceImpl.genCode(ctx, start, stop, stepDays, arr, elemType)}; From 9a521a3e3f3c767ed9a22185167beac60b97d344 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Mon, 6 Jul 2020 07:40:12 +0800 Subject: [PATCH 09/10] space correct --- .../spark/sql/catalyst/expressions/collectionOperations.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index bddcda6acfdc..a06997d9b516 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2682,8 +2682,8 @@ object Sequence { val check = if (scale == MICROS_PER_DAY) { s""" |if ($stepMonths == 0 && $stepDays == 0) { - | throw new IllegalArgumentException( - | "sequence step must be a day interval if start and end values are dates"); + | throw new IllegalArgumentException( + | "sequence step must be a day interval if start and end values are dates"); |} """.stripMargin } else { From d860c0e1bfddc3d2095c5e00b76508474ece5456 Mon Sep 17 00:00:00 2001 From: TJX2014 Date: Mon, 6 Jul 2020 17:50:00 +0800 Subject: [PATCH 10/10] new code comment to date and timestamp --- .../spark/sql/catalyst/expressions/collectionOperations.scala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala index ba9e61d9811b..bc1366a3493c 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala @@ -2618,9 +2618,11 @@ object Sequence { } if (stepMonths == 0 && stepMicros == 0 && scale == MICROS_PER_DAY) { + // Adding pure days to date start/end backedSequenceImpl.eval(start, stop, fromLong(stepDays)) } else if (stepMonths == 0 && stepDays == 0 && scale == 1) { + // Adding pure microseconds to timestamp start/end backedSequenceImpl.eval(start, stop, fromLong(stepMicros)) } else {