MaxGekk commented on a change in pull request #32985:
URL: https://github.com/apache/spark/pull/32985#discussion_r657275545
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
Review comment:
Please, add SPARK-35777 to the test title.
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
Review comment:
Please, simplify `YearMonthIntervalType(YEAR, YEAR)` to
`YearMonthIntervalType(YEAR)`
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
+ $"p".cast(YearMonthIntervalType(MONTH, MONTH)).as("p_m"))
val incMonth = udf((p: java.time.Period) => p.plusMonths(1))
- val result = input.select(incMonth($"p").as("new_p"))
- checkAnswer(result, Row(java.time.Period.ofYears(1)) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(result.schema === new StructType().add("new_p",
YearMonthIntervalType()))
+ val result = input.select(incMonth($"p").as("new_p"),
+ incMonth($"p_y").as("new_p_y"),
+ incMonth($"p_m").as("new_p_m"))
+ checkAnswer(result, Row(java.time.Period.ofMonths(14).normalized(),
+ java.time.Period.ofMonths(13).normalized(),
+ java.time.Period.ofMonths(14).normalized()) :: Nil)
+ assert(result.schema === new StructType()
+ .add("new_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_m", YearMonthIntervalType(YEAR, MONTH)))
Review comment:
Just the default one:
```suggestion
.add("new_p", YearMonthIntervalType())
.add("new_p_y", YearMonthIntervalType())
.add("new_p_m", YearMonthIntervalType()))
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
+ $"p".cast(YearMonthIntervalType(MONTH, MONTH)).as("p_m"))
val incMonth = udf((p: java.time.Period) => p.plusMonths(1))
- val result = input.select(incMonth($"p").as("new_p"))
- checkAnswer(result, Row(java.time.Period.ofYears(1)) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(result.schema === new StructType().add("new_p",
YearMonthIntervalType()))
+ val result = input.select(incMonth($"p").as("new_p"),
+ incMonth($"p_y").as("new_p_y"),
+ incMonth($"p_m").as("new_p_m"))
+ checkAnswer(result, Row(java.time.Period.ofMonths(14).normalized(),
+ java.time.Period.ofMonths(13).normalized(),
+ java.time.Period.ofMonths(14).normalized()) :: Nil)
+ assert(result.schema === new StructType()
+ .add("new_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_m", YearMonthIntervalType(YEAR, MONTH)))
// UDF produces `null`
val nullFunc = udf((_: java.time.Period) =>
null.asInstanceOf[java.time.Period])
- val nullResult = input.select(nullFunc($"p").as("null_p"))
- checkAnswer(nullResult, Row(null) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(nullResult.schema === new StructType().add("null_p",
YearMonthIntervalType()))
+ val nullResult = input.select(nullFunc($"p").as("null_p"),
+ nullFunc($"p_y").as("null_p_y"), nullFunc($"p_m").as("null_p_m"))
+ checkAnswer(nullResult, Row(null, null, null) :: Nil)
+ assert(nullResult.schema === new StructType()
+ .add("null_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_m", YearMonthIntervalType(YEAR, MONTH)))
// Input parameter of UDF is null
val nullInput = Seq(null.asInstanceOf[java.time.Period]).toDF("null_p")
+ .select($"null_p",
+ $"null_p".cast(YearMonthIntervalType(YEAR, YEAR)).as("null_p_y"),
+ $"null_p".cast(YearMonthIntervalType(MONTH, MONTH)).as("null_p_m"))
val constPeriod = udf((_: java.time.Period) =>
java.time.Period.ofYears(10))
- val constResult = nullInput.select(constPeriod($"null_p").as("10_years"))
- checkAnswer(constResult, Row(java.time.Period.ofYears(10)) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(constResult.schema === new StructType().add("10_years",
YearMonthIntervalType()))
+ val constResult = nullInput.select(constPeriod($"null_p").as("10_years"),
+ constPeriod($"null_p_y").as("p_y_10_years"),
constPeriod($"null_p_m").as("pm_10_years"))
+ checkAnswer(constResult, Row(java.time.Period.ofYears(10),
+ java.time.Period.ofYears(10), java.time.Period.ofYears(10)) :: Nil)
+ assert(constResult.schema === new StructType()
+ .add("10_years", YearMonthIntervalType())
+ .add("p_y_10_years", YearMonthIntervalType(YEAR, MONTH))
+ .add("pm_10_years", YearMonthIntervalType(YEAR, MONTH)))
Review comment:
Let's be consistent with above line:
```suggestion
.add("p_y_10_years", YearMonthIntervalType())
.add("pm_10_years", YearMonthIntervalType()))
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
+ $"p".cast(YearMonthIntervalType(MONTH, MONTH)).as("p_m"))
val incMonth = udf((p: java.time.Period) => p.plusMonths(1))
- val result = input.select(incMonth($"p").as("new_p"))
- checkAnswer(result, Row(java.time.Period.ofYears(1)) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(result.schema === new StructType().add("new_p",
YearMonthIntervalType()))
+ val result = input.select(incMonth($"p").as("new_p"),
+ incMonth($"p_y").as("new_p_y"),
+ incMonth($"p_m").as("new_p_m"))
+ checkAnswer(result, Row(java.time.Period.ofMonths(14).normalized(),
+ java.time.Period.ofMonths(13).normalized(),
+ java.time.Period.ofMonths(14).normalized()) :: Nil)
+ assert(result.schema === new StructType()
+ .add("new_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_m", YearMonthIntervalType(YEAR, MONTH)))
// UDF produces `null`
val nullFunc = udf((_: java.time.Period) =>
null.asInstanceOf[java.time.Period])
- val nullResult = input.select(nullFunc($"p").as("null_p"))
- checkAnswer(nullResult, Row(null) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(nullResult.schema === new StructType().add("null_p",
YearMonthIntervalType()))
+ val nullResult = input.select(nullFunc($"p").as("null_p"),
+ nullFunc($"p_y").as("null_p_y"), nullFunc($"p_m").as("null_p_m"))
+ checkAnswer(nullResult, Row(null, null, null) :: Nil)
+ assert(nullResult.schema === new StructType()
+ .add("null_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_m", YearMonthIntervalType(YEAR, MONTH)))
Review comment:
Let's simplify this:
```suggestion
.add("null_p", YearMonthIntervalType())
.add("null_p_y", YearMonthIntervalType())
.add("null_p_m", YearMonthIntervalType()))
```
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
+ $"p".cast(YearMonthIntervalType(MONTH, MONTH)).as("p_m"))
Review comment:
Use `YearMonthIntervalType(MONTH)`
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/UDFSuite.scala
##########
@@ -904,25 +905,43 @@ class UDFSuite extends QueryTest with SharedSparkSession {
test("SPARK-34663: using java.time.Period in UDF") {
// Regular case
- val input = Seq(java.time.Period.ofMonths(11)).toDF("p")
+ val input = Seq(java.time.Period.ofMonths(13)).toDF("p")
+ .select($"p", $"p".cast(YearMonthIntervalType(YEAR, YEAR)).as("p_y"),
+ $"p".cast(YearMonthIntervalType(MONTH, MONTH)).as("p_m"))
val incMonth = udf((p: java.time.Period) => p.plusMonths(1))
- val result = input.select(incMonth($"p").as("new_p"))
- checkAnswer(result, Row(java.time.Period.ofYears(1)) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(result.schema === new StructType().add("new_p",
YearMonthIntervalType()))
+ val result = input.select(incMonth($"p").as("new_p"),
+ incMonth($"p_y").as("new_p_y"),
+ incMonth($"p_m").as("new_p_m"))
+ checkAnswer(result, Row(java.time.Period.ofMonths(14).normalized(),
+ java.time.Period.ofMonths(13).normalized(),
+ java.time.Period.ofMonths(14).normalized()) :: Nil)
+ assert(result.schema === new StructType()
+ .add("new_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("new_p_m", YearMonthIntervalType(YEAR, MONTH)))
// UDF produces `null`
val nullFunc = udf((_: java.time.Period) =>
null.asInstanceOf[java.time.Period])
- val nullResult = input.select(nullFunc($"p").as("null_p"))
- checkAnswer(nullResult, Row(null) :: Nil)
- // TODO(SPARK-35777): Check all year-month interval types in UDF
- assert(nullResult.schema === new StructType().add("null_p",
YearMonthIntervalType()))
+ val nullResult = input.select(nullFunc($"p").as("null_p"),
+ nullFunc($"p_y").as("null_p_y"), nullFunc($"p_m").as("null_p_m"))
+ checkAnswer(nullResult, Row(null, null, null) :: Nil)
+ assert(nullResult.schema === new StructType()
+ .add("null_p", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_y", YearMonthIntervalType(YEAR, MONTH))
+ .add("null_p_m", YearMonthIntervalType(YEAR, MONTH)))
// Input parameter of UDF is null
val nullInput = Seq(null.asInstanceOf[java.time.Period]).toDF("null_p")
+ .select($"null_p",
+ $"null_p".cast(YearMonthIntervalType(YEAR, YEAR)).as("null_p_y"),
+ $"null_p".cast(YearMonthIntervalType(MONTH, MONTH)).as("null_p_m"))
Review comment:
```suggestion
$"null_p".cast(YearMonthIntervalType(YEAR)).as("null_p_y"),
$"null_p".cast(YearMonthIntervalType(MONTH)).as("null_p_m"))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
