[SPARK-39905][SQL][TESTS] Remove checkErrorClass() and use checkError() instead
#37322
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
HyukjinKwon
reviewed
Jul 28, 2022
sql/core/src/test/scala/org/apache/spark/sql/DatasetUnpivotSuite.scala
Outdated
Show resolved
Hide resolved
MaxGekk
changed the title
checkErrorClass() and use checkError() insteadcheckErrorClass() and use checkError() instead
|
cc @srielau |
|
@cloud-fan @HyukjinKwon @gengliangwang Could you have a look at the PR, please. I wanna switch Serge's checkError completely, and extend checkError() to check query context as the next step. |
cloud-fan
reviewed
Jul 28, 2022
| "\"BIGINT\" \\(`long1#\\d+L`, `long2#\\d+L`\\)\\];(\n.*)*", | ||
| matchMsg = true) | ||
| errorSubClass = None, | ||
| sqlState = None, |
There was a problem hiding this comment.
Does checkError provide default parameter value for errorSubClass and sqlState?
There was a problem hiding this comment.
It has but if I remove settings of the parameters, I am getting the errors:
overloaded method value checkError with alternatives:
(exception: org.apache.spark.SparkThrowable,errorClass: String,parameters: Map[String,String])Unit <and>
(exception: org.apache.spark.SparkThrowable,errorClass: String,sqlState: String,parameters: Map[String,String])Unit <and>
(exception: org.apache.spark.SparkThrowable,errorClass: String,errorSubClass: String,sqlState: String,parameters: Map[String,String])Unit <and>
(exception: org.apache.spark.SparkThrowable,errorClass: String,errorSubClass: Option[String],sqlState: Option[String],parameters: Map[String,String],matchPVals: Boolean)Unit
cannot be applied to (exception: org.apache.spark.sql.AnalysisException, errorClass: String, parameters: scala.collection.immutable.Map[String,String], matchPVals: Boolean)
checkError(
There was a problem hiding this comment.
Let me add the overloaded method:
protected def checkError(
exception: SparkThrowable,
errorClass: String,
parameters: Map[String, String],
matchPVals: Boolean): Unit =There was a problem hiding this comment.
but I think it is better to set default values in the most wide method. I will remove one overloaded method and add default values.
|
@anchovYu @cloud-fan @HyukjinKwon @gengliangwang Could you review this PR, please. |
gengliangwang
approved these changes
Jul 29, 2022
|
Merging to master. Thank you, @gengliangwang for review. |
MaxGekk
added a commit
that referenced
this pull request
Aug 2, 2022
…Error()` ### What changes were proposed in this pull request? 1. Re-implemented `validateParsingError()` using `checkError()`. 2. Removed `checkParsingError()` and replaced by `checkError()`. ### Why are the changes needed? 1. To prepare test infra for testing of query contexts. 3. To check message parameters instead of entire text message. This PR is some kind of follow up of #36693 and #37322. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? By running the modified test suites: ``` $ build/sbt -Phive -Phive-thriftserver "test:testOnly *TruncateTableParserSuite" $ build/sbt -Phive-2.3 -Phive-thriftserver "test:testOnly *ShowPartitionsParserSuite" $ build/sbt "sql/testOnly *QueryParsingErrorsSuite" ``` Closes #37363 from MaxGekk/checkParsingError-to-checkError. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Replace all invokes of
checkErrorClass()bycheckError()and removecheckErrorClass().Why are the changes needed?
Does this PR introduce any user-facing change?
No.
How was this patch tested?
By running the modified test suites: