You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "MaxGekk (via GitHub)" <gi...@apache.org> on 2023/05/25 20:05:37 UTC
[GitHub] [spark] MaxGekk commented on a diff in pull request #41203: [SPARK-16484][SQL] Update hll function type checks to also check for non-foldable inputs
MaxGekk commented on code in PR #41203:
URL: https://github.com/apache/spark/pull/41203#discussion_r1205944545
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala:
##########
@@ -1876,6 +1876,49 @@ class DataFrameAggregateSuite extends QueryTest
checkAnswer(res, Nil)
}
assert(error7.toString contains "UnsupportedOperationException")
+
+ // validate specific args require foldable = true
+ val error8 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (1, 12)
+ | as tab(col, logk)
+ |)
+ |select hll_sketch_agg(col, logk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error8.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
+
+ val error9 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (binary(1), true)
+ | as tab(col, allow_different_lgconfigk)
+ |)
+ |select hll_union_agg(col, allow_different_lgconfigk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error9.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
+
+ val error10 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (binary(1), binary(1), true)
+ | as tab(col1, col2, allow_different_lgconfigk)
+ |)
+ |select hll_union(col1, col2, allow_different_lgconfigk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error10.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
Review Comment:
Please, use checkError
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala:
##########
@@ -1876,6 +1876,49 @@ class DataFrameAggregateSuite extends QueryTest
checkAnswer(res, Nil)
}
assert(error7.toString contains "UnsupportedOperationException")
+
+ // validate specific args require foldable = true
+ val error8 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (1, 12)
+ | as tab(col, logk)
+ |)
+ |select hll_sketch_agg(col, logk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error8.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
Review Comment:
Could you use `checkError` for checking the error class and other fields, please.
##########
sql/core/src/test/scala/org/apache/spark/sql/DataFrameAggregateSuite.scala:
##########
@@ -1876,6 +1876,49 @@ class DataFrameAggregateSuite extends QueryTest
checkAnswer(res, Nil)
}
assert(error7.toString contains "UnsupportedOperationException")
+
+ // validate specific args require foldable = true
+ val error8 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (1, 12)
+ | as tab(col, logk)
+ |)
+ |select hll_sketch_agg(col, logk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error8.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
+
+ val error9 = intercept[AnalysisException] {
+ val res = sql(
+ """with cte as (
+ |select * from values
+ | (binary(1), true)
+ | as tab(col, allow_different_lgconfigk)
+ |)
+ |select hll_union_agg(col, allow_different_lgconfigk) from cte
+ |""".stripMargin
+ )
+ checkAnswer(res, Nil)
+ }
+ assert(error9.toString contains "UNEXPECTED_INPUT_FOLDABLE_VALUE")
Review Comment:
ditto: Use `checkError`
##########
core/src/main/resources/error/error-classes.json:
##########
@@ -471,6 +471,11 @@
"class <className> not found."
]
},
+ "UNEXPECTED_INPUT_FOLDABLE_VALUE" : {
+ "message" : [
+ "Parameter <paramIndex> requires a foldable value of <requiredFoldable>, however <inputSql> has a foldable value of <inputFoldable>."
Review Comment:
From user's point of view, the message is not clear. What's the `foldable value of true`?
Can't you just re-use the existing error class: `NON_FOLDABLE_INPUT`?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org