jubins commented on code in PR #59094:
URL: https://github.com/apache/spark/pull/59094#discussion_r4148194075
##########
sql/core/src/test/scala/org/apache/spark/sql/ApproxTopKSuite.scala:
##########
@@ -549,6 +549,71 @@ class ApproxTopKSuite extends SharedSparkSession {
)
}
+
+ private val nullSketchState =
+ """CAST(NULL AS STRUCT<sketch: BINARY, maxItemsTracked: INT,
+ | itemDataType: INT, itemDataTypeDDL: STRING>)""".stripMargin
+
+ test("SPARK-59818: estimate of a foldable NULL state returns NULL") {
+ checkAnswer(sql(s"SELECT approx_top_k_estimate($nullSketchState, 5)"),
Row(null))
+ }
+
+ test("SPARK-59818: estimate of a non-foldable NULL state returns NULL") {
+ withTempView("estimate_null_state") {
+ sql(
+ s"""SELECT approx_top_k_accumulate(expr) AS state
+ |FROM VALUES 0, 1, 1 AS tab(expr)
+ |UNION ALL
+ |SELECT $nullSketchState AS state""".stripMargin)
+ .createOrReplaceTempView("estimate_null_state")
+ val res = sql("SELECT approx_top_k_estimate(state, 2) FROM
estimate_null_state")
+ checkAnswer(res, Seq(Row(Seq(Row(1, 2), Row(0, 1))), Row(null)))
+ }
+ }
+
+ test("SPARK-59818: estimate nullability follows the nullability of its
state") {
+ withTempView("estimate_nullable_state") {
+ sql(s"SELECT $nullSketchState AS state")
+ .createOrReplaceTempView("estimate_nullable_state")
+ val nullableRes =
+ sql("SELECT approx_top_k_estimate(state, 2) FROM
estimate_nullable_state")
+ assert(nullableRes.schema.fields.head.nullable)
+ }
+ // The common case: a sketch produced by approx_top_k_accumulate is
non-nullable, so the
+ // estimate over it must stay non-nullable.
+ val nonNullableRes = sql(
+ """SELECT approx_top_k_estimate(approx_top_k_accumulate(expr), 2)
+ |FROM VALUES 0, 1, 1 AS tab(expr)""".stripMargin)
+ assert(!nonNullableRes.schema.fields.head.nullable)
+ }
+
+ test("SPARK-59818: estimate rejects a non-positive k even for a NULL state")
{
+ Seq(0, -1).foreach { invalidK =>
+ checkError(
+ exception = intercept[SparkRuntimeException] {
+ sql(s"SELECT approx_top_k_estimate($nullSketchState,
$invalidK)").collect()
+ },
+ condition = "APPROX_TOP_K_NON_POSITIVE_ARG",
+ parameters = Map("argName" -> "`k`", "argValue" -> invalidK.toString)
+ )
+ }
+ withTempView("estimate_invalid_k") {
+ sql(
+ s"""SELECT approx_top_k_accumulate(expr) AS state
Review Comment:
good catch, fixed this in d6d289d. the row-evaluated fixture is now NULL
states only, so nothing else can raise the error. confirmed by moving checkK
back behind the NULL return, which now fails where the old fixture stayed green.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
