[SPARK-40152][SQL][TESTS] Add tests for SplitPart #37626
Conversation
| @@ -2522,4 +2522,24 @@ class CollectionExpressionsSuite extends SparkFunSuite with ExpressionEvalHelper | |||
| Date.valueOf("2017-02-12"))) | |||
| } | |||
| } | |||
|
|
|||
| test("SplitPart") { | |||
There was a problem hiding this comment.
It seems can't test SplitPart. So just test replacement:
checkEvaluation(SplitPart(Literal.create("11.12.13", StringType), Literal.create(".", StringType), Literal(3)), "13")Caused by: org.apache.spark.SparkUnsupportedOperationException: [INTERNAL_ERROR] Cannot evaluate expression: split_part(11.12.13, ., 3)
at org.apache.spark.sql.errors.QueryExecutionErrors$.cannotEvaluateExpressionError(QueryExecutionErrors.scala:71)
at org.apache.spark.sql.catalyst.expressions.RuntimeReplaceable.eval(Expression.scala:369)
at org.apache.spark.sql.catalyst.expressions.RuntimeReplaceable.eval$(Expression.scala:368)
at org.apache.spark.sql.catalyst.expressions.SplitPart.eval(stringExpressions.scala:2859)
There was a problem hiding this comment.
Can we simply add more tests in the existing test case test("elementAt")? We just need to test the outOfBoundValue
...catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
Show resolved
Hide resolved
| checkEvaluation(ElementAt(StringSplitSQL(Literal.create("11.12.13", StringType), | ||
| Literal.create(null, StringType)), Literal(1), outOfBoundValue), null) | ||
|
|
||
| intercept[Exception] { |
There was a problem hiding this comment.
Can we catch narrower exception?
There was a problem hiding this comment.
Do you mean TestFailedException?
There was a problem hiding this comment.
Yeah, let's catch a narrower exception.
There was a problem hiding this comment.
org.scalatest.exceptions.TestFailedException is scala test exception, not Spark exception. It seems meaningless:
intercept[SparkRuntimeException] {
checkEvaluation(ElementAt(str, Literal(0), outOfBoundValue), null)
}.getMessage.contains("The index 0 is invalid")Expected exception org.apache.spark.SparkRuntimeException to be thrown, but org.scalatest.exceptions.TestFailedException was thrown
ScalaTestFailureLocation: org.apache.spark.sql.catalyst.expressions.CollectionExpressionsSuite at (CollectionExpressionsSuite.scala:2543)
org.scalatest.exceptions.TestFailedException: Expected exception org.apache.spark.SparkRuntimeException to be thrown, but org.scalatest.exceptions.TestFailedException was thrown
at org.scalatest.Assertions.newAssertionFailedException(Assertions.scala:472)
at org.scalatest.Assertions.newAssertionFailedException$(Assertions.scala:471)
at org.scalatest.funsuite.AnyFunSuite.newAssertionFailedException(AnyFunSuite.scala:1563)
### What changes were proposed in this pull request? Add tests for `SplitPart`. ### Why are the changes needed? Improve test coverage. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? N/A. Closes #37626 from wangyum/SPARK-40152-2. Authored-by: Yuming Wang <yumwang@ebay.com> Signed-off-by: Sean Owen <srowen@gmail.com> (cherry picked from commit 4f525ee) Signed-off-by: Sean Owen <srowen@gmail.com>
|
Merged to master/3.3 |
| Literal.create(null, StringType)), Literal(1), outOfBoundValue), null) | ||
|
|
||
| intercept[Exception] { | ||
| checkEvaluation(ElementAt(str, Literal(0), outOfBoundValue), null) |
There was a problem hiding this comment.
We have a dedicated function checkExceptionInExpression for testing the expression error behavior.
There was a problem hiding this comment.
Actually this is already tested in test("elementAt")
There was a problem hiding this comment.
See https://github.com/apache/spark/pull/37626/files#r952639395
I think we should just expand the existing test("elementAt").
|
Ah sorry did it again, merged this already. If it's important make another PR and I'll merge it |
What changes were proposed in this pull request?
Add tests for
SplitPart.Why are the changes needed?
Improve test coverage.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
N/A.