Skip to content
Snippets Groups Projects
Commit 68b4020d authored by Cheng Lian's avatar Cheng Lian Committed by Wenchen Fan
Browse files

[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 #14295 from liancheng/spark-16648-last-value.
parent 468a3c3a
No related branches found
No related tags found
No related merge requests found
......@@ -43,7 +43,7 @@ case class First(child: Expression, ignoreNullsExpr: Expression) extends Declara
throw new AnalysisException("The second argument of First should be a boolean literal.")
}
override def children: Seq[Expression] = child :: Nil
override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil
override def nullable: Boolean = true
......@@ -54,7 +54,7 @@ case class First(child: Expression, ignoreNullsExpr: Expression) extends Declara
override def dataType: DataType = child.dataType
// Expected input data type.
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType)
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, BooleanType)
private lazy val first = AttributeReference("first", child.dataType)()
......
......@@ -40,7 +40,7 @@ case class Last(child: Expression, ignoreNullsExpr: Expression) extends Declarat
throw new AnalysisException("The second argument of First should be a boolean literal.")
}
override def children: Seq[Expression] = child :: Nil
override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil
override def nullable: Boolean = true
......@@ -51,7 +51,7 @@ case class Last(child: Expression, ignoreNullsExpr: Expression) extends Declarat
override def dataType: DataType = child.dataType
// Expected input data type.
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType)
override def inputTypes: Seq[AbstractDataType] = Seq(AnyDataType, BooleanType)
private lazy val last = AttributeReference("last", child.dataType)()
......
......@@ -247,4 +247,16 @@ class WindowQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleto
|from part
""".stripMargin))
}
test("SPARK-16646: LAST_VALUE(FALSE) OVER ()") {
checkAnswer(sql("SELECT LAST_VALUE(FALSE) OVER ()"), Row(false))
checkAnswer(sql("SELECT LAST_VALUE(FALSE, FALSE) OVER ()"), Row(false))
checkAnswer(sql("SELECT LAST_VALUE(TRUE, TRUE) OVER ()"), Row(true))
}
test("SPARK-16646: FIRST_VALUE(FALSE) OVER ()") {
checkAnswer(sql("SELECT FIRST_VALUE(FALSE) OVER ()"), Row(false))
checkAnswer(sql("SELECT FIRST_VALUE(FALSE, FALSE) OVER ()"), Row(false))
checkAnswer(sql("SELECT FIRST_VALUE(TRUE, TRUE) OVER ()"), Row(true))
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment