Skip to content

Commit 68b4020

Browse files
lianchengcloud-fan
authored andcommitted
[SPARK-16648][SQL] Make ignoreNullsExpr a child expression of First and Last
## What changes were proposed in this pull request? Default `TreeNode.withNewChildren` implementation doesn't work for `Last` and when both constructor arguments are the same, e.g.: ```sql LAST_VALUE(FALSE) -- The 2nd argument defaults to FALSE LAST_VALUE(FALSE, FALSE) LAST_VALUE(TRUE, TRUE) ``` This is because although `Last` is a unary expression, both of its constructor arguments, `child` and `ignoreNullsExpr`, are `Expression`s. When they have the same value, `TreeNode.withNewChildren` treats both of them as child nodes by mistake. `First` is also affected by this issue in exactly the same way. This PR fixes this issue by making `ignoreNullsExpr` a child expression of `First` and `Last`. ## How was this patch tested? New test case added in `WindowQuerySuite`. Author: Cheng Lian <lian@databricks.com> Closes apache#14295 from liancheng/spark-16648-last-value.
1 parent 468a3c3 commit 68b4020

File tree

3 files changed

+16
-4
lines changed

3 files changed

+16
-4
lines changed

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/First.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ case class First(child: Expression, ignoreNullsExpr: Expression) extends Declara
4343
throw new AnalysisException("The second argument of First should be a boolean literal.")
4444
}
4545

46-
override def children: Seq[Expression] = child :: Nil
46+
override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil
4747

4848
override def nullable: Boolean = true
4949

@@ -54,7 +54,7 @@ case class First(child: Expression, ignoreNullsExpr: Expression) extends Declara
5454
override def dataType: DataType = child.dataType
5555

5656
// Expected input data type.
57-
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType)
57+
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, BooleanType)
5858

5959
private lazy val first = AttributeReference("first", child.dataType)()
6060

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/Last.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ case class Last(child: Expression, ignoreNullsExpr: Expression) extends Declarat
4040
throw new AnalysisException("The second argument of First should be a boolean literal.")
4141
}
4242

43-
override def children: Seq[Expression] = child :: Nil
43+
override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil
4444

4545
override def nullable: Boolean = true
4646

@@ -51,7 +51,7 @@ case class Last(child: Expression, ignoreNullsExpr: Expression) extends Declarat
5151
override def dataType: DataType = child.dataType
5252

5353
// Expected input data type.
54-
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType)
54+
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, BooleanType)
5555

5656
private lazy val last = AttributeReference("last", child.dataType)()
5757

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/WindowQuerySuite.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,4 +247,16 @@ class WindowQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleto
247247
|from part
248248
""".stripMargin))
249249
}
250+
251+
test("SPARK-16646: LAST_VALUE(FALSE) OVER ()") {
252+
checkAnswer(sql("SELECT LAST_VALUE(FALSE) OVER ()"), Row(false))
253+
checkAnswer(sql("SELECT LAST_VALUE(FALSE, FALSE) OVER ()"), Row(false))
254+
checkAnswer(sql("SELECT LAST_VALUE(TRUE, TRUE) OVER ()"), Row(true))
255+
}
256+
257+
test("SPARK-16646: FIRST_VALUE(FALSE) OVER ()") {
258+
checkAnswer(sql("SELECT FIRST_VALUE(FALSE) OVER ()"), Row(false))
259+
checkAnswer(sql("SELECT FIRST_VALUE(FALSE, FALSE) OVER ()"), Row(false))
260+
checkAnswer(sql("SELECT FIRST_VALUE(TRUE, TRUE) OVER ()"), Row(true))
261+
}
250262
}

0 commit comments

Comments
 (0)